Commit 184cefbe authored by Benjamin Coddington's avatar Benjamin Coddington Committed by Chuck Lever
Browse files

NLM: Defend against file_lock changes after vfs_test_lock() 



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: default avatarRoberto Bergantinos Corpas <rbergant@redhat.com>
Tested-by: default avatarRoberto Bergantinos Corpas <rbergant@redhat.com>
Signed-off-by: default avatarBenjamin Coddington <bcodding@redhat.com>
Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
parent c770f31d

fs/lockd/svc4proc.c

+3 −1
Original line number Diff line number Diff line
@@ -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;

fs/lockd/svclock.c

+1 −9
Original line number Diff line number Diff line
@@ -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;

fs/lockd/svcproc.c

+4 −1
Original line number Diff line number Diff line
@@ -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;

include/linux/lockd/lockd.h

+1 −0
Original line number Diff line number Diff line
@@ -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 *);