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

[Ran Hongyun]

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 <ranhongyun1@xxxxxxxxxx>
---
 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