gfs2 filesystem and dlm development
 help / color / mirror / Atom feed
* [PATCH RESEND dlm/next 0/8] dlm: pending fixes based on v7.3-rc1
@ 2026-09-01 17:47 Alexander Aring
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 1/8] dlm: gate dlm_plock device on CAP_SYS_ADMIN Alexander Aring
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Alexander Aring @ 2026-09-01 17:47 UTC (permalink / raw)
  To: teigland; +Cc: aahringo, gfs2

Hi David,

this patch series contains pending fixed for dlm.

Some critical to be ensure that monitor/plock chardevs are only be
opened under administrator rights which should be as e.g. dlm_controld
is using them running as root and can conflict with additional multiple
usages. To forbid any misusage we forbid to open those devices for
normal users.

Then we switch back to rhashtables to dynamic key lengths as this might
get conflicts for DLM users who actually using different resource name
lengths in DLM.

Additional malformed packets drops to avoid kernel crashes, however we
still cannot recover from those drops but this is another problem.

Then there are missing barriers for srcu as call_srcu() uses the srcu
static structures which can end in an use after free when the module is
removed.

- Alex

Alexander Aring (1):
  dlm: fix variable key length lookup

Danila Chernetsov (2):
  dlm: validate lock modes in recovery messages
  dlm: fix NULL pointer dereference in dlm_dump_rsb_name()

Haofeng Li (2):
  dlm: gate dlm_plock device on CAP_SYS_ADMIN
  dlm: require CAP_SYS_ADMIN for dlm-monitor device

Joseph Qi (1):
  dlm: fix buffer overflow from negative len in dlm_search_rsb_tree

Samuel Moelius (1):
  dlm: validate userspace lock resource name length

Zqiang (1):
  dlm: wait for outstanding SRCU callbacks to complete in exit paths

 fs/dlm/config.c       | 30 ++++++++++++++++++++++++++++--
 fs/dlm/dlm_internal.h |  5 +++++
 fs/dlm/lock.c         | 18 +++++++++++++-----
 fs/dlm/lock.h         |  4 ++--
 fs/dlm/lowcomms.c     |  1 +
 fs/dlm/midcomms.c     |  1 +
 fs/dlm/plock.c        | 14 +++++++++++++-
 fs/dlm/user.c         | 23 +++++++++++++++++++++++
 8 files changed, 86 insertions(+), 10 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH RESEND dlm/next 1/8] dlm: gate dlm_plock device on CAP_SYS_ADMIN
  2026-09-01 17:47 [PATCH RESEND dlm/next 0/8] dlm: pending fixes based on v7.3-rc1 Alexander Aring
