Skip to content

Commit 4acb4df

Browse files
committed
NLM: Defend against file_lock changes after vfs_test_lock()
jira LE-1907 Rebuild_History Non-Buildable kernel-rt-5.14.0-284.30.1.rt14.315.el9_2 commit-author Benjamin Coddington <[email protected]> commit 184cefb 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 <[email protected]> Tested-by: Roberto Bergantinos Corpas <[email protected]> Signed-off-by: Benjamin Coddington <[email protected]> Signed-off-by: Chuck Lever <[email protected]> (cherry picked from commit 184cefb) Signed-off-by: Jonathan Maple <[email protected]>
1 parent ec08e43 commit 4acb4df

File tree

4 files changed

+9
-11
lines changed

4 files changed

+9
-11
lines changed

fs/lockd/svc4proc.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ __nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp)
8787
struct nlm_args *argp = rqstp->rq_argp;
8888
struct nlm_host *host;
8989
struct nlm_file *file;
90+
struct nlm_lockowner *test_owner;
9091
__be32 rc = rpc_success;
9192

9293
dprintk("lockd: TEST4 called\n");
@@ -96,14 +97,15 @@ __nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp)
9697
if ((resp->status = nlm4svc_retrieve_args(rqstp, argp, &host, &file)))
9798
return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success;
9899

100+
test_owner = argp->lock.fl.fl_owner;
99101
/* Now check for conflicting locks */
100102
resp->status = nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie);
101103
if (resp->status == nlm_drop_reply)
102104
rc = rpc_drop_reply;
103105
else
104106
dprintk("lockd: TEST4 status %d\n", ntohl(resp->status));
105107

106-
nlmsvc_release_lockowner(&argp->lock);
108+
nlmsvc_put_lockowner(test_owner);
107109
nlmsvc_release_host(host);
108110
nlm_release_file(file);
109111
return rc;

fs/lockd/svclock.c

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ nlmsvc_get_lockowner(struct nlm_lockowner *lockowner)
340340
return lockowner;
341341
}
342342

343-
static void nlmsvc_put_lockowner(struct nlm_lockowner *lockowner)
343+
void nlmsvc_put_lockowner(struct nlm_lockowner *lockowner)
344344
{
345345
if (!refcount_dec_and_lock(&lockowner->count, &lockowner->host->h_lock))
346346
return;
@@ -590,7 +590,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
590590
int error;
591591
int mode;
592592
__be32 ret;
593-
struct nlm_lockowner *test_owner;
594593

595594
dprintk("lockd: nlmsvc_testlock(%s/%ld, ty=%d, %Ld-%Ld)\n",
596595
nlmsvc_file_inode(file)->i_sb->s_id,
@@ -604,9 +603,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
604603
goto out;
605604
}
606605

607-
/* If there's a conflicting lock, remember to clean up the test lock */
608-
test_owner = (struct nlm_lockowner *)lock->fl.fl_owner;
609-
610606
mode = lock_to_openmode(&lock->fl);
611607
error = vfs_test_lock(file->f_file[mode], &lock->fl);
612608
if (error) {
@@ -635,10 +631,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
635631
conflock->fl.fl_end = lock->fl.fl_end;
636632
locks_release_private(&lock->fl);
637633

638-
/* Clean up the test lock */
639-
lock->fl.fl_owner = NULL;
640-
nlmsvc_put_lockowner(test_owner);
641-
642634
ret = nlm_lck_denied;
643635
out:
644636
return ret;

fs/lockd/svcproc.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp)
116116
struct nlm_args *argp = rqstp->rq_argp;
117117
struct nlm_host *host;
118118
struct nlm_file *file;
119+
struct nlm_lockowner *test_owner;
119120
__be32 rc = rpc_success;
120121

121122
dprintk("lockd: TEST called\n");
@@ -125,6 +126,8 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp)
125126
if ((resp->status = nlmsvc_retrieve_args(rqstp, argp, &host, &file)))
126127
return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success;
127128

129+
test_owner = argp->lock.fl.fl_owner;
130+
128131
/* Now check for conflicting locks */
129132
resp->status = cast_status(nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie));
130133
if (resp->status == nlm_drop_reply)
@@ -133,7 +136,7 @@ __nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_res *resp)
133136
dprintk("lockd: TEST status %d vers %d\n",
134137
ntohl(resp->status), rqstp->rq_vers);
135138

136-
nlmsvc_release_lockowner(&argp->lock);
139+
nlmsvc_put_lockowner(test_owner);
137140
nlmsvc_release_host(host);
138141
nlm_release_file(file);
139142
return rc;

include/linux/lockd/lockd.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ void nlmsvc_locks_init_private(struct file_lock *, struct nlm_host *, pid_t);
292292
__be32 nlm_lookup_file(struct svc_rqst *, struct nlm_file **,
293293
struct nlm_lock *);
294294
void nlm_release_file(struct nlm_file *);
295+
void nlmsvc_put_lockowner(struct nlm_lockowner *);
295296
void nlmsvc_release_lockowner(struct nlm_lock *);
296297
void nlmsvc_mark_resources(struct net *);
297298
void nlmsvc_free_host_resources(struct nlm_host *);

0 commit comments

Comments
 (0)