Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 16 Jun 2003 16:12:41 -0400 (EDT)
From:      Robert Watson <rwatson@freebsd.org>
To:        John-Mark Gurney <gurney_j@efn.org>
Cc:        arch@freebsd.org
Subject:   Re: make /dev/pci really readable
Message-ID:  <Pine.NEB.3.96L.1030616155956.8726C-100000@fledge.watson.org>
In-Reply-To: <20030616184204.GL73854@funkthat.com>

next in thread | previous in thread | raw e-mail | index | archive | help

On Mon, 16 Jun 2003, 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:
> > 
> > > Does anyone have an objection to making /dev/pci really honor the
> > > permissions, and giving normal users (or just group wheel) premission to
> > > run pciconf -l.  Right now the code requires the write bit set for any
> > > operation. 
> > 
> > I seem to recall that there was a problem wherein user processes could
> > cause cause unaligned accesses using /dev/pci.  There's also some rather
> 
> again, I just proposed -l, not -r to become user readable. I know that
> -r has problems.  I've crashed the sparc box a number of times by
> specifing pciconf -r pci1:5:0 0x0:0xf. 

I guess I found the question unclear -- you didn't quite specify what the
change you were making was...

> > odd use of useracc(), printf(), etc, in the ioctl code.  I suspect this
> 
> well, do you mean odd use of printf as in providing diagnostics to catch
> mismatched userland/kernel?

Spamming the console with messages can have debilitating effects on the
operation of the system if performed by unprivileged users...  I.e., if
you're using a serial console.

> for useracc, it checks to make sure that various pointers passed to it
> are either readable or writable.  I don't see this as odd.  Or is there
> another better method of checking user data when accessing user space
> buffers?

Generally, calls to useracc() are redundant with the existing checks in
our copyin/out routines, or are signs that the proper routines aren't
being used.  All of the fuword/suword/copyin/copyout/uio routines already
perform any necessary checks; manual checking is race-prone in a
multi-threaded smp environment regardless.  The (cio==NULL) test is also
redundant.  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.

One of problems with exposing this sort of code to unprivileged consumers
is that frequently (unfortunately), the code is not robust against a
malicious consumer. 

> other than a minor bug that could hit if there was more pci_devinfo's in
> the list than pci_numdevs (which should never happen, but will prevent a
> NULL deref), I didn't see anything wrong with -l. 

Allowing arbitrary users to panic the system is bad :-).

> > code needs some fairly thorough review and cleanup before we should reduce
> > the level of privilege required to use the device (note that we make it
> > world readable by default, so changes in the semantics of read permissions
> 
> > will affect all users in the system).  Could you do that cleanup in the
> > first pass, then revisit the permissions change?
> 
> sure, no problem. 

Great.  I think that there's nothing with the idea of loosening up the
restrictions here; one question I do have to wonder about in the long term
is whether this is information that should be exported using one of the
existing device general-purpose device configuration monitoring
interfaces, such as the sysctls used to support devinfo(8) in some more
generic form. 

Robert N M Watson             FreeBSD Core Team, TrustedBSD Projects
robert@fledge.watson.org      Network Associates Laboratories




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1030616155956.8726C-100000>