Date: Tue, 17 Jun 2003 12:36:46 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Robert Watson <rwatson@freebsd.org> Cc: John-Mark Gurney <gurney_j@efn.org> Subject: Re: make /dev/pci really readable Message-ID: <20030617120956.N30677@gamplex.bde.org> In-Reply-To: <Pine.NEB.3.96L.1030616155956.8726C-100000@fledge.watson.org> References: <Pine.NEB.3.96L.1030616155956.8726C-100000@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 16 Jun 2003, Robert Watson wrote: > 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. Yes, it seems that -l is fairly safe, and it wasn't the default just because no one wrote the small hack to change the permissions check for the -l case only. > > > 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. I.e., if you are using a console. Spamming of syscons consoles messes them up too, and is more likely to cause panics since the syscons console output is less reentrant than sio console output. > > 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. useracc() can be used to "improve" error handling. That is done here. The "improvements" include dangerous printf()s and returning the undocumented errno EACCES instead of the incompletely documented one EFAULT. > 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. Please check the error handling carefully if you remove the useracc()'s. The one for writing the results back to userland gets replaced by checks in copyout() for each part of the results. I think we give up after the first copyout() error and return that error. That seems right. > The (cio==NULL) test is also > redundant. It is just bogus. cio is a kernel pointer and we need it to be more than non-NULL to access it. sys_generic.c:ioctl() guarantees this provided the definition of PCICGETCONF is correct. > 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. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030617120956.N30677>