Date: Mon, 16 Jun 2003 14:40:17 -0600 From: Scott Long <scottl@freebsd.org> To: John-Mark Gurney <gurney_j@efn.org> Cc: Robert Watson <rwatson@freebsd.org> Subject: Re: make /dev/pci really readable Message-ID: <3EEE2B31.4020406@freebsd.org> In-Reply-To: <20030616194935.GR73854@funkthat.com> References: <20030616074122.GF73854@funkthat.com> <Pine.NEB.3.96L.1030616135002.8726B-100000@fledge.watson.org> <20030616194935.GR73854@funkthat.com>
next in thread | previous in thread | raw e-mail | index | archive | help
You should not always assume that reading PCI registers has no side-effects. It is certainly legal and possible for a PCI device to detect the read request and alter the contents of the register (or some other register) as a side effect, or change an internal state machine. 'Fixing' the various bits to allow unpriviledged access to 'pciconf -r' is dangerous since you would have to teach the system about every pci device in existance and how to trap on registers that have side-effects. I see little reason why unpriviledged users should be given register-level access to anything. We don't let them read /dev/mem, do we? Fixing 'pciconf -l' is fine, but it really doesn't need to extend beyond that. I would consider 'pciconf -r' to be a security risk and would treat it as such when it comes time for a release. Scott John-Mark Gurney wrote: > Robert Watson wrote this message on Mon, Jun 16, 2003 at 13:54 -0400: > >>On Mon, 16 Jun 2003, John-Mark Gurney wrote: >>will affect all users in the system). Could you do that cleanup in the >>first pass, then revisit the permissions change? > > > ok, I've taken a look at it, and I don't see any major problems with it > besides what I mentioned earlier. I've attached a patch that only lets > you do pciconf -l (query pci devices we know about), but it also enforces > the register to be in valid bounds and also makes sure it's aligned. I > think it's safest to do it here. Should I copy the restrictions on > read into the write block (actually, take them out of the case so they > can be shared)? > > > > ------------------------------------------------------------------------ > > 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/16 19:44:39 > @@ -176,7 +176,7 @@ > const char *name; > int error; > > - if (!(flag & FWRITE)) > + if (!(flag & FWRITE) && cmd != PCIOCGETCONF) > return EPERM; > > > @@ -342,7 +342,7 @@ > 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) > @@ -412,7 +412,10 @@ > } > case PCIOCREAD: > io = (struct pci_io *)data; > - switch(io->pi_width) { > + if (io->pi_reg < 0 || io->pi_reg + io_pi_width > PCI_REGMAX || > + io->pi_reg & (io->pi_width - 1)) > + error = EINVAL; > + else switch(io->pi_width) { > case 4: > case 2: > case 1: > @@ -439,7 +442,7 @@ > } > break; > default: > - error = ENODEV; > + error = EINVAL; > break; > } > break; > > > ------------------------------------------------------------------------ > > _______________________________________________ > freebsd-arch@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3EEE2B31.4020406>