| Message ID | 9688295e35c07d3b3d6c71970b6996348c2d8f1e.1654798464.git.bcodding@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NLM: Defend against file_lock changes after vfs_test_lock() | expand |
> On Jun 13, 2022, at 9:40 AM, Benjamin Coddington <bcodding@redhat.com> wrote: > > Instead of trusting that struct file_lock returns completely unchanged > after vfs_test_lock() when there's no conflicting lock, stash away our > nlm_lockowner reference so we can properly release it for all cases. > > This defends against another file_lock implementation overwriting fl_owner > when the return type is F_UNLCK. > > Reported-by: Roberto Bergantinos Corpas <rbergant@redhat.com> > Tested-by: Roberto Bergantinos Corpas <rbergant@redhat.com> > Signed-off-by: Benjamin Coddington <bcodding@redhat.com> LGTM. Applied internally for more testing. Comment period still open. > --- > fs/lockd/svc4proc.c | 4 +++- > fs/lockd/svclock.c | 10 +--------- > fs/lockd/svcproc.c | 5 ++++- > include/linux/lockd/lockd.h | 1 + > 4 files changed, 9 insertions(+), 11 deletions(-) > > diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c > index 176b468a61c7..4f247ab8be61 100644 > --- a/fs/lockd/svc4proc.c > +++ b/fs/lockd/svc4proc.c > @@ -87,6 +87,7 @@ __nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) > struct nlm_args *argp = rqstp->rq_argp; > struct nlm_host *host; > struct nlm_file *file; > + struct nlm_lockowner *test_owner; > __be32 rc = rpc_success; > > dprintk("lockd: TEST4 called\n"); > @@ -96,6 +97,7 @@ __nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) > if ((resp->status = nlm4svc_retrieve_args(rqstp, argp, &host, &file))) > return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success; > > + test_owner = argp->lock.fl.fl_owner; > /* Now check for conflicting locks */ > resp->status = nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie); > if (resp->status == nlm_drop_reply) > @@ -103,7 +105,7 @@ __nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) > else > dprintk("lockd: TEST4 status %d\n", ntohl(resp->status)); > > - nlmsvc_release_lockowner(&argp->lock); > + nlmsvc_put_lockowner(test_owner); > nlmsvc_release_host(host); > nlm_release_file(file); > return rc; > diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c > index cb3658ab9b7a..9c1aa75441e1 100644 > --- a/fs/lockd/svclock.c > +++ b/fs/lockd/svclock.c > @@ -340,7 +340,7 @@ nlmsvc_get_lockowner(struct nlm_lockowner *lockowner) > return lockowner; > } > > -static void nlmsvc_put_lockowner(struct nlm_lockowner *lockowner) > +void nlmsvc_put_lockowner(struct nlm_lockowner *lockowner) > { > if (!refcount_dec_and_lock(&lockowner->count, &lockowner->host->h_lock)) > return; > @@ -590,7 +590,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file, > int error; > int mode; > __be32 ret; > - struct nlm_lockowner *test_owner; > > dprintk("lockd: nlmsvc_testlock(%s/%ld, ty=%d, %Ld-%Ld)\n", > nlmsvc_file_inode(file)->i_sb->s_id, > @@ -604,9 +603,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file, > goto out; > } > > - /* If there's a conflicting lock, remember to clean up the test lock */ > - test_owner = (struct nlm_lockowner *)lock->fl.fl_owner; > - > mode = lock_to_openmode(&lock->fl); > error = vfs_test_lock(file->f_file[mode], &lock->fl); > if (error) { > @@ -635,10 +631,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file, > conflock->fl.fl_end = lock->fl.fl_end; > locks_release_private(&lock->fl); > > - /* Clean up the test lock */ > - lock->fl.fl_owner = NULL; > - nlmsvc_put_lockowner(test_owner); > - > ret = nlm_lck_denied; > out: > return ret; > diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c > index 4dc1b40a489a..b09ca35b527c 100644 > --- a/fs/lockd/svcproc.c > +++ b/fs/lockd/svcproc.c > @@ -116,6 +116,7 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) > struct nlm_args *argp = rqstp->rq_argp; > struct nlm_host *host; > struct nlm_file *file; > + struct nlm_lockowner *test_owner; > __be32 rc = rpc_success; > > dprintk("lockd: TEST called\n"); > @@ -125,6 +126,8 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) > if ((resp->status = nlmsvc_retrieve_args(rqstp, argp, &host, &file))) > return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success; > > + test_owner = argp->lock.fl.fl_owner; > + > /* Now check for conflicting locks */ > resp->status = cast_status(nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie)); > if (resp->status == nlm_drop_reply) > @@ -133,7 +136,7 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) > dprintk("lockd: TEST status %d vers %d\n", > ntohl(resp->status), rqstp->rq_vers); > > - nlmsvc_release_lockowner(&argp->lock); > + nlmsvc_put_lockowner(test_owner); > nlmsvc_release_host(host); > nlm_release_file(file); > return rc; > diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h > index fcef192e5e45..70ce419e2709 100644 > --- a/include/linux/lockd/lockd.h > +++ b/include/linux/lockd/lockd.h > @@ -292,6 +292,7 @@ void nlmsvc_locks_init_private(struct file_lock *, struct nlm_host *, pid_t); > __be32 nlm_lookup_file(struct svc_rqst *, struct nlm_file **, > struct nlm_lock *); > void nlm_release_file(struct nlm_file *); > +void nlmsvc_put_lockowner(struct nlm_lockowner *); > void nlmsvc_release_lockowner(struct nlm_lock *); > void nlmsvc_mark_resources(struct net *); > void nlmsvc_free_host_resources(struct nlm_host *); > -- > 2.31.1 > -- Chuck Lever
On Mon, Jun 13, 2022 at 4:01 PM Chuck Lever III <chuck.lever@oracle.com> wrote: > > > > > On Jun 13, 2022, at 9:40 AM, Benjamin Coddington <bcodding@redhat.com> wrote: > > > > Instead of trusting that struct file_lock returns completely unchanged > > after vfs_test_lock() when there's no conflicting lock, stash away our > > nlm_lockowner reference so we can properly release it for all cases. > > > > This defends against another file_lock implementation overwriting fl_owner > > when the return type is F_UNLCK. > > > > Reported-by: Roberto Bergantinos Corpas <rbergant@redhat.com> > > Tested-by: Roberto Bergantinos Corpas <rbergant@redhat.com> > > Signed-off-by: Benjamin Coddington <bcodding@redhat.com> > > LGTM. Applied internally for more testing. Comment period > still open. > Chuck, Are you planning to take this in the next merge window, or what is the status of this patch? > > > --- > > fs/lockd/svc4proc.c | 4 +++- > > fs/lockd/svclock.c | 10 +--------- > > fs/lockd/svcproc.c | 5 ++++- > > include/linux/lockd/lockd.h | 1 + > > 4 files changed, 9 insertions(+), 11 deletions(-) > > > > diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c > > index 176b468a61c7..4f247ab8be61 100644 > > --- a/fs/lockd/svc4proc.c > > +++ b/fs/lockd/svc4proc.c > > @@ -87,6 +87,7 @@ __nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) > > struct nlm_args *argp = rqstp->rq_argp; > > struct nlm_host *host; > > struct nlm_file *file; > > + struct nlm_lockowner *test_owner; > > __be32 rc = rpc_success; > > > > dprintk("lockd: TEST4 called\n"); > > @@ -96,6 +97,7 @@ __nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) > > if ((resp->status = nlm4svc_retrieve_args(rqstp, argp, &host, &file))) > > return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success; > > > > + test_owner = argp->lock.fl.fl_owner; > > /* Now check for conflicting locks */ > > resp->status = nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie); > > if (resp->status == nlm_drop_reply) > > @@ -103,7 +105,7 @@ __nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) > > else > > dprintk("lockd: TEST4 status %d\n", ntohl(resp->status)); > > > > - nlmsvc_release_lockowner(&argp->lock); > > + nlmsvc_put_lockowner(test_owner); > > nlmsvc_release_host(host); > > nlm_release_file(file); > > return rc; > > diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c > > index cb3658ab9b7a..9c1aa75441e1 100644 > > --- a/fs/lockd/svclock.c > > +++ b/fs/lockd/svclock.c > > @@ -340,7 +340,7 @@ nlmsvc_get_lockowner(struct nlm_lockowner *lockowner) > > return lockowner; > > } > > > > -static void nlmsvc_put_lockowner(struct nlm_lockowner *lockowner) > > +void nlmsvc_put_lockowner(struct nlm_lockowner *lockowner) > > { > > if (!refcount_dec_and_lock(&lockowner->count, &lockowner->host->h_lock)) > > return; > > @@ -590,7 +590,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file, > > int error; > > int mode; > > __be32 ret; > > - struct nlm_lockowner *test_owner; > > > > dprintk("lockd: nlmsvc_testlock(%s/%ld, ty=%d, %Ld-%Ld)\n", > > nlmsvc_file_inode(file)->i_sb->s_id, > > @@ -604,9 +603,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file, > > goto out; > > } > > > > - /* If there's a conflicting lock, remember to clean up the test lock */ > > - test_owner = (struct nlm_lockowner *)lock->fl.fl_owner; > > - > > mode = lock_to_openmode(&lock->fl); > > error = vfs_test_lock(file->f_file[mode], &lock->fl); > > if (error) { > > @@ -635,10 +631,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file, > > conflock->fl.fl_end = lock->fl.fl_end; > > locks_release_private(&lock->fl); > > > > - /* Clean up the test lock */ > > - lock->fl.fl_owner = NULL; > > - nlmsvc_put_lockowner(test_owner); > > - > > ret = nlm_lck_denied; > > out: > > return ret; > > diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c > > index 4dc1b40a489a..b09ca35b527c 100644 > > --- a/fs/lockd/svcproc.c > > +++ b/fs/lockd/svcproc.c > > @@ -116,6 +116,7 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) > > struct nlm_args *argp = rqstp->rq_argp; > > struct nlm_host *host; > > struct nlm_file *file; > > + struct nlm_lockowner *test_owner; > > __be32 rc = rpc_success; > > > > dprintk("lockd: TEST called\n"); > > @@ -125,6 +126,8 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) > > if ((resp->status = nlmsvc_retrieve_args(rqstp, argp, &host, &file))) > > return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success; > > > > + test_owner = argp->lock.fl.fl_owner; > > + > > /* Now check for conflicting locks */ > > resp->status = cast_status(nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie)); > > if (resp->status == nlm_drop_reply) > > @@ -133,7 +136,7 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) > > dprintk("lockd: TEST status %d vers %d\n", > > ntohl(resp->status), rqstp->rq_vers); > > > > - nlmsvc_release_lockowner(&argp->lock); > > + nlmsvc_put_lockowner(test_owner); > > nlmsvc_release_host(host); > > nlm_release_file(file); > > return rc; > > diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h > > index fcef192e5e45..70ce419e2709 100644 > > --- a/include/linux/lockd/lockd.h > > +++ b/include/linux/lockd/lockd.h > > @@ -292,6 +292,7 @@ void nlmsvc_locks_init_private(struct file_lock *, struct nlm_host *, pid_t); > > __be32 nlm_lookup_file(struct svc_rqst *, struct nlm_file **, > > struct nlm_lock *); > > void nlm_release_file(struct nlm_file *); > > +void nlmsvc_put_lockowner(struct nlm_lockowner *); > > void nlmsvc_release_lockowner(struct nlm_lock *); > > void nlmsvc_mark_resources(struct net *); > > void nlmsvc_free_host_resources(struct nlm_host *); > > -- > > 2.31.1 > > > > -- > Chuck Lever > > >
> On Jun 27, 2022, at 8:48 AM, David Wysochanski <dwysocha@redhat.com> wrote: > > On Mon, Jun 13, 2022 at 4:01 PM Chuck Lever III <chuck.lever@oracle.com> wrote: >> >> >> >>> On Jun 13, 2022, at 9:40 AM, Benjamin Coddington <bcodding@redhat.com> wrote: >>> >>> Instead of trusting that struct file_lock returns completely unchanged >>> after vfs_test_lock() when there's no conflicting lock, stash away our >>> nlm_lockowner reference so we can properly release it for all cases. >>> >>> This defends against another file_lock implementation overwriting fl_owner >>> when the return type is F_UNLCK. >>> >>> Reported-by: Roberto Bergantinos Corpas <rbergant@redhat.com> >>> Tested-by: Roberto Bergantinos Corpas <rbergant@redhat.com> >>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com> >> >> LGTM. Applied internally for more testing. Comment period >> still open. >> > > Chuck, > > Are you planning to take this in the next merge window, or what is the > status of this patch? It is destined for the next merge window. It will appear in the for-next branch here: https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git In a week or two. >>> --- >>> fs/lockd/svc4proc.c | 4 +++- >>> fs/lockd/svclock.c | 10 +--------- >>> fs/lockd/svcproc.c | 5 ++++- >>> include/linux/lockd/lockd.h | 1 + >>> 4 files changed, 9 insertions(+), 11 deletions(-) >>> >>> diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c >>> index 176b468a61c7..4f247ab8be61 100644 >>> --- a/fs/lockd/svc4proc.c >>> +++ b/fs/lockd/svc4proc.c >>> @@ -87,6 +87,7 @@ __nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) >>> struct nlm_args *argp = rqstp->rq_argp; >>> struct nlm_host *host; >>> struct nlm_file *file; >>> + struct nlm_lockowner *test_owner; >>> __be32 rc = rpc_success; >>> >>> dprintk("lockd: TEST4 called\n"); >>> @@ -96,6 +97,7 @@ __nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) >>> if ((resp->status = nlm4svc_retrieve_args(rqstp, argp, &host, &file))) >>> return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success; >>> >>> + test_owner = argp->lock.fl.fl_owner; >>> /* Now check for conflicting locks */ >>> resp->status = nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie); >>> if (resp->status == nlm_drop_reply) >>> @@ -103,7 +105,7 @@ __nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) >>> else >>> dprintk("lockd: TEST4 status %d\n", ntohl(resp->status)); >>> >>> - nlmsvc_release_lockowner(&argp->lock); >>> + nlmsvc_put_lockowner(test_owner); >>> nlmsvc_release_host(host); >>> nlm_release_file(file); >>> return rc; >>> diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c >>> index cb3658ab9b7a..9c1aa75441e1 100644 >>> --- a/fs/lockd/svclock.c >>> +++ b/fs/lockd/svclock.c >>> @@ -340,7 +340,7 @@ nlmsvc_get_lockowner(struct nlm_lockowner *lockowner) >>> return lockowner; >>> } >>> >>> -static void nlmsvc_put_lockowner(struct nlm_lockowner *lockowner) >>> +void nlmsvc_put_lockowner(struct nlm_lockowner *lockowner) >>> { >>> if (!refcount_dec_and_lock(&lockowner->count, &lockowner->host->h_lock)) >>> return; >>> @@ -590,7 +590,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file, >>> int error; >>> int mode; >>> __be32 ret; >>> - struct nlm_lockowner *test_owner; >>> >>> dprintk("lockd: nlmsvc_testlock(%s/%ld, ty=%d, %Ld-%Ld)\n", >>> nlmsvc_file_inode(file)->i_sb->s_id, >>> @@ -604,9 +603,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file, >>> goto out; >>> } >>> >>> - /* If there's a conflicting lock, remember to clean up the test lock */ >>> - test_owner = (struct nlm_lockowner *)lock->fl.fl_owner; >>> - >>> mode = lock_to_openmode(&lock->fl); >>> error = vfs_test_lock(file->f_file[mode], &lock->fl); >>> if (error) { >>> @@ -635,10 +631,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file, >>> conflock->fl.fl_end = lock->fl.fl_end; >>> locks_release_private(&lock->fl); >>> >>> - /* Clean up the test lock */ >>> - lock->fl.fl_owner = NULL; >>> - nlmsvc_put_lockowner(test_owner); >>> - >>> ret = nlm_lck_denied; >>> out: >>> return ret; >>> diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c >>> index 4dc1b40a489a..b09ca35b527c 100644 >>> --- a/fs/lockd/svcproc.c >>> +++ b/fs/lockd/svcproc.c >>> @@ -116,6 +116,7 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) >>> struct nlm_args *argp = rqstp->rq_argp; >>> struct nlm_host *host; >>> struct nlm_file *file; >>> + struct nlm_lockowner *test_owner; >>> __be32 rc = rpc_success; >>> >>> dprintk("lockd: TEST called\n"); >>> @@ -125,6 +126,8 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) >>> if ((resp->status = nlmsvc_retrieve_args(rqstp, argp, &host, &file))) >>> return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success; >>> >>> + test_owner = argp->lock.fl.fl_owner; >>> + >>> /* Now check for conflicting locks */ >>> resp->status = cast_status(nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie)); >>> if (resp->status == nlm_drop_reply) >>> @@ -133,7 +136,7 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) >>> dprintk("lockd: TEST status %d vers %d\n", >>> ntohl(resp->status), rqstp->rq_vers); >>> >>> - nlmsvc_release_lockowner(&argp->lock); >>> + nlmsvc_put_lockowner(test_owner); >>> nlmsvc_release_host(host); >>> nlm_release_file(file); >>> return rc; >>> diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h >>> index fcef192e5e45..70ce419e2709 100644 >>> --- a/include/linux/lockd/lockd.h >>> +++ b/include/linux/lockd/lockd.h >>> @@ -292,6 +292,7 @@ void nlmsvc_locks_init_private(struct file_lock *, struct nlm_host *, pid_t); >>> __be32 nlm_lookup_file(struct svc_rqst *, struct nlm_file **, >>> struct nlm_lock *); >>> void nlm_release_file(struct nlm_file *); >>> +void nlmsvc_put_lockowner(struct nlm_lockowner *); >>> void nlmsvc_release_lockowner(struct nlm_lock *); >>> void nlmsvc_mark_resources(struct net *); >>> void nlmsvc_free_host_resources(struct nlm_host *); >>> -- >>> 2.31.1 >>> >> >> -- >> Chuck Lever -- Chuck Lever
diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c index 176b468a61c7..4f247ab8be61 100644 --- a/fs/lockd/svc4proc.c +++ b/fs/lockd/svc4proc.c @@ -87,6 +87,7 @@ __nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) struct nlm_args *argp = rqstp->rq_argp; struct nlm_host *host; struct nlm_file *file; + struct nlm_lockowner *test_owner; __be32 rc = rpc_success; dprintk("lockd: TEST4 called\n"); @@ -96,6 +97,7 @@ __nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) if ((resp->status = nlm4svc_retrieve_args(rqstp, argp, &host, &file))) return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success; + test_owner = argp->lock.fl.fl_owner; /* Now check for conflicting locks */ resp->status = nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie); if (resp->status == nlm_drop_reply) @@ -103,7 +105,7 @@ __nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) else dprintk("lockd: TEST4 status %d\n", ntohl(resp->status)); - nlmsvc_release_lockowner(&argp->lock); + nlmsvc_put_lockowner(test_owner); nlmsvc_release_host(host); nlm_release_file(file); return rc; diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c index cb3658ab9b7a..9c1aa75441e1 100644 --- a/fs/lockd/svclock.c +++ b/fs/lockd/svclock.c @@ -340,7 +340,7 @@ nlmsvc_get_lockowner(struct nlm_lockowner *lockowner) return lockowner; } -static void nlmsvc_put_lockowner(struct nlm_lockowner *lockowner) +void nlmsvc_put_lockowner(struct nlm_lockowner *lockowner) { if (!refcount_dec_and_lock(&lockowner->count, &lockowner->host->h_lock)) return; @@ -590,7 +590,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file, int error; int mode; __be32 ret; - struct nlm_lockowner *test_owner; dprintk("lockd: nlmsvc_testlock(%s/%ld, ty=%d, %Ld-%Ld)\n", nlmsvc_file_inode(file)->i_sb->s_id, @@ -604,9 +603,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file, goto out; } - /* If there's a conflicting lock, remember to clean up the test lock */ - test_owner = (struct nlm_lockowner *)lock->fl.fl_owner; - mode = lock_to_openmode(&lock->fl); error = vfs_test_lock(file->f_file[mode], &lock->fl); if (error) { @@ -635,10 +631,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file, conflock->fl.fl_end = lock->fl.fl_end; locks_release_private(&lock->fl); - /* Clean up the test lock */ - lock->fl.fl_owner = NULL; - nlmsvc_put_lockowner(test_owner); - ret = nlm_lck_denied; out: return ret; diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c index 4dc1b40a489a..b09ca35b527c 100644 --- a/fs/lockd/svcproc.c +++ b/fs/lockd/svcproc.c @@ -116,6 +116,7 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) struct nlm_args *argp = rqstp->rq_argp; struct nlm_host *host; struct nlm_file *file; + struct nlm_lockowner *test_owner; __be32 rc = rpc_success; dprintk("lockd: TEST called\n"); @@ -125,6 +126,8 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) if ((resp->status = nlmsvc_retrieve_args(rqstp, argp, &host, &file))) return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success; + test_owner = argp->lock.fl.fl_owner; + /* Now check for conflicting locks */ resp->status = cast_status(nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie)); if (resp->status == nlm_drop_reply) @@ -133,7 +136,7 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp) dprintk("lockd: TEST status %d vers %d\n", ntohl(resp->status), rqstp->rq_vers); - nlmsvc_release_lockowner(&argp->lock); + nlmsvc_put_lockowner(test_owner); nlmsvc_release_host(host); nlm_release_file(file); return rc; diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h index fcef192e5e45..70ce419e2709 100644 --- a/include/linux/lockd/lockd.h +++ b/include/linux/lockd/lockd.h @@ -292,6 +292,7 @@ void nlmsvc_locks_init_private(struct file_lock *, struct nlm_host *, pid_t); __be32 nlm_lookup_file(struct svc_rqst *, struct nlm_file **, struct nlm_lock *); void nlm_release_file(struct nlm_file *); +void nlmsvc_put_lockowner(struct nlm_lockowner *); void nlmsvc_release_lockowner(struct nlm_lock *); void nlmsvc_mark_resources(struct net *); void nlmsvc_free_host_resources(struct nlm_host *);