[PATCH] dlm: don't return a lkb that has no rsb from find_lkb()

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

_create_lkb() publishes a new lkb in ls_lkbxa, which is what assigns its
lkb_id:

	rv = xa_alloc(&ls->ls_lkbxa, &lkb->lkb_id, lkb, limit, GFP_ATOMIC);

but the lkb only gets an rsb later, once request_lock() has resolved the
resource name and calls attach_lkb():

	static void attach_lkb(struct dlm_rsb *r, struct dlm_lkb *lkb)
	{
		hold_rsb(r);
		lkb->lkb_resource = r;
	}

So between those two points the lkb is fully addressable by its lkb_id
while lkb_resource is still NULL. find_lkb() will hand it out, and its
callers all go straight for the rsb without checking:

	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 is still being built
by another thread. hold_rsb() then 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
   device_write+0x905/0xed0 fs/dlm/user.c:590

Reject an unattached lkb in find_lkb() rather than in each caller. Every
find_lkb() caller dereferences lkb->lkb_resource -- convert_lock(),
unlock_lock() and cancel_lock() through the r = lkb->lkb_resource above,
dlm_recover_process_copy() the same way, add_to_waiters() via
lkb->lkb_resource->res_ls -- so none of them wants a half-built lkb, and
a lkb without an rsb is not a lock anyone outside can name yet. Callers
already handle find_lkb() failing.

Testing lkb_resource under ls_lkbxa_lock next to the existing kref_read()
check is enough. The value is not stable under that lock, as attach_lkb()
does not take it, but it does not need to be: once a non-NULL rsb has
been observed it stays attached for the life of the reference taken here,
because detach_lkb() only runs from __put_lkb() on the last reference.
Observing NULL while attach_lkb() races is the case being rejected, and
the thread still inside request_lock() has not returned the lkid to
anyone at that point.

This is the null-ptr-deref only. The refcount warning syzbot reports in
dlm_user_request() itself, where hold_lkb() runs on a lkb whose count
already reached zero, is a separate race on an lkb that is past
attach_lkb() and is not addressed here.

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
Assisted-by: LLM
Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
---
 fs/dlm/lock.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index 2609e4fdeba8..73e92237fa07 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -1554,9 +1554,17 @@ static int find_lkb(struct dlm_ls *ls, uint32_t lkid, struct dlm_lkb **lkb_ret)
 		/* check if lkb is still part of lkbxa under lkbxa_lock as
 		 * the lkb_ref is tight to the lkbxa data structure, see
 		 * __put_lkb().
+		 *
+		 * _create_lkb() publishes the lkb in lkbxa before
+		 * attach_lkb() gives it an rsb, so a lkid that comes from
+		 * outside can name a lkb that is still being built. Every
+		 * caller here dereferences lkb->lkb_resource, so treat such
+		 * a lkb as not found. Once an rsb has been seen it stays
+		 * attached, as detach_lkb() only runs from __put_lkb() on
+		 * the last reference and we are about to take one.
 		 */
 		read_lock_bh(&ls->ls_lkbxa_lock);
-		if (kref_read(&lkb->lkb_ref))
+		if (kref_read(&lkb->lkb_ref) && lkb->lkb_resource)
 			kref_get(&lkb->lkb_ref);
 		else
 			lkb = NULL;
-- 
2.55.0.windows.5