Date: Fri, 20 Jun 2003 18:10:02 -0700 From: John-Mark Gurney <gurney_j@efn.org> To: Bruce Evans <bde@zeta.org.au>, Robert Watson <rwatson@freebsd.org>, arch@freebsd.org Subject: Re: make /dev/pci really readable Message-ID: <20030621011002.GG15336@funkthat.com> In-Reply-To: <20030617052917.GF73854@funkthat.com> References: <Pine.NEB.3.96L.1030616155956.8726C-100000@fledge.watson.org> <20030617120956.N30677@gamplex.bde.org> <20030617052917.GF73854@funkthat.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--ReaqsoxgOBHFXBhH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
John-Mark Gurney wrote this message on Mon, Jun 16, 2003 at 22:29 -0700:
> Bruce Evans wrote this message on Tue, Jun 17, 2003 at 12:36 +1000:
> > On Mon, 16 Jun 2003, Robert Watson wrote:
> > > It looks like (although I haven't tried), user processes can
> > > also cause the kernel to allocate unlimited amounts of kernel memory,
> > > which is another bit we probably need to tighten down.
> >
> > Much more serious.
>
> Yep, the pattern_buf is allocated, and in some cases a berak happens
> w/o freeing it. So there is a memory leak her. Will be fixed soon.
Ok, I think I have a good patch. It's attached. Fixes the memory
leak. I have also fix the pci manpage to talk about the errors, but
it isn't included in the patch.
--
John-Mark Gurney Voice: +1 415 225 5579
"All that I will do, has been done, All that I have, has not."
--ReaqsoxgOBHFXBhH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="pci_user2.patch"
Index: pci_user.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/pci/pci_user.c,v
retrieving revision 1.9
diff -u -r1.9 pci_user.c
--- pci_user.c 2003/03/03 12:15:44 1.9
+++ pci_user.c 2003/06/21 00:29:48
@@ -176,9 +176,14 @@
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 @@
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 @@
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 @@
*/
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 @@
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 @@
else
cio->status = PCI_GETCONF_MORE_DEVS;
+getconfexit:
if (pattern_buf != NULL)
free(pattern_buf, M_TEMP);
@@ -439,7 +405,7 @@
}
break;
default:
- error = ENODEV;
+ error = EINVAL;
break;
}
break;
@@ -473,7 +439,7 @@
}
break;
default:
- error = ENODEV;
+ error = EINVAL;
break;
}
break;
--ReaqsoxgOBHFXBhH--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030621011002.GG15336>