The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] ocfs2/dlm: validate lockname_len in dlm_mig_lockres_handler()
@ 2026-07-20 17:08 Ibrahim Hashimov
  2026-07-21  1:21 ` Joseph Qi
  0 siblings, 1 reply; 3+ messages in thread
From: Ibrahim Hashimov @ 2026-07-20 17:08 UTC (permalink / raw)
  To: joseph.qi, mark, jlbec; +Cc: ocfs2-devel, linux-kernel, stable

dlm_mig_lockres_handler() handles an incoming DLM_MIG_LOCKRES_MSG o2net
message from a remote DLM domain node and copies the migratable lockres
straight out of the wire buffer:

    struct dlm_migratable_lockres *mres =
        (struct dlm_migratable_lockres *)msg->buf;
    ...
    res = dlm_new_lockres(dlm, mres->lockname, mres->lockname_len);

mres->lockname_len is a raw u8 field taken verbatim from the network
message (dlmcommon.h), so it can be anywhere in [0, 255].
dlm_new_lockres() forwards it unchanged into dlm_init_lockres():

    qname = (char *) res->lockname.name;
    memcpy(qname, name, namelen);

res->lockname.name is allocated from dlm_lockname_cache, a dedicated
kmem_cache sized exactly DLM_LOCKID_NAME_MAX (32) bytes
(dlm_init_master_caches(), dlmmaster.c). Any lockname_len above 32
therefore makes memcpy() write past the end of that 32-byte slab object;
the maximum attacker-controlled value of 255 yields a 223-byte
slab-out-of-bounds write (CWE-787) with fully attacker-controlled content
(the bytes are mres->lockname itself).

The o2net receive path only bounds-checks the whole payload against
nh_max_len (cluster/tcp.c); it has no notion of the lockname_len
sub-field and cannot catch this. Every other DLM message handler that
consumes an attacker-supplied name length already guards it against
DLM_LOCKID_NAME_MAX right after pulling it out of the wire structure:
dlm_master_request_handler(), dlm_assert_master_handler(),
dlm_deref_lockres_handler() and dlm_deref_lockres_done_handler()
(dlmmaster.c), and the equivalent check in dlmconvert.c / dlmast.c.
dlm_mig_lockres_handler() is simply missing the same check -- present
since the file's introduction and never covered because exercising it
requires two heartbeating o2cb cluster nodes and a joined DLM domain,
i.e. it is invisible to single-node testing and to fuzzers that don't
speak the o2net/o2cb protocol.

The attacker is any node it fully controls (or is on-path and can spoof)
that is heartbeating in the same o2cb cluster and has been permitted to
join the DLM domain. o2net authentication is limited to cluster
membership (nodes list + heartbeat), not per-message payload validation,
so a single rogue or compromised cluster member, or an on-path attacker
able to inject on the unauthenticated o2net TCP stream, can send one
crafted DLM_MIG_LOCKRES_MSG frame with lockname_len = 255 to any other
domain member and corrupt adjacent dlm_lockname_cache slab objects,
leading to memory corruption and, depending on slab layout, a crash or
worse.

Reject the message before it reaches dlm_new_lockres() /
__dlm_lookup_lockres_full() if mres->lockname_len exceeds
DLM_LOCKID_NAME_MAX, mirroring the validation idiom already used by the
sibling handlers above. The check is placed immediately after the
existing dlm_joined() gate, before any use of mres->lockname /
mres->lockname_len, and follows this function's own early-return style
(dlm_put(dlm); return -EINVAL;) rather than the later "leave:" label,
since at this point buf/item are still NULL and no refs have been taken.
This is the smallest possible fix: one bounds check, no change to wire
format, on-disk format, or any other code path.

Reproduced on a v6.19 KASAN-instrumented kernel with an in-kernel
injector that calls dlm_mig_lockres_handler() directly (bypassing the
o2net accept/heartbeat gate that otherwise requires a second heartbeating
cluster node) against a live single-node OCFS2/o2cb DLM domain. A control
frame with lockname_len = 8 completed cleanly; an otherwise-identical
frame differing only in lockname_len = 255 reliably produced
"KASAN: slab-out-of-bounds in dlm_new_lockres", deterministically and
with no dependence on timing or allocator state. Full network delivery
over a real two-node o2cb cluster was not exercised: the injector proves
handler-reachability and the write is deterministic once the handler
runs, but the o2net wire path itself was not verified end-to-end.

Fixes: 6714d8e86bf4 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
 fs/ocfs2/dlm/dlmrecovery.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
index 9b97bf73df22..ec7edffc8ca8 100644
--- a/fs/ocfs2/dlm/dlmrecovery.c
+++ b/fs/ocfs2/dlm/dlmrecovery.c
@@ -1366,6 +1366,12 @@ int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data,
 		return -EINVAL;
 	}
 
+	if (mres->lockname_len > DLM_LOCKID_NAME_MAX) {
+		mlog(ML_ERROR, "Invalid name length!");
+		dlm_put(dlm);
+		return -EINVAL;
+	}
+
 	BUG_ON(!(mres->flags & (DLM_MRES_RECOVERY|DLM_MRES_MIGRATION)));
 
 	real_master = mres->master;
-- 
2.50.1 (Apple Git-155)


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

* Re: [PATCH] ocfs2/dlm: validate lockname_len in dlm_mig_lockres_handler()
  2026-07-20 17:08 [PATCH] ocfs2/dlm: validate lockname_len in dlm_mig_lockres_handler() Ibrahim Hashimov
@ 2026-07-21  1:21 ` Joseph Qi
  2026-07-21 11:53   ` Ibrahim Hashimov
  0 siblings, 1 reply; 3+ messages in thread
From: Joseph Qi @ 2026-07-21  1:21 UTC (permalink / raw)
  To: Ibrahim Hashimov; +Cc: mark, jlbec, ocfs2-devel, linux-kernel

This has been posted before in:
https://lore.kernel.org/ocfs2-devel/20260629-b4-disp-94fb6521-v1-2-6953bcc0421f@proton.me/

And it is now queued in mm-tree.

On 7/21/26 1:08 AM, Ibrahim Hashimov wrote:
> dlm_mig_lockres_handler() handles an incoming DLM_MIG_LOCKRES_MSG o2net
> message from a remote DLM domain node and copies the migratable lockres
> straight out of the wire buffer:
> 
>     struct dlm_migratable_lockres *mres =
>         (struct dlm_migratable_lockres *)msg->buf;
>     ...
>     res = dlm_new_lockres(dlm, mres->lockname, mres->lockname_len);
> 
> mres->lockname_len is a raw u8 field taken verbatim from the network
> message (dlmcommon.h), so it can be anywhere in [0, 255].
> dlm_new_lockres() forwards it unchanged into dlm_init_lockres():
> 
>     qname = (char *) res->lockname.name;
>     memcpy(qname, name, namelen);
> 
> res->lockname.name is allocated from dlm_lockname_cache, a dedicated
> kmem_cache sized exactly DLM_LOCKID_NAME_MAX (32) bytes
> (dlm_init_master_caches(), dlmmaster.c). Any lockname_len above 32
> therefore makes memcpy() write past the end of that 32-byte slab object;
> the maximum attacker-controlled value of 255 yields a 223-byte
> slab-out-of-bounds write (CWE-787) with fully attacker-controlled content
> (the bytes are mres->lockname itself).
> 
> The o2net receive path only bounds-checks the whole payload against
> nh_max_len (cluster/tcp.c); it has no notion of the lockname_len
> sub-field and cannot catch this. Every other DLM message handler that
> consumes an attacker-supplied name length already guards it against
> DLM_LOCKID_NAME_MAX right after pulling it out of the wire structure:
> dlm_master_request_handler(), dlm_assert_master_handler(),
> dlm_deref_lockres_handler() and dlm_deref_lockres_done_handler()
> (dlmmaster.c), and the equivalent check in dlmconvert.c / dlmast.c.
> dlm_mig_lockres_handler() is simply missing the same check -- present
> since the file's introduction and never covered because exercising it
> requires two heartbeating o2cb cluster nodes and a joined DLM domain,
> i.e. it is invisible to single-node testing and to fuzzers that don't
> speak the o2net/o2cb protocol.
> 
> The attacker is any node it fully controls (or is on-path and can spoof)
> that is heartbeating in the same o2cb cluster and has been permitted to
> join the DLM domain. o2net authentication is limited to cluster
> membership (nodes list + heartbeat), not per-message payload validation,
> so a single rogue or compromised cluster member, or an on-path attacker
> able to inject on the unauthenticated o2net TCP stream, can send one
> crafted DLM_MIG_LOCKRES_MSG frame with lockname_len = 255 to any other
> domain member and corrupt adjacent dlm_lockname_cache slab objects,
> leading to memory corruption and, depending on slab layout, a crash or
> worse.
> 
> Reject the message before it reaches dlm_new_lockres() /
> __dlm_lookup_lockres_full() if mres->lockname_len exceeds
> DLM_LOCKID_NAME_MAX, mirroring the validation idiom already used by the
> sibling handlers above. The check is placed immediately after the
> existing dlm_joined() gate, before any use of mres->lockname /
> mres->lockname_len, and follows this function's own early-return style
> (dlm_put(dlm); return -EINVAL;) rather than the later "leave:" label,
> since at this point buf/item are still NULL and no refs have been taken.
> This is the smallest possible fix: one bounds check, no change to wire
> format, on-disk format, or any other code path.
> 
> Reproduced on a v6.19 KASAN-instrumented kernel with an in-kernel
> injector that calls dlm_mig_lockres_handler() directly (bypassing the
> o2net accept/heartbeat gate that otherwise requires a second heartbeating
> cluster node) against a live single-node OCFS2/o2cb DLM domain. A control
> frame with lockname_len = 8 completed cleanly; an otherwise-identical
> frame differing only in lockname_len = 255 reliably produced
> "KASAN: slab-out-of-bounds in dlm_new_lockres", deterministically and
> with no dependence on timing or allocator state. Full network delivery
> over a real two-node o2cb cluster was not exercised: the injector proves
> handler-reachability and the write is deterministic once the handler
> runs, but the o2net wire path itself was not verified end-to-end.
> 
> Fixes: 6714d8e86bf4 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
> Assisted-by: AuditCode-AI:2026.07
> ---
>  fs/ocfs2/dlm/dlmrecovery.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
> index 9b97bf73df22..ec7edffc8ca8 100644
> --- a/fs/ocfs2/dlm/dlmrecovery.c
> +++ b/fs/ocfs2/dlm/dlmrecovery.c
> @@ -1366,6 +1366,12 @@ int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data,
>  		return -EINVAL;
>  	}
>  
> +	if (mres->lockname_len > DLM_LOCKID_NAME_MAX) {
> +		mlog(ML_ERROR, "Invalid name length!");
> +		dlm_put(dlm);
> +		return -EINVAL;
> +	}
> +
>  	BUG_ON(!(mres->flags & (DLM_MRES_RECOVERY|DLM_MRES_MIGRATION)));
>  
>  	real_master = mres->master;


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

* Re: [PATCH] ocfs2/dlm: validate lockname_len in dlm_mig_lockres_handler()
  2026-07-21  1:21 ` Joseph Qi
@ 2026-07-21 11:53   ` Ibrahim Hashimov
  0 siblings, 0 replies; 3+ messages in thread
From: Ibrahim Hashimov @ 2026-07-21 11:53 UTC (permalink / raw)
  To: joseph.qi; +Cc: mark, jlbec, ocfs2-devel, linux-kernel

Thanks, and sorry for the duplicate -- I missed the earlier posting.
Please drop mine; the queued version covers it and I'll follow that one.

Ibrahim

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

end of thread, other threads:[~2026-07-21 11:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20 17:08 [PATCH] ocfs2/dlm: validate lockname_len in dlm_mig_lockres_handler() Ibrahim Hashimov
2026-07-21  1:21 ` Joseph Qi
2026-07-21 11:53   ` Ibrahim Hashimov

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