On Fri, 11 Oct 2024, cel@xxxxxxxxxx wrote:
> From: Chuck Lever <chuck.lever@xxxxxxxxxx>
>
> NFSv4 LOCK operations should not avoid the set of authorization
> checks that apply to all other NFSv4 operations. Also, the
> "no_auth_nlm" export option should apply only to NLM LOCK requests.
> It's not necessary or sensible to apply it to NFSv4 LOCK operations.
>
> The replacement MAY bit mask,
> "NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE", comes from the access
> bits that are set in nfsd_permission() when the caller has set
> NFSD_MAY_LOCK.
>
> Reported-by: NeilBrown <neilb@xxxxxxx>
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
> fs/nfsd/nfs4state.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 9c2b1d251ab3..3f2c11414390 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -7967,11 +7967,10 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> if (check_lock_length(lock->lk_offset, lock->lk_length))
> return nfserr_inval;
>
> - if ((status = fh_verify(rqstp, &cstate->current_fh,
> - S_IFREG, NFSD_MAY_LOCK))) {
> - dprintk("NFSD: nfsd4_lock: permission denied!\n");
> + status = fh_verify(rqstp, &cstate->current_fh, S_IFREG,
> + NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE);
> + if (status != nfs_ok)
> return status;
> - }
Reviewed-by: NeilBrown <neilb@xxxxxxx>
though I think we want a follow-on patch which uses NFSD_MAY_WRITE for
write locks for consistency with check_fmode_for_setlk().
And I'm wondering about NFSD_MAY_OWNER_OVERRIDE ... that is really an
NFSv3 thing. For NFSv4 we should be checking permission at "open" time,
recording that in the state (both of which we do) and then performing
permission checks against the state rather than against the inode.
But that is a whole different can of worms.
Thanks,
NeilBrown
> sb = cstate->current_fh.fh_dentry->d_sb;
>
> if (lock->lk_is_new) {
> --
> 2.46.2
>
>
