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

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

[Yogesh Gaur]

_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

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

[Alexander Aring]

Hi,

On Wed, Sep 9, 2026 at 7:21 AM Yogesh Gaur <yogeshgaur.83@gmail.com> wrote:
>
> _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.

I agree that the one design flaws of DLM is actually to have lkbs
around that does not have rsbs (lkb->lkb_resource) set, which makes
troubel all over the place if somebody forgets about this case...

It should not be possible to have lkbs around, e.g., in xarray of
"ls->ls_lkbxa" and the rsb is not set, however this is not what your
patch is doing. Your patch is simply to not return a lkb when there is
no lkb->lkb_resource set. It seems we can run into this case as the
above KASAN syzbot shows, although I am worried about cases where we
know it is not set and we handle it.
I would more suggest changing the patch so that it's impossible to
have lkbs around without an rsb being set.

- Alex

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

[Yogesh Gaur]

_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
   device_write+0x905/0xed0 fs/dlm/user.c:590

Make it impossible for ls_lkbxa to hold a lkb without an rsb instead of
filtering such lkbs in find_lkb(). _create_lkb() now only reserves the
lkb_id, by passing a NULL entry to xa_alloc(), so xa_load() returns NULL
for it; attach_lkb() stores the lkb once lkb_resource is set. All four
attach_lkb() callers hold the rsb lock, and nothing takes an rsb lock
under ls_lkbxa_lock, so taking it there adds no new lock ordering.

An lkb that never gets an rsb is still released by __put_lkb(), whose
xa_erase() frees the reserved id as before, and detach_lkb() already
copes with a NULL lkb_resource. The lkb_id is still assigned at create
time, so it is available to tracing and to the caller as before.

The xa_for_each() walks in lockspace.c skip reserved entries. For
lockspace_busy() that means an lkb still between create and attach no
longer makes the lockspace busy. That is the same as the request
arriving just after the check, which can already happen; the caller
holds a lockspace reference, which remove_lockspace() waits for.

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

---
v2: Following review, stop ls_lkbxa from ever holding
    a lkb without an rsb, rather than filtering such lkbs in find_lkb().
    _create_lkb() now only reserves the id; attach_lkb() publishes the
    lkb. Subject changed to match.
v1: https://lore.kernel.org/all/20260909112108.2281-1-yogeshgaur.83@gmail.com/

Not runtime-tested; syzbot has no reproducer for this report. Built
with W=1.

---
 fs/dlm/lock.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index 2609e4fdeba8..2b7f3ff94310 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -1488,10 +1488,22 @@ 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. */
 
+/*
+ * An lkb only becomes visible to find_lkb() here, once it has an rsb.
+ * _create_lkb() just reserves its lkb_id in ls_lkbxa.
+ */
 static void attach_lkb(struct dlm_rsb *r, struct dlm_lkb *lkb)
 {
+	struct dlm_ls *ls = r->res_ls;
+	void *old;
+
 	hold_rsb(r);
 	lkb->lkb_resource = r;
+
+	write_lock_bh(&ls->ls_lkbxa_lock);
+	old = xa_store(&ls->ls_lkbxa, lkb->lkb_id, lkb, GFP_ATOMIC);
+	write_unlock_bh(&ls->ls_lkbxa_lock);
+	WARN_ON_ONCE(old);
 }
 
 static void detach_lkb(struct dlm_lkb *lkb)
@@ -1525,8 +1537,9 @@ 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);
 
+	/* reserve the id only, attach_lkb() publishes the lkb */
 	write_lock_bh(&ls->ls_lkbxa_lock);
-	rv = xa_alloc(&ls->ls_lkbxa, &lkb->lkb_id, lkb, limit, GFP_ATOMIC);
+	rv = xa_alloc(&ls->ls_lkbxa, &lkb->lkb_id, NULL, limit, GFP_ATOMIC);
 	write_unlock_bh(&ls->ls_lkbxa_lock);
 
 	if (rv < 0) {
-- 
2.55.0.windows.5

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

[Alexander Aring]

Hi,

On Sun, Oct 4, 2026 at 1:19 AM Yogesh Gaur <yogeshgaur.83@gmail.com> wrote:
>
> _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
>    device_write+0x905/0xed0 fs/dlm/user.c:590
>
> Make it impossible for ls_lkbxa to hold a lkb without an rsb instead of
> filtering such lkbs in find_lkb(). _create_lkb() now only reserves the
> lkb_id, by passing a NULL entry to xa_alloc(), so xa_load() returns NULL
> for it; attach_lkb() stores the lkb once lkb_resource is set. All four
> attach_lkb() callers hold the rsb lock, and nothing takes an rsb lock
> under ls_lkbxa_lock, so taking it there adds no new lock ordering.
>
> An lkb that never gets an rsb is still released by __put_lkb(), whose
> xa_erase() frees the reserved id as before, and detach_lkb() already
> copes with a NULL lkb_resource. The lkb_id is still assigned at create
> time, so it is available to tracing and to the caller as before.
>
> The xa_for_each() walks in lockspace.c skip reserved entries. For
> lockspace_busy() that means an lkb still between create and attach no
> longer makes the lockspace busy. That is the same as the request
> arriving just after the check, which can already happen; the caller
> holds a lockspace reference, which remove_lockspace() waits for.
>
> 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
>
> ---
> v2: Following review, stop ls_lkbxa from ever holding
>     a lkb without an rsb, rather than filtering such lkbs in find_lkb().
>     _create_lkb() now only reserves the id; attach_lkb() publishes the
>     lkb. Subject changed to match.
> v1: https://lore.kernel.org/all/20260909112108.2281-1-yogeshgaur.83@gmail.com/
>
> Not runtime-tested; syzbot has no reproducer for this report. Built
> with W=1.
>
> ---
>  fs/dlm/lock.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
> index 2609e4fdeba8..2b7f3ff94310 100644
> --- a/fs/dlm/lock.c
> +++ b/fs/dlm/lock.c
> @@ -1488,10 +1488,22 @@ 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. */
>
> +/*
> + * An lkb only becomes visible to find_lkb() here, once it has an rsb.
> + * _create_lkb() just reserves its lkb_id in ls_lkbxa.
> + */
>  static void attach_lkb(struct dlm_rsb *r, struct dlm_lkb *lkb)
>  {
> +       struct dlm_ls *ls = r->res_ls;
> +       void *old;
> +
>         hold_rsb(r);
>         lkb->lkb_resource = r;
> +
> +       write_lock_bh(&ls->ls_lkbxa_lock);
> +       old = xa_store(&ls->ls_lkbxa, lkb->lkb_id, lkb, GFP_ATOMIC);
> +       write_unlock_bh(&ls->ls_lkbxa_lock);
> +       WARN_ON_ONCE(old);
>  }
>
>  static void detach_lkb(struct dlm_lkb *lkb)
> @@ -1525,8 +1537,9 @@ 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);
>
> +       /* reserve the id only, attach_lkb() publishes the lkb */
>         write_lock_bh(&ls->ls_lkbxa_lock);
> -       rv = xa_alloc(&ls->ls_lkbxa, &lkb->lkb_id, lkb, limit, GFP_ATOMIC);
> +       rv = xa_alloc(&ls->ls_lkbxa, &lkb->lkb_id, NULL, limit, GFP_ATOMIC);

Now, all potential iterators need to check if there are reserved lkbs
(being NULL) in ls_lkbxa which makes it not better.

What I meant what ls_lkbxa should contain only valid lkbs (not
reserved as being NULL) that have a lkb_resource set up and not being
NULL. An lkb should always have an rsb.

- Alex