Re: CVS commit: src/sys/nfs

[Chuck Silvers <chuq%chuq.com@localhost>]

On Sat, Jul 09, 2005 at 11:10:20PM +0900, YAMAMOTO Takashi wrote:
> hi,
> 
> > The whole structure of the code is stupid. We go from:
> > 
> > nfs_readlink -> nfs_bioread -> nfs_doio -> nfs_doio_read -> nfs_readlinkrpc
> >                |
> >                \-> nfs_readlinkrpc
> > 
> > We have the proper credentials here in the beginning of the call chain,
> > but we stupidly drop them between nfs_bioread and nfs_doio. The proper
> > fix is to pass cred down the chain instead of relying on curproc.
> 
> prior to ubc, struct buf used to hold credentials.
> so chuq might have some opinions.

the cached creds in struct buf worked fine when all cached data was cached in
bufs, but with UBC data is cached in vm_pages and I didn't want to add the
extra fields to a vm_page.  we really only need one pair of creds (read and
write) per file, since that's the granularity of access control.  so for
file data we now cache the cred pointers in the nfsnode instead of the buf.
it would make sense to do the same for any other need that NFS has for creds.


> maybe it isn't very important which credential is used here
> because the result is cached and shared between users anyway.
> 
> > It would be nice to have the notion of "who are we doing the i/o on behalf
> > of" in uio, but we don't, and yes I agree uio_procp->p_ucred is not it
> > (since in the UIO_SYSSPACE case uio_procp is NULL).
> 
> i'm not sure if it belongs to uio.
> 
> how about letting nfs_readlink() hold a credential in n_rcred as
> nfs_open() does, so that it can be used by nfs_doio_read() later?

yea, exactly.

-Chuck