@ 2026-09-01 17:47 ` Alexander Aring
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 2/8] dlm: require CAP_SYS_ADMIN for dlm-monitor device Alexander Aring
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Alexander Aring @ 2026-09-01 17:47 UTC (permalink / raw)
  To: teigland; +Cc: aahringo, gfs2

From: Haofeng Li <lihaofeng@kylinos.cn>

fs/dlm/plock.c registers /dev/dlm_plock via a miscdevice whose
file_operations has no .open callback and whose .mode field is unset.
The kernel therefore performs no capability check on open, and any
process that can open the node becomes an unprivileged plock daemon
with full read+write access to the pending-plock queue.

Attack chain (when the device node is reachable by an unprivileged
opener — see mitigation note below):

  1. attacker open("/dev/dlm_plock") succeeds with no cap check
  2. dev_read() drains pending plock requests straight to user
     space, leaking dlm_plock_info fields: fsid, number (resource
     id / inode), start, end, owner, pid, ex, wait
  3. dev_write() matches an attacker-supplied dlm_plock_info
     against a pending op on recv_list (matched on fsid+number+
     owner+pid+start+end+ex+wait) and memcpy()'s it into the
     in-kernel op, including a forged .rv == 0
  4. when a matched op exists, the forged result is then applied:
     for async ops (op->data != NULL), dlm_plock_callback() runs
     posix_lock_file(); for sync ops, the requester wakes and
     proceeds as if the cluster had granted the lock.  Either way
     the requester ends up holding a POSIX lock without a real DLM
     grant, or sees attacker-chosen rv that breaks lock correctness.
     With no pending op on recv_list, dev_write() still returns
     sizeof(info) but does nothing beyond a pr_debug ("dlm dev_write
     no op ...") — so a syntactically accepted write does not by
     itself prove an applied grant.

Mitigation: on a stock kernel, devtmpfs creates /dev/dlm_plock as
0600 root:root, so steps 1-4 are only reachable where the node is
exposed to a less privileged principal — e.g. udev MODE=0666,
container bind-mount of the node, or an fd passed via SCM_RIGHTS.
The in-kernel capability gap is real regardless of node mode.

Reproduction (kernel 7.2.0-rc3, dlm loaded):

  # ./exploit_h2   # as root
  [*] node /dev/dlm_plock mode=0600 uid=0 gid=0
  [!!!] AUTH BYPASS: opened with no capability check (fd=3)
  [!!!] FORGE ACCEPTED: kernel accepted forged plock result (rv=0);
       pending ops are grant-forgeable
  [VULNERABLE] open + read-leak + grant-forge demonstrated

  The "FORGE ACCEPTED" line means dev_write() returned sizeof(info),
  i.e. the write path is reachable and the version check passed; it
  does not by itself mean a grant was applied.  Demonstrating an
  actual forged grant requires a concurrent plock op on recv_list to
  match against.

  $ setpriv --reuid 65534 --regid 65534 ./exploit_h2
  [OK ] open denied: Permission denied   # devtmpfs 0600,
                                         not a kernel cap check

Fix: add a .open callback that requires CAP_SYS_ADMIN, and set
.mode = 0600 on the miscdevice so the explicit expectation matches
the devtmpfs default and survives future defaults.

Acked-by: Alexander Aring <aahringo@redhat.com>
Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn>
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/plock.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c
index e9598b3fe5d09..711e8bc3a46a6 100644
--- a/fs/dlm/plock.c
+++ b/fs/dlm/plock.c
@@ -4,6 +4,7 @@
  */
 
 #include <linux/fs.h>
+#include <linux/capability.h>
 #include <linux/filelock.h>
 #include <linux/miscdevice.h>
 #include <linux/poll.h>
@@ -477,6 +478,15 @@ int dlm_posix_get(dlm_lockspace_t *lockspace, u64 number, struct file *file,
 }
 EXPORT_SYMBOL_GPL(dlm_posix_get);
 
+static int dev_open(struct inode *inode, struct file *file)
+{
+	/* Userspace plock daemon is a privileged cluster component. */
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	return 0;
+}
+
 /* a read copies out one plock request from the send list */
 static ssize_t dev_read(struct file *file, char __user *u, size_t count,
 			loff_t *ppos)
@@ -598,6 +608,7 @@ static __poll_t dev_poll(struct file *file, poll_table *wait)
 }
 
 static const struct file_operations dev_fops = {
+	.open    = dev_open,
 	.read    = dev_read,
 	.write   = dev_write,
 	.poll    = dev_poll,
@@ -608,7 +619,8 @@ static const struct file_operations dev_fops = {
 static struct miscdevice plock_dev_misc = {
 	.minor = MISC_DYNAMIC_MINOR,
 	.name = DLM_PLOCK_MISC_NAME,
-	.fops = &dev_fops
+	.fops = &dev_fops,
+	.mode = 0600,
 };
 
 int dlm_plock_init(void)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RESEND dlm/next 2/8] dlm: require CAP_SYS_ADMIN for dlm-monitor device
  2026-09-01 17:47 [PATCH RESEND dlm/next 0/8] dlm: pending fixes based on v7.3-rc1 Alexander Aring
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 1/8] dlm: gate dlm_plock device on CAP_SYS_ADMIN Alexander Aring
@ 2026-09-01 17:47 ` Alexander Aring
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 3/8] dlm: validate userspace lock resource name length Alexander Aring
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Alexander Aring @ 2026-09-01 17:47 UTC (permalink / raw)
  To: teigland; +Cc: aahringo, gfs2

