make /dev/pci really readable

[Scott Long]

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 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-arch
> To unsubscribe, send any mail to "freebsd-arch-unsubscribe at freebsd.org"