[PATCH v3] dlm: don't publish a lkb in ls_lkbxa before it has an rsb

[Yogesh Gaur <yogeshgaur.83@gmail.com>]

_create_lkb() puts a new lkb into ls_lkbxa straight away, which is also
what assigns its lkb_id, but the lkb only gets an rsb later, when
request_lock(), receive_request(), dlm_recover_master_copy() or
dlm_debug_add_lkb() call attach_lkb(). In between, find_lkb() hands the
lkb out with lkb_resource still NULL, and its callers go straight for
the rsb:

	r = lkb->lkb_resource;

	hold_rsb(r);
	lock_rsb(r);

For the userspace API the lkid is simply whatever was written to the
misc device, so a lkid can be aimed at a lkb that another thread is
still building, and hold_rsb() reads res_flags off NULL:

  BUG: KASAN: null-ptr-deref in rsb_flag fs/dlm/dlm_internal.h:386 [inline]
  BUG: KASAN: null-ptr-deref in hold_rsb fs/dlm/lock.c:334 [inline]
  BUG: KASAN: null-ptr-deref in unlock_lock fs/dlm/lock.c:3333 [inline]
  BUG: KASAN: null-ptr-deref in dlm_user_unlock+0x2ab/0x690 fs/dlm/lock.c:5956
  Read of size 8 at addr 0000000000000050 by task syz.3.570/7893
   rsb_flag fs/dlm/dlm_internal.h:386 [inline]
   hold_rsb fs/dlm/lock.c:334 [inline]
   unlock_lock fs/dlm/lock.c:3333 [inline]
   dlm_user_unlock+0x2ab/0x690 fs/dlm/lock.c:5956
   device_user_unlock+0x1ca/0x260 fs/dlm/user.c:321

Make it impossible for ls_lkbxa to hold a lkb without an rsb, rather
than filtering such lkbs in find_lkb() or reserving a NULL entry that
every walker would have to know about.  create_lkb() no longer touches
ls_lkbxa; attach_lkb() sets lkb_resource and only then allocates the
lkb_id and stores the lkb, so the lkb becomes visible already attached.
dlm_debug_add_lkb() presets the id it wants and attach_lkb() allocates
exactly that one.

attach_lkb() can now fail, and each of its four callers releases the
lkb on the error path it already has for a failure just before the
attach.  On failure attach_lkb() also clears lkb_id, so that
__put_lkb() cannot erase another lkb's entry when a requested id was
busy.  All four callers hold the rsb lock, and nothing takes an rsb
lock under ls_lkbxa_lock, so taking it there adds no new lock ordering.

As a side effect an lkb has no lkb_id until it is attached, so
trace_dlm_lock_start() and the validate_lock_args() and
receive_rcom_lock_args() error messages report 0 for a new lock.

The unchecked r = lkb->lkb_resource goes back to the original DLM
import, but an untrusted lkid only became possible once the userspace
device interface was added, so that is the tag below.

Fixes: 597d0cae0f99 ("[DLM] dlm: user locks")
Reported-by: syzbot+da6dc573ce5e6624f505@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=da6dc573ce5e6624f505
Suggested-by: Alexander Aring <aahringo@redhat.com>
Assisted-by: LLM
Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
---
v3: Incorporated Alexander Aring's review, don't reserve a NULL
    entry in ls_lkbxa either.  The lkb_id is now allocated in
    attach_lkb(), after lkb_resource is set, so ls_lkbxa only ever
    holds attached lkbs.  attach_lkb() can fail; all four callers
    handle it.  _create_lkb() folded into create_lkb().
v2: https://lore.kernel.org/all/20261004051912.1045-1-yogeshgaur.83@gmail.com/
v1: https://lore.kernel.org/all/20260909112108.2281-1-yogeshgaur.83@gmail.com/

Not runtime-tested. Built with W=1.

 fs/dlm/lock.c | 90 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 62 insertions(+), 28 deletions(-)

diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index c715e171bab7..3b3e15157f4c 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -1488,10 +1488,38 @@ void free_inactive_rsb(struct dlm_rsb *r)
 /* Attaching/detaching lkb's from rsb's is for rsb reference counting.
    The rsb must exist as long as any lkb's for it do. */
 
-static void attach_lkb(struct dlm_rsb *r, struct dlm_lkb *lkb)
+/*
+ * An lkb is only put into ls_lkbxa here, once it has an rsb, so
+ * find_lkb() never returns an lkb without one.  The lkb_id is assigned
+ * here too; dlm_debug_add_lkb() presets the one it wants.
+ */
+static int attach_lkb(struct dlm_rsb *r, struct dlm_lkb *lkb)
 {
+	struct dlm_ls *ls = r->res_ls;
+	struct xa_limit limit = XA_LIMIT(1, U32_MAX);
+	int rv;
+
+	if (lkb->lkb_id)
+		limit = XA_LIMIT(lkb->lkb_id, lkb->lkb_id);
+
 	hold_rsb(r);
 	lkb->lkb_resource = r;
+
+	write_lock_bh(&ls->ls_lkbxa_lock);
+	rv = xa_alloc(&ls->ls_lkbxa, &lkb->lkb_id, lkb, limit, GFP_ATOMIC);
+	write_unlock_bh(&ls->ls_lkbxa_lock);
+
+	if (rv < 0) {
+		log_error(ls, "%s xa error %d", __func__, rv);
+		/* the caller still holds its own rsb reference */
+		lkb->lkb_resource = NULL;
+		put_rsb(r);
+		/* don't let __put_lkb() erase another lkb's id */
+		lkb->lkb_id = 0;
+		return rv;
+	}
+
+	return 0;
 }
 
 static void detach_lkb(struct dlm_lkb *lkb)