From: Haofeng Li <lihaofeng@kylinos.cn>

monitor_device_open() in fs/dlm/user.c performs only
atomic_inc(&dlm_monitor_opened) and sets dlm_monitor_unused = 0; it
does no capability check.  monitor_device_close() does
atomic_dec_and_test(&dlm_monitor_opened) and, when the count reaches
zero, calls dlm_stop_lockspaces() — which stops every lockspace on
the node.  The miscdevice is also registered with no .mode field.

Attack chain (when the device node is reachable by an unprivileged
opener — see mitigation note below):

  1. attacker open("/dev/dlm-monitor") with no cap check; the
     global counter goes 0 -> 1
  2. attacker close(fd); atomic_dec_and_test reaches zero again
     and dlm_stop_lockspaces() runs -> every DLM lockspace on the
     local node is stopped.  Other cluster members then observe
     the node losing its lockspaces (membership / recovery side
     effects), so the impact is not strictly local to GFS2 /
     OCFS2 / lvmlockd / cluster-md workloads on this node.
  variant: attacker holds the fd open indefinitely to suppress
     the intended stop when dlm_controld later closes its own fd
     (inverse abuse — recovery / shutdown stalls)

Mitigation: devtmpfs creates /dev/dlm-monitor as 0600 root:root on
a stock kernel, so unprivileged open is blocked by the node mode,
not by a kernel cap check.  The gap is real wherever the node is
reachable (udev MODE=0666, container bind-mount, fd via SCM_RIGHTS,
or any setup where dlm_controld shares its monitor fd).

Reproduction (kernel 7.2.0-rc3, dlm loaded, no live lockspace):

  # ./exploit_h3   # as root
  [*] lockspace devices present: 0
  [!!!] AUTH BYPASS: opened /dev/dlm-monitor, no cap check (fd=3)
  [VULNERABLE] monitor open auth bypass demonstrated

  $ setpriv --reuid 65534 --regid 65534 ./exploit_h3
  [OK ] open denied: Permission denied   # node 0600, not cap check

The destructive close path is opt-in in the PoX
(--i-know-it-stops-lockspaces); we did not drive it here.  Driving
the close path on a node with active lockspaces would stop them;
on this throw-away node there are none, but we keep the opt-in gate
so the same PoX is safe to re-run on production-like clusters.

Fix: gate monitor_device_open() on capable(CAP_SYS_ADMIN) and set
.mode = 0600 on monitor_device, matching the dlm_controld-only
intended usage.

Acked-by: Alexander Aring <aahringo@redhat.com>
Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn>
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/user.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/dlm/user.c b/fs/dlm/user.c
index a8ed4c8fdc5bd..cd7e142ca670d 100644
--- a/fs/dlm/user.c
+++ b/fs/dlm/user.c
@@ -4,6 +4,7 @@
  */
 
 #include <linux/miscdevice.h>
+#include <linux/capability.h>
 #include <linux/init.h>
 #include <linux/wait.h>
 #include <linux/file.h>
