make /dev/pci really readable

[John-Mark Gurney]

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."