On Tue, 2026-03-24 at 21:13 +1100, NeilBrown wrote:
> From: NeilBrown <[email protected]>
>
> The F_GETLK fcntl can work with either read access or write access or
> both. It can query F_RDLCK and F_WRLCK locks in either case.
>
> However lockd currently treats F_GETLK similar to F_SETLK in that read
> access is required to query an F_RDLCK lock and write access is required
> to query a F_WRLCK lock.
>
> This is wrong and can cause problem - e.g. when qemu accesses a
> read-only (e.g. iso) filesystem image over NFS (though why it queries
> if it can get a write lock - I don't know. But it does, and this works
> with local filesystems).
>
> So we need TEST requests to be handled differently. To do this:
>
> - change nlm_do_fopen() to accept O_RDWR as a mode and in that case
> succeed if either a O_RDONLY or O_WRONLY file can be opened.
> - change nlm_lookup_file() to accept a mode argument from caller,
> instead of deducing base on lock time, and pass that on to nlm_do_fopen()
> - change nlm4svc_retrieve_args() and nlmsvc_retrieve_args() to detect
> TEST requests and pass O_RDWR as a mode to nlm_lookup_file, passing
> the same mode as before for other requests. Also set
> lock->fl.c.flc_file to whichever file is available for TEST requests.
> - change nlmsvc_testlock() to also not calculate the mode, but to use
> whenever was stored in lock->fl.c.flc_file.
>
> Reported-by: Tj <[email protected]>
> Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1128861
> Fixes: 7f024fcd5c97 ("Keep read and write fds with each nlm_file")
> Signed-off-by: NeilBrown <[email protected]>
> ---
> fs/lockd/svc4proc.c | 13 ++++++++++---
> fs/lockd/svclock.c | 4 +---
> fs/lockd/svcproc.c | 15 ++++++++++++---
> fs/lockd/svcsubs.c | 26 +++++++++++++++++---------
> include/linux/lockd/lockd.h | 2 +-
> 5 files changed, 41 insertions(+), 19 deletions(-)
>
> diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
> index 4b6f18d97734..75e020a8bfd0 100644
> --- a/fs/lockd/svc4proc.c
> +++ b/fs/lockd/svc4proc.c
> @@ -26,6 +26,8 @@ nlm4svc_retrieve_args(struct svc_rqst *rqstp, struct
> nlm_args *argp,
> struct nlm_host *host = NULL;
> struct nlm_file *file = NULL;
> struct nlm_lock *lock = &argp->lock;
> + bool is_test = (rqstp->rq_proc == NLMPROC_TEST ||
> + rqstp->rq_proc == NLMPROC_TEST_MSG);
> __be32 error = 0;
>
> /* nfsd callbacks must have been installed for this procedure */
> @@ -46,15 +48,20 @@ nlm4svc_retrieve_args(struct svc_rqst *rqstp, struct
> nlm_args *argp,
> if (filp != NULL) {
> int mode = lock_to_openmode(&lock->fl);
>
> + if (is_test)
> + mode = O_RDWR;
> +
> lock->fl.c.flc_flags = FL_POSIX;
>
> - error = nlm_lookup_file(rqstp, &file, lock);
> + error = nlm_lookup_file(rqstp, &file, lock, mode);
> if (error)
> goto no_locks;
> *filp = file;
> -
> /* Set up the missing parts of the file_lock structure */
> - lock->fl.c.flc_file = file->f_file[mode];
> + if (is_test)
> + lock->fl.c.flc_file = nlmsvc_file_file(file);
> + else
> + lock->fl.c.flc_file = file->f_file[mode];
> lock->fl.c.flc_pid = current->tgid;
> lock->fl.fl_start = (loff_t)lock->lock_start;
> lock->fl.fl_end = lock->lock_len ?
> diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> index 255a847ca0b6..adfd8c072898 100644
> --- a/fs/lockd/svclock.c
> +++ b/fs/lockd/svclock.c
> @@ -614,7 +614,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file
> *file,
> struct nlm_lock *conflock)
> {
> int error;
> - int mode;
> __be32 ret;
>
> dprintk("lockd: nlmsvc_testlock(%s/%ld, ty=%d, %Ld-%Ld)\n",
> @@ -632,14 +631,13 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file
> *file,
> goto out;
> }
>
> - mode = lock_to_openmode(&lock->fl);
> locks_init_lock(&conflock->fl);
> /* vfs_test_lock only uses start, end, and owner, but tests flc_file */
> conflock->fl.c.flc_file = lock->fl.c.flc_file;
> conflock->fl.fl_start = lock->fl.fl_start;
> conflock->fl.fl_end = lock->fl.fl_end;
> conflock->fl.c.flc_owner = lock->fl.c.flc_owner;
> - error = vfs_test_lock(file->f_file[mode], &conflock->fl);
> + error = vfs_test_lock(lock->fl.c.flc_file, &conflock->fl);
> if (error) {
> ret = nlm_lck_denied_nolocks;
> goto out;
> diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c
> index 5817ef272332..d98e8d684376 100644
> --- a/fs/lockd/svcproc.c
> +++ b/fs/lockd/svcproc.c
> @@ -55,6 +55,8 @@ nlmsvc_retrieve_args(struct svc_rqst *rqstp, struct
> nlm_args *argp,
> struct nlm_host *host = NULL;
> struct nlm_file *file = NULL;
> struct nlm_lock *lock = &argp->lock;
> + bool is_test = (rqstp->rq_proc == NLMPROC_TEST ||
> + rqstp->rq_proc == NLMPROC_TEST_MSG);
> int mode;
> __be32 error = 0;
>
> @@ -70,15 +72,22 @@ nlmsvc_retrieve_args(struct svc_rqst *rqstp, struct
> nlm_args *argp,
>
> /* Obtain file pointer. Not used by FREE_ALL call. */
> if (filp != NULL) {
> - error = cast_status(nlm_lookup_file(rqstp, &file, lock));
> + mode = lock_to_openmode(&lock->fl);
> +
> + if (is_test)
> + mode = O_RDWR;
> +
> + error = cast_status(nlm_lookup_file(rqstp, &file, lock, mode));
> if (error != 0)
> goto no_locks;
> *filp = file;
>
> /* Set up the missing parts of the file_lock structure */
> - mode = lock_to_openmode(&lock->fl);
> lock->fl.c.flc_flags = FL_POSIX;
> - lock->fl.c.flc_file = file->f_file[mode];
> + if (is_test)
> + lock->fl.c.flc_file = nlmsvc_file_file(file);
> + else
> + lock->fl.c.flc_file = file->f_file[mode];
> lock->fl.c.flc_pid = current->tgid;
> lock->fl.fl_lmops = &nlmsvc_lock_operations;
> nlmsvc_locks_init_private(&lock->fl, host, (pid_t)lock->svid);
> diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
> index dd0214dcb695..b92eb032849f 100644
> --- a/fs/lockd/svcsubs.c
> +++ b/fs/lockd/svcsubs.c
> @@ -82,18 +82,28 @@ int lock_to_openmode(struct file_lock *lock)
> *
> * We have to make sure we have the right credential to open
> * the file.
> + *
> + * mode can be O_RDONLY(0), O_WRONLY(1) or O_RDWR(2) meaning either
Meaning either... ?
> */
> static __be32 nlm_do_fopen(struct svc_rqst *rqstp,
> struct nlm_file *file, int mode)
> {
> - struct file **fp = &file->f_file[mode];
> + struct file **fp;
> __be32 nfserr;
> + int m;
>
> - if (*fp)
> - return 0;
> - nfserr = nlmsvc_ops->fopen(rqstp, &file->f_handle, fp, mode);
> - if (nfserr)
> - dprintk("lockd: open failed (error %d)\n", nfserr);
> + for (m = O_RDONLY ; m <= O_WRONLY ; m++) {
> + if (mode != O_RDWR && mode != m)
> + continue;
> +
> + fp = &file->f_file[m];
> + if (*fp)
> + return 0;
> + nfserr = nlmsvc_ops->fopen(rqstp, &file->f_handle, fp, m);
> + if (!nfserr)
> + return 0;
> + }
> + dprintk("lockd: open failed (error %d)\n", nfserr);
> return nfserr;
> }
>
> @@ -103,17 +113,15 @@ static __be32 nlm_do_fopen(struct svc_rqst *rqstp,
> */
> __be32
> nlm_lookup_file(struct svc_rqst *rqstp, struct nlm_file **result,
> - struct nlm_lock *lock)
> + struct nlm_lock *lock, int mode)
> {
> struct nlm_file *file;
> unsigned int hash;
> __be32 nfserr;
> - int mode;
>
> nlm_debug_print_fh("nlm_lookup_file", &lock->fh);
>
> hash = file_hash(&lock->fh);
> - mode = lock_to_openmode(&lock->fl);
>
> /* Lock file table */
> mutex_lock(&nlm_file_mutex);
> diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
> index 330e38776bb2..fe5cdd4d66f4 100644
> --- a/include/linux/lockd/lockd.h
> +++ b/include/linux/lockd/lockd.h
> @@ -294,7 +294,7 @@ void nlmsvc_locks_init_private(struct
> file_lock *, struct nlm_host *, pid_t);
> * File handling for the server personality
> */
> __be32 nlm_lookup_file(struct svc_rqst *, struct nlm_file **,
> - struct nlm_lock *);
> + struct nlm_lock *, int);
> void nlm_release_file(struct nlm_file *);
> void nlmsvc_put_lockowner(struct nlm_lockowner *);
> void nlmsvc_release_lockowner(struct nlm_lock *);
Patch looks good though.
Reviewed-by: Jeff Layton <[email protected]>
