2022-11-11 19:38:08

by Jeff Layton

Subject: [PATCH 3/4] lockd: fix file selection in nlmsvc_cancel_blocked

We currently do a lock_to_openmode call based on the arguments from the
NLM_UNLOCK call, but that will always set the fl_type of the lock to
F_UNLCK, the the O_RDONLY descriptor is always chosen.

Fix it to use the file_lock from the block instead.

Signed-off-by: Jeff Layton <[email protected]>
---
fs/lockd/svclock.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index 9eae99e08e69..4e30f3c50970 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -699,9 +699,10 @@ nlmsvc_cancel_blocked(struct net *net, struct nlm_file *file, struct nlm_lock *l
block = nlmsvc_lookup_block(file, lock);
mutex_unlock(&file->f_mutex);
if (block != NULL) {
- mode = lock_to_openmode(&lock->fl);
- vfs_cancel_lock(block->b_file->f_file[mode],
- &block->b_call->a_args.lock.fl);
+ struct file_lock *fl = &block->b_call->a_args.lock.fl;
+
+ mode = lock_to_openmode(fl);
+ vfs_cancel_lock(block->b_file->f_file[mode], fl);
status = nlmsvc_unlink_block(block);
nlmsvc_release_block(block);
}
--
2.38.1

2022-11-11 20:48:47

by Chuck Lever III

Subject: Re: [PATCH 3/4] lockd: fix file selection in nlmsvc_cancel_blocked

> On Nov 11, 2022, at 2:36 PM, Jeff Layton <[email protected]> wrote:
>
> We currently do a lock_to_openmode call based on the arguments from the
> NLM_UNLOCK call, but that will always set the fl_type of the lock to
> F_UNLCK, the the O_RDONLY descriptor is always chosen.

Except for the above sentence, these all look sane to me.
I can apply them to nfsd's for-next once they've seen some
review on fsdevel, as you mentioned in the other thread.


> Fix it to use the file_lock from the block instead.
>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/lockd/svclock.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> index 9eae99e08e69..4e30f3c50970 100644
> --- a/fs/lockd/svclock.c
> +++ b/fs/lockd/svclock.c
> @@ -699,9 +699,10 @@ nlmsvc_cancel_blocked(struct net *net, struct nlm_file *file, struct nlm_lock *l
> block = nlmsvc_lookup_block(file, lock);
> mutex_unlock(&file->f_mutex);
> if (block != NULL) {
> - mode = lock_to_openmode(&lock->fl);
> - vfs_cancel_lock(block->b_file->f_file[mode],
> - &block->b_call->a_args.lock.fl);
> + struct file_lock *fl = &block->b_call->a_args.lock.fl;
> +
> + mode = lock_to_openmode(fl);
> + vfs_cancel_lock(block->b_file->f_file[mode], fl);
> status = nlmsvc_unlink_block(block);
> nlmsvc_release_block(block);
> }
> --
> 2.38.1
>

--
Chuck Lever

2022-11-11 21:53:49

by Jeff Layton

Subject: Re: [PATCH 3/4] lockd: fix file selection in nlmsvc_cancel_blocked

On Fri, 2022-11-11 at 20:29 +0000, Chuck Lever III wrote:
>
> > On Nov 11, 2022, at 2:36 PM, Jeff Layton <[email protected]> wrote:
> >
> > We currently do a lock_to_openmode call based on the arguments from the
> > NLM_UNLOCK call, but that will always set the fl_type of the lock to
> > F_UNLCK, the the O_RDONLY descriptor is always chosen.
>
> Except for the above sentence, these all look sane to me.
> I can apply them to nfsd's for-next once they've seen some
> review on fsdevel, as you mentioned in the other thread.
>
>

Thanks. That should say "and the O_RDONLY...". Fixed in my tree.

I'll go ahead and resend with fsdevel included.

> > Fix it to use the file_lock from the block instead.
> >
> > Signed-off-by: Jeff Layton <[email protected]>
> > ---
> > fs/lockd/svclock.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> > index 9eae99e08e69..4e30f3c50970 100644
> > --- a/fs/lockd/svclock.c
> > +++ b/fs/lockd/svclock.c
> > @@ -699,9 +699,10 @@ nlmsvc_cancel_blocked(struct net *net, struct nlm_file *file, struct nlm_lock *l
> > block = nlmsvc_lookup_block(file, lock);
> > mutex_unlock(&file->f_mutex);
> > if (block != NULL) {
> > - mode = lock_to_openmode(&lock->fl);
> > - vfs_cancel_lock(block->b_file->f_file[mode],
> > - &block->b_call->a_args.lock.fl);
> > + struct file_lock *fl = &block->b_call->a_args.lock.fl;
> > +
> > + mode = lock_to_openmode(fl);
> > + vfs_cancel_lock(block->b_file->f_file[mode], fl);
> > status = nlmsvc_unlink_block(block);
> > nlmsvc_release_block(block);
> > }
> > --
> > 2.38.1
> >
>
> --
> Chuck Lever
>
>
>

--
Jeff Layton <[email protected]>

2022-11-14 18:42:06

by Jeff Layton

Subject: Re: [PATCH 3/4] lockd: fix file selection in nlmsvc_cancel_blocked

On Fri, 2022-11-11 at 16:52 -0500, Jeff Layton wrote:
> On Fri, 2022-11-11 at 20:29 +0000, Chuck Lever III wrote:
> >
> > > On Nov 11, 2022, at 2:36 PM, Jeff Layton <[email protected]> wrote:
> > >
> > > We currently do a lock_to_openmode call based on the arguments from the
> > > NLM_UNLOCK call, but that will always set the fl_type of the lock to
> > > F_UNLCK, the the O_RDONLY descriptor is always chosen.
> >
> > Except for the above sentence, these all look sane to me.
> > I can apply them to nfsd's for-next once they've seen some
> > review on fsdevel, as you mentioned in the other thread.
> >
> >
>
> Thanks. That should say "and the O_RDONLY...". Fixed in my tree.
>
> I'll go ahead and resend with fsdevel included.
>

I reposted the series Friday afternoon.

What might be best is for you to carry the first 3 patches in the nfsd
tree, and I'll take the filelock: patch into the locks-next branch,
along with the other filelock API cleanups.

Sound OK?

> > > Fix it to use the file_lock from the block instead.
> > >
> > > Signed-off-by: Jeff Layton <[email protected]>
> > > ---
> > > fs/lockd/svclock.c | 7 ++++---
> > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> > > index 9eae99e08e69..4e30f3c50970 100644
> > > --- a/fs/lockd/svclock.c
> > > +++ b/fs/lockd/svclock.c
> > > @@ -699,9 +699,10 @@ nlmsvc_cancel_blocked(struct net *net, struct nlm_file *file, struct nlm_lock *l
> > > block = nlmsvc_lookup_block(file, lock);
> > > mutex_unlock(&file->f_mutex);
> > > if (block != NULL) {
> > > - mode = lock_to_openmode(&lock->fl);
> > > - vfs_cancel_lock(block->b_file->f_file[mode],
> > > - &block->b_call->a_args.lock.fl);
> > > + struct file_lock *fl = &block->b_call->a_args.lock.fl;
> > > +
> > > + mode = lock_to_openmode(fl);
> > > + vfs_cancel_lock(block->b_file->f_file[mode], fl);
> > > status = nlmsvc_unlink_block(block);
> > > nlmsvc_release_block(block);
> > > }
> > > --
> > > 2.38.1
> > >
> >
> > --
> > Chuck Lever
> >
> >
> >
>

--
Jeff Layton <[email protected]>

2022-11-14 18:54:57

by Chuck Lever III

Subject: Re: [PATCH 3/4] lockd: fix file selection in nlmsvc_cancel_blocked

> On Nov 14, 2022, at 1:38 PM, Jeff Layton <[email protected]> wrote:
>
> On Fri, 2022-11-11 at 16:52 -0500, Jeff Layton wrote:
>> On Fri, 2022-11-11 at 20:29 +0000, Chuck Lever III wrote:
>>>
>>>> On Nov 11, 2022, at 2:36 PM, Jeff Layton <[email protected]> wrote:
>>>>
>>>> We currently do a lock_to_openmode call based on the arguments from the
>>>> NLM_UNLOCK call, but that will always set the fl_type of the lock to
>>>> F_UNLCK, the the O_RDONLY descriptor is always chosen.
>>>
>>> Except for the above sentence, these all look sane to me.
>>> I can apply them to nfsd's for-next once they've seen some
>>> review on fsdevel, as you mentioned in the other thread.
>>>
>>>
>>
>> Thanks. That should say "and the O_RDONLY...". Fixed in my tree.
>>
>> I'll go ahead and resend with fsdevel included.
>>
>
> I reposted the series Friday afternoon.
>
> What might be best is for you to carry the first 3 patches in the nfsd
> tree, and I'll take the filelock: patch into the locks-next branch,
> along with the other filelock API cleanups.
>
> Sound OK?

1/4 through 3/4 have been applied and pushed. Thanks!

--
Chuck Lever