@@ -910,6 +911,10 @@ static int ctl_device_close(struct inode *inode, struct file *file)
 
 static int monitor_device_open(struct inode *inode, struct file *file)
 {
+	/* dlm_controld is the only expected opener; last close stops LS. */
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
 	atomic_inc(&dlm_monitor_opened);
 	dlm_monitor_unused = 0;
 	return 0;
@@ -958,6 +963,7 @@ static struct miscdevice monitor_device = {
 	.name  = "dlm-monitor",
 	.fops  = &monitor_device_fops,
 	.minor = MISC_DYNAMIC_MINOR,
+	.mode  = 0600,
 };
 
 int __init dlm_user_init(void)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RESEND dlm/next 3/8] dlm: validate userspace lock resource name length
  2026-09-01 17:47 [PATCH RESEND dlm/next 0/8] dlm: pending fixes based on v7.3-rc1 Alexander Aring
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 1/8] dlm: gate dlm_plock device on CAP_SYS_ADMIN Alexander Aring
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 2/8] dlm: require CAP_SYS_ADMIN for dlm-monitor device Alexander Aring
@ 2026-09-01 17:47 ` Alexander Aring
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 4/8] dlm: fix buffer overflow from negative len in dlm_search_rsb_tree Alexander Aring
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Alexander Aring @ 2026-09-01 17:47 UTC (permalink / raw)
  To: teigland; +Cc: aahringo, gfs2

From: Samuel Moelius <sam.moelius@trailofbits.com>

The DLM userspace device accepts a flexible resource name after
`struct dlm_write_request`.  `device_write()` bounded the total write
size, but did not verify that `i.lock.namelen` was covered by the bytes
actually supplied by the write.

A short `DLM_USER_LOCK` request can therefore claim a full
`DLM_RESNAME_MAXLEN` resource name while providing no name bytes.  The
request path later hashes and copies the claimed name length, reading
past the `memdup_user_nul()` allocation.

Reject non-conversion lock requests whose claimed resource name length
exceeds the flexible name payload supplied with the write.  Valid lock
requests with complete names are unchanged.  Track the payload length
before compat conversion so 32-bit requests keep using their own request
header size.

Assisted-by: Codex:gpt-5.5-cyber-preview
Acked-by: Alexander Aring <aahringo@redhat.com>
Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/user.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/fs/dlm/user.c b/fs/dlm/user.c
index cd7e142ca670d..0b0a7e1bd1095 100644
--- a/fs/dlm/user.c
+++ b/fs/dlm/user.c
@@ -512,6 +512,7 @@ static ssize_t device_write(struct file *file, const char __user *buf,
 			    size_t count, loff_t *ppos)
 {
 	struct dlm_user_proc *proc = file->private_data;
+	size_t name_payload = 0;
 	struct dlm_write_request *kbuf;
 	int error;
 
@@ -545,6 +546,7 @@ static ssize_t device_write(struct file *file, const char __user *buf,
 
 		if (count > sizeof(struct dlm_write_request32))
 			namelen = count - sizeof(struct dlm_write_request32);
+		name_payload = namelen;
 
 		k32buf = (struct dlm_write_request32 *)kbuf;
 
@@ -561,7 +563,13 @@ static ssize_t device_write(struct file *file, const char __user *buf,
 
 		compat_input(kbuf, k32buf, namelen);
 		kfree(k32buf);
+	} else {
+		if (count > sizeof(*kbuf))
+			name_payload = count - sizeof(*kbuf);
 	}
+#else
+	if (count > sizeof(*kbuf))
+		name_payload = count - sizeof(*kbuf);
 #endif
 
 	/* do we really need this? can a write happen after a close? */
@@ -571,6 +579,15 @@ static ssize_t device_write(struct file *file, const char __user *buf,
 		goto out_free;
 	}
 
+	if (kbuf->cmd == DLM_USER_LOCK &&
+	    !(kbuf->i.lock.flags & DLM_LKF_CONVERT)) {
+		if (kbuf->i.lock.namelen > name_payload ||
+		    kbuf->i.lock.namelen > DLM_RESNAME_MAXLEN) {
+			error = -EINVAL;
+			goto out_free;
+		}
+	}
+
 	error = -EINVAL;
 
 	switch (kbuf->cmd)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RESEND dlm/next 4/8] dlm: fix buffer overflow from negative len in dlm_search_rsb_tree
  2026-09-01 17:47 [PATCH RESEND dlm/next 0/8] dlm: pending fixes based on v7.3-rc1 Alexander Aring
                   ` (2 preceding siblings ...)
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 3/8] dlm: validate userspace lock resource name length Alexander Aring
@ 2026-09-01 17:47 ` Alexander Aring
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 5/8] dlm: validate lock modes in recovery messages Alexander Aring
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Alexander Aring @ 2026-09-01 17:47 UTC (permalink / raw)
  To: teigland; +Cc: aahringo, gfs2

From: Joseph Qi <joseph.qi@linux.alibaba.com>

commit 080e5563f878c ("dlm: validate length in dlm_search_rsb_tree")
only checks for len > DLM_RESNAME_MAXLEN, which does not catch negative
values. While the input 'len' can be negative and a negative int passed
to memcpy() is implicitly converted to a large size_t, causing a stack
buffer overflow on the key[] array.

Fix this by changing the 'len' parameter type from int to unsigned int.
This ensures negative values from callers are implicitly converted to
large unsigned values that are caught by the existing
len > DLM_RESNAME_MAXLEN check.

Acked-by: Alexander Aring <aahringo@redhat.com>
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/lock.c | 6 ++++--
 fs/dlm/lock.h | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index c381e10284465..373abdb4354a7 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -622,12 +622,14 @@ static int get_rsb_struct(struct dlm_ls *ls, const void *name, int len,
 	return 0;
 }
 
-int dlm_search_rsb_tree(struct rhashtable *rhash, const void *name, int len,
-			struct dlm_rsb **r_ret)
+int dlm_search_rsb_tree(struct rhashtable *rhash, const void *name,
+			unsigned int len, struct dlm_rsb **r_ret)
 {
 	char key[DLM_RESNAME_MAXLEN] = {};
+
 	if (len > DLM_RESNAME_MAXLEN)
 		return -EINVAL;
+
 	memcpy(key, name, len);
 	*r_ret = rhashtable_lookup_fast(rhash, &key, dlm_rhash_rsb_params);
 	if (*r_ret)
diff --git a/fs/dlm/lock.h b/fs/dlm/lock.h
index b23d7b854ed46..c75975937331c 100644
--- a/fs/dlm/lock.h
+++ b/fs/dlm/lock.h
@@ -31,8 +31,8 @@ void resume_scan_timer(struct dlm_ls *ls);
 int dlm_master_lookup(struct dlm_ls *ls, int from_nodeid, const char *name,
 		      int len, unsigned int flags, int *r_nodeid, int *result);
 
-int dlm_search_rsb_tree(struct rhashtable *rhash, const void *name, int len,
-			struct dlm_rsb **r_ret);
+int dlm_search_rsb_tree(struct rhashtable *rhash, const void *name,
+			unsigned int len, struct dlm_rsb **r_ret);
 
 void dlm_recover_purge(struct dlm_ls *ls, const struct list_head *root_list);
 void dlm_purge_mstcpy_locks(struct dlm_rsb *r);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RESEND dlm/next 5/8] dlm: validate lock modes in recovery messages
  2026-09-01 17:47 [PATCH RESEND dlm/next 0/8] dlm: pending fixes based on v7.3-rc1 Alexander Aring
                   ` (3 preceding siblings ...)
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 4/8] dlm: fix buffer overflow from negative len in dlm_search_rsb_tree Alexander Aring
@ 2026-09-01 17:47 ` Alexander Aring
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 6/8] dlm: fix NULL pointer dereference in dlm_dump_rsb_name() Alexander Aring
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Alexander Aring @ 2026-09-01 17:47 UTC (permalink / raw)
  To: teigland; +Cc: aahringo, gfs2

From: Danila Chernetsov <listdansp@mail.ru>

The DLM recovery path restores lock state from rcom_lock messages
received from remote nodes. The lock modes in these messages are
copied directly into the local lkb state without validating that they
are within the valid DLM lock mode range.

The rest of the DLM code assumes that lkb_rqmode and lkb_grmode
contain valid lock modes. In particular, LVB callback handling in
dlm_may_skip_callback() uses lock modes as indexes into the
dlm_lvb_operations array:

    dlm_lvb_operations[prev_mode + 1][mode + 1]

An invalid lock mode received during recovery could therefore result in
an out-of-bounds read during subsequent LVB callback processing.

Validate rl_rqmode and rl_grmode before storing them into the local LKB
state. This preserves the lock mode invariant required by the rest of
the DLM code.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: e7fd41792fc0 ("[DLM] The core of the DLM for GFS2/CLVM")
Acked-by: Alexander Aring <aahringo@redhat.com>
Signed-off-by: Danila Chernetsov <listdansp@mail.ru>
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/lock.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index 373abdb4354a7..99c7a8c4e6122 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -5531,6 +5531,10 @@ static int receive_rcom_lock_args(struct dlm_ls *ls, struct dlm_lkb *lkb,
 {
 	struct rcom_lock *rl = (struct rcom_lock *) rc->rc_buf;
 
+	if (rl->rl_rqmode < DLM_LOCK_IV || rl->rl_rqmode > DLM_LOCK_EX ||
+	    rl->rl_grmode < DLM_LOCK_IV || rl->rl_grmode > DLM_LOCK_EX)
+		return -EINVAL;
+
 	lkb->lkb_nodeid = le32_to_cpu(rc->rc_header.h_nodeid);
 	lkb->lkb_ownpid = le32_to_cpu(rl->rl_ownpid);
 	lkb->lkb_remid = le32_to_cpu(rl->rl_lkid);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RESEND dlm/next 6/8] dlm: fix NULL pointer dereference in dlm_dump_rsb_name()
  2026-09-01 17:47 [PATCH RESEND dlm/next 0/8] dlm: pending fixes based on v7.3-rc1 Alexander Aring
                   ` (4 preceding siblings ...)
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 5/8] dlm: validate lock modes in recovery messages Alexander Aring
@ 2026-09-01 17:47 ` Alexander Aring
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 7/8] dlm: wait for outstanding SRCU callbacks to complete in exit paths Alexander Aring
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 8/8] dlm: fix variable key length lookup Alexander Aring
  7 siblings, 0 replies; 9+ messages in thread
From: Alexander Aring @ 2026-09-01 17:47 UTC (permalink / raw)
  To: teigland; +Cc: aahringo, gfs2

From: Danila Chernetsov <listdansp@mail.ru>

The function dlm_dump_rsb_name() is called from receive_rcom_lookup()
when a debug dump is requested via a special RCOM_LOOKUP message with
rc_id == 0xFFFFFFFF.

The resource name passed to dlm_dump_rsb_name() comes from the received
message. There is no guarantee that an RSB with this name exists in the
local hash table.

dlm_search_rsb_tree() returns 0 when the RSB is found and stores a valid
pointer in r. When the lookup fails, it returns -EBADR and leaves r
NULL.

The current error handling is inverted:

    if (!error)
            goto out;

As a result, dlm_dump_rsb() is called only when the lookup fails and r
is NULL, resulting in a NULL pointer dereference.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 2d90354027ad ("dlm: merge toss and keep hash table lists into one list")
Acked-by: Alexander Aring <aahringo@redhat.com>
Signed-off-by: Danila Chernetsov <listdansp@mail.ru>
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/lock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index 99c7a8c4e6122..d73c4aed1f8ac 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -1423,7 +1423,7 @@ void dlm_dump_rsb_name(struct dlm_ls *ls, const char *name, int len)
 
 	rcu_read_lock();
 	error = dlm_search_rsb_tree(&ls->ls_rsbtbl, name, len, &r);
-	if (!error)
+	if (error)
 		goto out;
 
 	dlm_dump_rsb(r);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RESEND dlm/next 7/8] dlm: wait for outstanding SRCU callbacks to complete in exit paths
  2026-09-01 17:47 [PATCH RESEND dlm/next 0/8] dlm: pending fixes based on v7.3-rc1 Alexander Aring
                   ` (5 preceding siblings ...)
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 6/8] dlm: fix NULL pointer dereference in dlm_dump_rsb_name() Alexander Aring
@ 2026-09-01 17:47 ` Alexander Aring
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 8/8] dlm: fix variable key length lookup Alexander Aring
  7 siblings, 0 replies; 9+ messages in thread
From: Alexander Aring @ 2026-09-01 17:47 UTC (permalink / raw)
  To: teigland; +Cc: aahringo, gfs2

From: Zqiang <qiang.zhang@linux.dev>

The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the
srcu protected connection and node hash tables and hand each
element to call_srcu() for deferred freeing (connection_release()
and midcomms_node_release()). call_srcu() is asynchronous: the
callbacks are invoked only after an SRCU grace period, which may
happen after the exit function has already returned.

These exit functions are reached from exit_dlm() on module unload.
Once they return, module teardown continues and the module text
may be unloaded while call_srcu() callbacks are still pending. When
such a callback finally runs, it executes freed module code and
touches the static SRCU domains that are being torn down, resulting
in a use-after-free.

Add an srcu_barrier() after the call_srcu() loop in each exit function
to wait for all outstanding callbacks of the respective SRCU domain to
complete before returning. In dlm_midcomms_exit() the barrier is issued
before dlm_lowcomms_exit() so that node callbacks are drained prior to
tearing down the lower layer.

Signed-off-by: Zqiang <qiang.zhang@linux.dev>
Acked-by: Alexander Aring <aahringo@redhat.com>
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/lowcomms.c | 1 +
 fs/dlm/midcomms.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index 2aff1c7c17de4..ea8353c4638d0 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -1984,4 +1984,5 @@ void dlm_lowcomms_exit(void)
 		}
 	}
 	srcu_read_unlock(&connections_srcu, idx);
+	srcu_barrier(&connections_srcu);
 }
diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
index 8964164600d2d..0454315244945 100644
--- a/fs/dlm/midcomms.c
+++ b/fs/dlm/midcomms.c
@@ -1178,6 +1178,7 @@ void dlm_midcomms_exit(void)
 		}
 	}
 	srcu_read_unlock(&nodes_srcu, idx);
+	srcu_barrier(&nodes_srcu);
 
 	dlm_lowcomms_exit();
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RESEND dlm/next 8/8] dlm: fix variable key length lookup
  2026-09-01 17:47 [PATCH RESEND dlm/next 0/8] dlm: pending fixes based on v7.3-rc1 Alexander Aring
                   ` (6 preceding siblings ...)
  2026-09-01 17:47 ` [PATCH RESEND dlm/next 7/8] dlm: wait for outstanding SRCU callbacks to complete in exit paths Alexander Aring
@ 2026-09-01 17:47 ` Alexander Aring
  7 siblings, 0 replies; 9+ messages in thread
