From owner-freebsd-arch@FreeBSD.ORG Mon Jun 16 13:43:15 2003 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id BFBD537B408; Mon, 16 Jun 2003 13:43:12 -0700 (PDT) Received: from magic.adaptec.com (magic-mail.adaptec.com [208.236.45.100]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0F71343F75; Mon, 16 Jun 2003 13:43:12 -0700 (PDT) (envelope-from scottl@freebsd.org) Received: from redfish.adaptec.com (redfish.adaptec.com [162.62.50.11]) by magic.adaptec.com (8.11.6/8.11.6) with ESMTP id h5GKh1807861; Mon, 16 Jun 2003 13:43:01 -0700 Received: from freebsd.org (hollin.btc.adaptec.com [10.100.253.56]) by redfish.adaptec.com (8.8.8p2+Sun/8.8.8) with ESMTP id NAA16821; Mon, 16 Jun 2003 13:43:11 -0700 (PDT) Message-ID: <3EEE2B31.4020406@freebsd.org> Date: Mon, 16 Jun 2003 14:40:17 -0600 From: Scott Long User-Agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.3) Gecko/20030414 X-Accept-Language: en-us, en MIME-Version: 1.0 To: John-Mark Gurney References: <20030616074122.GF73854@funkthat.com> <20030616194935.GR73854@funkthat.com> In-Reply-To: <20030616194935.GR73854@funkthat.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit cc: arch@freebsd.org cc: Robert Watson Subject: Re: make /dev/pci really readable X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Jun 2003 20:43:16 -0000 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"