From owner-freebsd-arch@FreeBSD.ORG Mon Jun 16 22:28:48 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 E0DCF37B401; Mon, 16 Jun 2003 22:28:47 -0700 (PDT) Received: from mail.cyberonic.com (mail.cyberonic.com [4.17.179.4]) by mx1.FreeBSD.org (Postfix) with ESMTP id D4CF643FBF; Mon, 16 Jun 2003 22:28:46 -0700 (PDT) (envelope-from jmg@hydrogen.funkthat.com) Received: from hydrogen.funkthat.com (node-40244c0a.sfo.onnet.us.uu.net [64.36.76.10]) by mail.cyberonic.com (8.12.8/8.12.5) with ESMTP id h5H5sTMo014071; Tue, 17 Jun 2003 01:54:29 -0400 Received: (from jmg@localhost) by hydrogen.funkthat.com (8.12.9/8.11.6) id h5H5TH6g021743; Mon, 16 Jun 2003 22:29:17 -0700 (PDT) (envelope-from jmg) Date: Mon, 16 Jun 2003 22:29:17 -0700 From: John-Mark Gurney To: Bruce Evans Message-ID: <20030617052917.GF73854@funkthat.com> Mail-Followup-To: Bruce Evans , Robert Watson , arch@freebsd.org References: <20030617120956.N30677@gamplex.bde.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20030617120956.N30677@gamplex.bde.org> User-Agent: Mutt/1.4.1i X-Operating-System: FreeBSD 4.2-RELEASE i386 X-PGP-Fingerprint: B7 EC EF F8 AE ED A7 31 96 7A 22 B3 D8 56 36 F4 X-Files: The truth is out there X-URL: http://resnet.uoregon.edu/~gurney_j/ X-Resume: http://resnet.uoregon.edu/~gurney_j/resume.html 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 Reply-To: John-Mark Gurney 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 05:28:48 -0000 Bruce Evans wrote this message on Tue, Jun 17, 2003 at 12:36 +1000: > On Mon, 16 Jun 2003, Robert Watson wrote: > > > On Mon, 16 Jun 2003, John-Mark Gurney wrote: > > > > > 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. Ok, sounds like most people agree on -l being readable... > > > > 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. I removed those printf's since they really didn't contain any information more than the user already knew.. > > > 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. Doh, reminder, update documentation.. I'm going to let the copyout error be returned. > > 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. actually, we had a problem where if we tried to copy out, but failed, we would still increment num_matches even though the last one was bogus.. > > 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. Yep, the pattern_buf is allocated, and in some cases a berak happens w/o freeing it. So there is a memory leak her. Will be fixed soon. -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not."