From: Alexander Aring @ 2026-09-01 17:47 UTC (permalink / raw)
  To: teigland; +Cc: aahringo, gfs2

Before commit 6c648035cbe7 ("dlm: switch to use rhashtable for rsbs")
the rsb hashtable was dynamic key length. Accidentally it was changed to
switch to static key length which can end in different results. We
fixing this back to the original behaviour by adding the necessary
functionality to rhashtable to handle the objects and lookups as dynamic
key lengths.

Fixes: 6c648035cbe7 ("dlm: switch to use rhashtable for rsbs")
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/config.c       | 30 ++++++++++++++++++++++++++++--
 fs/dlm/dlm_internal.h |  5 +++++
 fs/dlm/lock.c         |  6 ++++--
 3 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/fs/dlm/config.c b/fs/dlm/config.c
index 53cd332930423..6c5c3f049b33a 100644
--- a/fs/dlm/config.c
+++ b/fs/dlm/config.c
@@ -64,11 +64,37 @@ static void release_node(struct config_item *);
 static struct configfs_attribute *comm_attrs[];
 static struct configfs_attribute *node_attrs[];
 
+static u32 rsb_hashfn(const void *data, u32 len, u32 seed)
+{
+	const struct dlm_rsb_key *key = data;
+
+	return jhash(key->name, key->len, 0);
+}
+
+static u32 rsb_obj_hashfn(const void *data, u32 len, u32 seed)
+{
+	const struct dlm_rsb *r = data;
+
+	return r->res_hash;
+}
+
+static int rsb_obj_cmpfn(struct rhashtable_compare_arg *arg, const void *obj)
+{
+	const struct dlm_rsb_key *key = arg->key;
+	const struct dlm_rsb *r = obj;
+
+	if (key->len != r->res_length)
+		return -1;
+
+	return memcmp(&r->res_name, key->name, key->len);
+}
+
 const struct rhashtable_params dlm_rhash_rsb_params = {
 	.nelem_hint = 3, /* start small */
-	.key_len = DLM_RESNAME_MAXLEN,
-	.key_offset = offsetof(struct dlm_rsb, res_name),
 	.head_offset = offsetof(struct dlm_rsb, res_node),
+	.hashfn = rsb_hashfn,
+	.obj_hashfn = rsb_obj_hashfn,
+	.obj_cmpfn = rsb_obj_cmpfn,
 	.automatic_shrinking = true,
 };
 
