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

[Alexander Aring]

Hi,

On Wed, Oct 7, 2026 at 7:32 AM Yogesh Gaur <yogeshgaur.83@xxxxxxxxx> 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
>
> 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=da6dc573ce5e6624f505
> Suggested-by: Alexander Aring <aahringo@xxxxxxxxxx>
> Assisted-by: LLM
> Signed-off-by: Yogesh Gaur <yogeshgaur.83@xxxxxxxxx>
> ---
> 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@xxxxxxxxx/
> v1: https://lore.kernel.org/all/20260909112108.2281-1-yogeshgaur.83@xxxxxxxxx/
>
> Not runtime-tested. Built with W=1.

I tried this, and it seems to run without problems. Although there are
questionable things like [0] as now create_lkb() does not add the lkb
to the xarray and [0] will call xa_erase() on a lkb that has not been
added if set_lock_args() fails. It probably will not harm as lkb_id 0
is invalid and never be part of the lkb xarray, so this xa_erase()
will do nothing.

I am willing to ack this patch because I think the cases where an lkb
has no rsb set have already affected us several times in the past and
it seems to be going in the right direction. We will just see what
happens.

Acked-by: Alexander Aring <aahringo@xxxxxxxxxx>

- Alex

[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/dlm/lock.c?h=v7.4-rc1#n3412