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.
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.
--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