diff --git a/fs/dlm/dlm_internal.h b/fs/dlm/dlm_internal.h
index 9df842421ae06..74c2e77d1b553 100644
--- a/fs/dlm/dlm_internal.h
+++ b/fs/dlm/dlm_internal.h
@@ -340,6 +340,11 @@ struct dlm_rsb {
 	char			res_name[DLM_RESNAME_MAXLEN+1];
 };
 
+struct dlm_rsb_key {
+	char name[DLM_RESNAME_MAXLEN];
+	size_t len;
+};
+
 /* dlm_master_lookup() flags */
 
 #define DLM_LU_RECOVER_DIR	1
diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index d73c4aed1f8ac..2609e4fdeba84 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -625,12 +625,14 @@ static int get_rsb_struct(struct dlm_ls *ls, const void *name, int len,
 int dlm_search_rsb_tree(struct rhashtable *rhash, const void *name,
 			unsigned int len, struct dlm_rsb **r_ret)
 {
-	char key[DLM_RESNAME_MAXLEN] = {};
+	struct dlm_rsb_key key = {
+		.len = len,
+	};
 
 	if (len > DLM_RESNAME_MAXLEN)
 		return -EINVAL;
 
-	memcpy(key, name, len);
+	memcpy(key.name, name, len);
 	*r_ret = rhashtable_lookup_fast(rhash, &key, dlm_rhash_rsb_params);
 	if (*r_ret)
 		return 0;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-09-01 17:47 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-01 17:47 [PATCH RESEND dlm/next 0/8] dlm: pending fixes based on v7.3-rc1 Alexander Aring
2026-09-01 17:47 ` [PATCH RESEND dlm/next 1/8] dlm: gate dlm_plock device on CAP_SYS_ADMIN Alexander Aring
2026-09-01 17:47 ` [PATCH RESEND dlm/next 2/8] dlm: require CAP_SYS_ADMIN for dlm-monitor device Alexander Aring
2026-09-01 17:47 ` [PATCH RESEND dlm/next 3/8] dlm: validate userspace lock resource name length Alexander Aring
2026-09-01 17:47 ` [PATCH RESEND dlm/next 4/8] dlm: fix buffer overflow from negative len in dlm_search_rsb_tree Alexander Aring
2026-09-01 17:47 ` [PATCH RESEND dlm/next 5/8] dlm: validate lock modes in recovery messages Alexander Aring
2026-09-01 17:47 ` [PATCH RESEND dlm/next 6/8] dlm: fix NULL pointer dereference in dlm_dump_rsb_name() Alexander Aring
2026-09-01 17:47 ` [PATCH RESEND dlm/next 7/8] dlm: wait for outstanding SRCU callbacks to complete in exit paths Alexander Aring
2026-09-01 17:47 ` [PATCH RESEND dlm/next 8/8] dlm: fix variable key length lookup Alexander Aring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox