From owner-freebsd-arch@FreeBSD.ORG Mon Jun 16 19:36:51 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 C3A1E37B401; Mon, 16 Jun 2003 19:36:51 -0700 (PDT) Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by mx1.FreeBSD.org (Postfix) with ESMTP id 6B89943FDD; Mon, 16 Jun 2003 19:36:50 -0700 (PDT) (envelope-from bde@zeta.org.au) Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246]) by mailman.zeta.org.au (8.9.3p2/8.8.7) with ESMTP id MAA09776; Tue, 17 Jun 2003 12:36:46 +1000 Date: Tue, 17 Jun 2003 12:36:46 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Robert Watson In-Reply-To: Message-ID: <20030617120956.N30677@gamplex.bde.org> References: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII cc: arch@freebsd.org cc: John-Mark Gurney 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: Tue, 17 Jun 2003 02:36:52 -0000 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