cleanup /dev/pci code some:

	read permision only required for listing, read/write required for
		read/write to registers
	fix a possible memory leak
	clean up error handling a bit

Reviewed by:	silence

1 files changed, 20 insertions, 54 deletions

diff --git a/sys/dev/pci/pci_user.c b/sys/dev/pci/pci_user.c
index f39a890..252422f 100644
--- a/sys/dev/pci/pci_user.c
+++ b/sys/dev/pci/pci_user.c
@@ -176,9 +176,14 @@ pci_ioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct thread *td)
 	const char *name;
 	int error;
 
-	if (!(flag & FWRITE))
+	if (!(flag & FWRITE) && cmd != PCIOCGETCONF)
 		return EPERM;
 
+	/* make sure register is in bounds and aligned */
+	if (cmd == PCIOCREAD || cmd == PCIOCWRITE)
+		if (io->pi_reg < 0 || io->pi_reg + io->pi_width > PCI_REGMAX ||
+		    io->pi_reg & (io->pi_width - 1))
+			error = EINVAL;
 
 	switch(cmd) {
 	case PCIOCGETCONF:
@@ -197,15 +202,6 @@ pci_ioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct thread *td)
 		dinfo = NULL;
 
 		/*
-		 * Hopefully the user won't pass in a null pointer, but it
-		 * can't hurt to check.
-		 */
-		if (cio == NULL) {
-			error = EINVAL;
-			break;
-		}
-
-		/*
 		 * If the user specified an offset into the device list,
 		 * but the list has changed since they last called this
 		 * ioctl, tell them that the list has changed.  They will
@@ -272,42 +268,22 @@ pci_ioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct thread *td)
 			    sizeof(struct pci_match_conf)) != cio->pat_buf_len){
 				/* The user made a mistake, return an error*/
 				cio->status = PCI_GETCONF_ERROR;
-				printf("pci_ioctl: pat_buf_len %d != "
-				       "num_patterns (%d) * sizeof(struct "
-				       "pci_match_conf) (%d)\npci_ioctl: "
-				       "pat_buf_len should be = %d\n",
-				       cio->pat_buf_len, cio->num_patterns,
-				       (int)sizeof(struct pci_match_conf),
-				       (int)sizeof(struct pci_match_conf) * 
-				       cio->num_patterns);
-				printf("pci_ioctl: do your headers match your "
-				       "kernel?\n");
 				cio->num_matches = 0;
 				error = EINVAL;
 				break;
 			}
 
 			/*
-			 * Check the user's buffer to make sure it's readable.
-			 */
-			if (!useracc((caddr_t)cio->patterns,
-				    cio->pat_buf_len, VM_PROT_READ)) {
-				printf("pci_ioctl: pattern buffer %p, "
-				       "length %u isn't user accessible for"
-				       " READ\n", cio->patterns,
-				       cio->pat_buf_len);
-				error = EACCES;
-				break;
-			}
-			/*
 			 * Allocate a buffer to hold the patterns.
 			 */
 			pattern_buf = malloc(cio->pat_buf_len, M_TEMP,
 					     M_WAITOK);
 			error = copyin(cio->patterns, pattern_buf,
 				       cio->pat_buf_len);
-			if (error != 0)
-				break;
+			if (error != 0) {
+				error = EINVAL;
+				goto getconfexit;
+			}
 			num_patterns = cio->num_patterns;
 
 		} else if ((cio->num_patterns > 0)
@@ -317,32 +293,19 @@ pci_ioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct thread *td)
 			 */
 			cio->status = PCI_GETCONF_ERROR;
 			cio->num_matches = 0;
-			printf("pci_ioctl: invalid GETCONF arguments\n");
 			error = EINVAL;
 			break;
 		} else
 			pattern_buf = NULL;
 
 		/*
-		 * Make sure we can write to the match buffer.
-		 */
-		if (!useracc((caddr_t)cio->matches,
-			     cio->match_buf_len, VM_PROT_WRITE)) {
-			printf("pci_ioctl: match buffer %p, length %u "
-			       "isn't user accessible for WRITE\n",
-			       cio->matches, cio->match_buf_len);
-			error = EACCES;
-			break;
-		}
-
-		/*
 		 * Go through the list of devices and copy out the devices
 		 * that match the user's criteria.
 		 */
 		for (cio->num_matches = 0, error = 0, i = 0,
 		     dinfo = STAILQ_FIRST(devlist_head);
 		     (dinfo != NULL) && (cio->num_matches < ionum)
-		     && (error == 0) && (i < pci_numdevs);
+		     && (error == 0) && (i < pci_numdevs) && (dinfo != NULL);
 		     dinfo = STAILQ_NEXT(dinfo, pci_links), i++) {
 
 			if (i < cio->offset)
@@ -375,10 +338,12 @@ pci_ioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct thread *td)
 				if (cio->num_matches >= ionum)
 					break;
 
-				error = copyout(&dinfo->conf,
-					        &cio->matches[cio->num_matches],
-						sizeof(struct pci_conf));
-				cio->num_matches++;
+				/* only if can copy it out do we count it */
+				if (!(error = copyout(&dinfo->conf,
+				    &cio->matches[cio->num_matches],
+				    sizeof(struct pci_conf)))) {
+					cio->num_matches++;
+				}
 			}
 		}
 
@@ -405,6 +370,7 @@ pci_ioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct thread *td)
 		else
 			cio->status = PCI_GETCONF_MORE_DEVS;
 
+getconfexit:
 		if (pattern_buf != NULL)
 			free(pattern_buf, M_TEMP);
 
@@ -439,7 +405,7 @@ pci_ioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct thread *td)
 			}
 			break;
 		default:
-			error = ENODEV;
+			error = EINVAL;
 			break;
 		}
 		break;
@@ -473,7 +439,7 @@ pci_ioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct thread *td)
 			}
 			break;
 		default:
-			error = ENODEV;
+			error = EINVAL;
 			break;
 		}
 		break;