Re: Does NFSv4 need to call inode_permission on every write???

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2343 bytes --]

On Thu, 21 May 2015 09:07:35 -0400 "J. Bruce Fields" <bfields@fieldses.org>
wrote:

> On Thu, May 21, 2015 at 02:40:29PM +1000, NeilBrown wrote:
> > 
> > 
> > Apologies if this has been answered before, however...
> > 
> > In nfsd_write() we have:
> > 
> > 	if (file) {
> > 		err = nfsd_permission(rqstp, fhp->fh_export, fhp->fh_dentry,
> > 				NFSD_MAY_WRITE|NFSD_MAY_OWNER_OVERRIDE);
> > 		if (err)
> > 			goto out;
> > 		err = nfsd_vfs_write(rqstp, fhp, file, offset, vec, vlen, cnt,
> > 				stablep);
> > 	} else {
> > 
> > So if a 'file' is already available - because the request came via NFSv4 and
> > there was a valid state id, and a 'struct file' was associated with that
> > state - we still call nfsd_permission().
> > 
> > Is that really needed?  The permission check will have been performed at open
> > - it shouldn't be needed again now.
> > 
> > With NFSv3 we have to check permission at each IO, and this is slightly
> > different from POSIX semantics.  We shouldn't have to with NFSv4... should we?
> > 
> > The particular issue that brought this to my attention is that "chattr +i" -
> > to make a file immutable - is not supposed to affect current opens, only
> > future opens.  But a current open over NFSv4 is affected.
> > 
> > Is there some reason that we cannot just remove that nfsd_permission() check?
> 
> The only proof that this write is part of the open is a stateid provided
> as part of the write arguments.  Anyone could sniff or guess that
> stateid.

But a stateid is tied to a clientid and the clientid is tied to server
credentials....??

I guess it is harder than I at first imagined.

> 
> We could try to make that work by checking the stateid against the
> principal from the rpc header.  Unfortunately that turns out to be more
> complicated than "is this the same principal as did the open"; among
> other things I think it's possible the stateid resulted from opens done
> by different principals, so we'd need to keep a list.  If we added that
> kind of check, could we drop the per-operation check?  It's not obvious
> to me.

Delegations would certainly make that interesting.  Who exactly does
authentic writes when a delegation is flushed ... I don't remember.

Sounds like this belongs in the too-hard basket.

Thanks,
NeilBrown


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]