On Tue, Mar 14, 2023 at 12:21 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> Commit 6930bcbfb6ce dropped the setting of the file_lock range when
> decoding a nlm_lock off the wire. This causes the client side grant
> callback to miss matching blocks and reject the lock, only to rerequest
> it 30s later.
>
> Add a helper function to set the file_lock range from the start and end
> values that the protocol uses, and have the nlm_lock decoder call that to
> set up the file_lock args properly.
>
> Fixes: 6930bcbfb6ce ("lockd: detect and reject lock arguments that overflow")
> Reported-by: Amir Goldstein <amir73il@xxxxxxxxx>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/lockd/clnt4xdr.c | 9 +--------
> fs/lockd/xdr4.c | 13 ++++++++++++-
> include/linux/lockd/xdr4.h | 1 +
> 3 files changed, 14 insertions(+), 9 deletions(-)
>
> Hi Amir,
>
> Thanks for the bug report. This patch seems to fix up the 30s stalls for
> me. Can you give it a spin and see if it also helps you? I am still
Works like a charm :)
I tested a backport to 5.15.y (trivial conflict in h file).
against 5.10.109 and 5.15.88 servers.
Tested-by: Amir Goldstein <amir73il@xxxxxxxxx>
> seeing one test failure with nfstest_lock, but I'm not sure it's related
> to this. I'll plan to follow up with Jorge.
I don't see this failure in my tests.
Please fast track this fix which also fixes an LTS 5.15.y kernel regression.
Please cc stable and let me know if you want me to post the 5.15.y backport.
Thanks for the prompt fix,
Amir.
>
> Thanks,
> Jeff
>
> diff --git a/fs/lockd/clnt4xdr.c b/fs/lockd/clnt4xdr.c
> index 7df6324ccb8a..8161667c976f 100644
> --- a/fs/lockd/clnt4xdr.c
> +++ b/fs/lockd/clnt4xdr.c
> @@ -261,7 +261,6 @@ static int decode_nlm4_holder(struct xdr_stream *xdr, struct nlm_res *result)
> u32 exclusive;
> int error;
> __be32 *p;
> - s32 end;
>
> memset(lock, 0, sizeof(*lock));
> locks_init_lock(fl);
> @@ -285,13 +284,7 @@ static int decode_nlm4_holder(struct xdr_stream *xdr, struct nlm_res *result)
> fl->fl_type = exclusive != 0 ? F_WRLCK : F_RDLCK;
> p = xdr_decode_hyper(p, &l_offset);
> xdr_decode_hyper(p, &l_len);
> - end = l_offset + l_len - 1;
> -
> - fl->fl_start = (loff_t)l_offset;
> - if (l_len == 0 || end < 0)
> - fl->fl_end = OFFSET_MAX;
> - else
> - fl->fl_end = (loff_t)end;
> + nlm4svc_set_file_lock_range(fl, l_offset, l_len);
> error = 0;
> out:
> return error;
> diff --git a/fs/lockd/xdr4.c b/fs/lockd/xdr4.c
> index 712fdfeb8ef0..5fcbf30cd275 100644
> --- a/fs/lockd/xdr4.c
> +++ b/fs/lockd/xdr4.c
> @@ -33,6 +33,17 @@ loff_t_to_s64(loff_t offset)
> return res;
> }
>
> +void nlm4svc_set_file_lock_range(struct file_lock *fl, u64 off, u64 len)
> +{
> + s64 end = off + len - 1;
> +
> + fl->fl_start = off;
> + if (len == 0 || end < 0)
> + fl->fl_end = OFFSET_MAX;
> + else
> + fl->fl_end = end;
> +}
> +
> /*
> * NLM file handles are defined by specification to be a variable-length
> * XDR opaque no longer than 1024 bytes. However, this implementation
> @@ -80,7 +91,7 @@ svcxdr_decode_lock(struct xdr_stream *xdr, struct nlm_lock *lock)
> locks_init_lock(fl);
> fl->fl_flags = FL_POSIX;
> fl->fl_type = F_RDLCK;
> -
> + nlm4svc_set_file_lock_range(fl, lock->lock_start, lock->lock_len);
> return true;
> }
>
> diff --git a/include/linux/lockd/xdr4.h b/include/linux/lockd/xdr4.h
> index 9a6b55da8fd6..72831e35dca3 100644
> --- a/include/linux/lockd/xdr4.h
> +++ b/include/linux/lockd/xdr4.h
> @@ -22,6 +22,7 @@
> #define nlm4_fbig cpu_to_be32(NLM_FBIG)
> #define nlm4_failed cpu_to_be32(NLM_FAILED)
>
> +void nlm4svc_set_file_lock_range(struct file_lock *fl, u64 off, u64 len);
> bool nlm4svc_decode_void(struct svc_rqst *rqstp, struct xdr_stream *xdr);
> bool nlm4svc_decode_testargs(struct svc_rqst *rqstp, struct xdr_stream *xdr);
> bool nlm4svc_decode_lockargs(struct svc_rqst *rqstp, struct xdr_stream *xdr);
> --
> 2.39.2
>