@@ -1502,15 +1530,9 @@ static void detach_lkb(struct dlm_lkb *lkb)
 	}
 }
 
-static int _create_lkb(struct dlm_ls *ls, struct dlm_lkb **lkb_ret,
-		       unsigned long start, unsigned long end)
+static int create_lkb(struct dlm_ls *ls, struct dlm_lkb **lkb_ret)
 {
-	struct xa_limit limit;
 	struct dlm_lkb *lkb;
-	int rv;
-
-	limit.max = end;
-	limit.min = start;
 
 	lkb = dlm_allocate_lkb();
 	if (!lkb)
@@ -1525,25 +1547,10 @@ static int _create_lkb(struct dlm_ls *ls, struct dlm_lkb **lkb_ret,
 	INIT_LIST_HEAD(&lkb->lkb_ownqueue);
 	INIT_LIST_HEAD(&lkb->lkb_rsb_lookup);
 
-	write_lock_bh(&ls->ls_lkbxa_lock);
-	rv = xa_alloc(&ls->ls_lkbxa, &lkb->lkb_id, lkb, limit, GFP_ATOMIC);
-	write_unlock_bh(&ls->ls_lkbxa_lock);
-
-	if (rv < 0) {
-		log_error(ls, "create_lkb xa error %d", rv);
-		dlm_free_lkb(lkb);
-		return rv;
-	}
-
 	*lkb_ret = lkb;
 	return 0;
 }
 
-static int create_lkb(struct dlm_ls *ls, struct dlm_lkb **lkb_ret)
-{
-	return _create_lkb(ls, lkb_ret, 1, ULONG_MAX);
-}
-
 static int find_lkb(struct dlm_ls *ls, uint32_t lkid, struct dlm_lkb **lkb_ret)
 {
 	struct dlm_lkb *lkb;
@@ -3294,7 +3301,12 @@ static int request_lock(struct dlm_ls *ls, struct dlm_lkb *lkb,
 
 	lock_rsb(r);
 
-	attach_lkb(r, lkb);
+	error = attach_lkb(r, lkb);
+	if (error) {
+		unlock_rsb(r);
+		put_rsb(r);
+		return error;
+	}
 	lkb->lkb_lksb->sb_lkid = lkb->lkb_id;
 
 	error = _request_lock(r, lkb);
@@ -4038,7 +4050,14 @@ static int receive_request(struct dlm_ls *ls, const struct dlm_message *ms)
 		}
 	}
 
-	attach_lkb(r, lkb);
+	error = attach_lkb(r, lkb);
+	if (error) {
+		unlock_rsb(r);
+		put_rsb(r);
+		__put_lkb(ls, lkb);
+		goto fail;
+	}
+
 	error = do_request(r, lkb);
 	send_request_reply(r, lkb, error);
 	do_request_effects(r, lkb, error);
@@ -5642,7 +5661,12 @@ int dlm_recover_master_copy(struct dlm_ls *ls, const struct dlm_rcom *rc,
 		goto out_unlock;
 	}
 
-	attach_lkb(r, lkb);
+	error = attach_lkb(r, lkb);
+	if (error) {
+		__put_lkb(ls, lkb);
+		goto out_unlock;
+	}
+
 	add_lkb(r, lkb, rl->rl_status);
 	ls->ls_recover_locks_in++;
 
@@ -6300,12 +6324,14 @@ int dlm_debug_add_lkb(struct dlm_ls *ls, uint32_t lkb_id, char *name, int len,
 	if (!lksb)
 		return -ENOMEM;
 
-	error = _create_lkb(ls, &lkb, lkb_id, lkb_id + 1);
+	error = create_lkb(ls, &lkb);
 	if (error) {
 		kfree(lksb);
 		return error;
 	}
 
+	lkb->lkb_id = lkb_id;
+
 	dlm_set_dflags_val(lkb, lkb_dflags);
 	lkb->lkb_nodeid = lkb_nodeid;
 	lkb->lkb_lksb = lksb;
@@ -6321,7 +6347,15 @@ int dlm_debug_add_lkb(struct dlm_ls *ls, uint32_t lkb_id, char *name, int len,
 	}
 
 	lock_rsb(r);
-	attach_lkb(r, lkb);
+	error = attach_lkb(r, lkb);
+	if (error) {
+		unlock_rsb(r);
+		put_rsb(r);
+		kfree(lksb);
+		__put_lkb(ls, lkb);
+		return error;
+	}
+
 	add_lkb(r, lkb, lkb_status);
 	unlock_rsb(r);
 	put_rsb(r);
-- 
2.55.0.windows.5