[PATCH 1/2] lockd: fix NULL pointer dereference in nlmclnt_locks_release_private

Ran Hongyun <[email protected]>
Newsgroups org.kernel.vger.linux-nfs,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
nlmclnt_locks_init_private() unconditionally sets fl->fl_ops even when
nlmclnt_find_lockowner() returns NULL due to allocation failure. When
locks_release_private() later sees a non-NULL fl_ops, it calls
fl_release_private, which dereferences fl->fl_u.nfs_fl.owner.

nlmclnt_proc()
	nlmclnt_locks_init_private <----set fl->fl_ops unconditionally
		if (!fl->fl_u.nfs_fl.owner) <----forget to clear fl->fl_ops

locks_release_private()
	if (fl->fl_ops) <----fl->fl_ops is not NULL but owner is NULL
		fl->fl_ops->fl_release_private()
			nlmclnt_locks_release_private() <----NULL ptr dereference

Fix it by inlining nlmclnt_locks_init_private() into its single
caller nlmclnt_proc(), so that fl_ops is only set after
nlmclnt_find_lockowner() succeeds

Fixes: bf8848918d75 ("lockd: handle lockowner allocation failure in nlmclnt_proc()")
Signed-off-by: Ran Hongyun <[email protected]>
---
 fs/lockd/clntproc.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c
index f06faf577cea..dc0519ac0172 100644
--- a/fs/lockd/clntproc.c
+++ b/fs/lockd/clntproc.c
@@ -31,11 +31,11 @@ static int	nlmclnt_test(struct nlm_rqst *, struct file_lock *);
 static int	nlmclnt_lock(struct nlm_rqst *, struct file_lock *);
 static int	nlmclnt_unlock(struct nlm_rqst *, struct file_lock *);
 static int	nlm_stat_to_errno(__be32 stat);
-static void	nlmclnt_locks_init_private(struct file_lock *fl, struct nlm_host *host);
 static int	nlmclnt_cancel(struct nlm_host *, int , struct file_lock *);
 
 static const struct rpc_call_ops nlmclnt_unlock_ops;
 static const struct rpc_call_ops nlmclnt_cancel_ops;
+static const struct file_lock_operations nlmclnt_lock_ops;
 
 /*
  * Cookie counter for NLM requests
@@ -172,12 +172,16 @@ int nlmclnt_proc(struct nlm_host *host, int cmd, struct file_lock *fl, void *dat
 	if (nlmclnt_ops && nlmclnt_ops->nlmclnt_alloc_call)
 		nlmclnt_ops->nlmclnt_alloc_call(data);
 
-	nlmclnt_locks_init_private(fl, host);
+	fl->fl_u.nfs_fl.state = 0;
+	fl->fl_u.nfs_fl.owner = nlmclnt_find_lockowner(host, fl->c.flc_owner);
 	if (!fl->fl_u.nfs_fl.owner) {
 		/* lockowner allocation has failed */
 		nlmclnt_release_call(call);
 		return -ENOMEM;
 	}
+	INIT_LIST_HEAD(&fl->fl_u.nfs_fl.list);
+	fl->fl_ops = &nlmclnt_lock_ops;
+
 	/* Set up the argument struct */
 	nlmclnt_setlockargs(call, fl);
 	call->a_callback_data = data;
@@ -484,15 +488,6 @@ static const struct file_lock_operations nlmclnt_lock_ops = {
 	.fl_release_private = nlmclnt_locks_release_private,
 };
 
-static void nlmclnt_locks_init_private(struct file_lock *fl, struct nlm_host *host)
-{
-	fl->fl_u.nfs_fl.state = 0;
-	fl->fl_u.nfs_fl.owner = nlmclnt_find_lockowner(host,
-						       fl->c.flc_owner);
-	INIT_LIST_HEAD(&fl->fl_u.nfs_fl.list);
-	fl->fl_ops = &nlmclnt_lock_ops;
-}
-
 static int do_vfs_lock(struct file_lock *fl)
 {
 	return locks_lock_file_wait(fl->c.flc_file, fl);
-- 
2.52.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.