Re: [PATCH 2/2] ocfs2: validate lengths in dlm_mig_lockres_handler

On 6/29/26 1:01 PM, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <[email protected]>
> 
> A node receiving a DLM_MIG_LOCKRES message trusts several fields of the
> peer-supplied dlm_migratable_lockres without validation.  num_locks and
> lockname_len are bounded only on the sending side, and the message is
> never checked to actually carry num_locks migratable_lock entries.  As a
> result dlm_process_recovery_data() walks mres->ml[0..num_locks) past the
> kmalloc(data_len) copy of the message (an out-of-bounds read that ends in
> a BUG_ON panic), and dlm_init_lockres() copies lockname_len bytes into the
> fixed 32-byte o2dlm_lockname slab object (a heap out-of-bounds write).
> Both are reachable by any node in the domain.
> 
> Validate these fields right after dlm_grab(), before anything uses them --
> including the not-joined error path, which already prints mres->lockname
> with the unbounded lockname_len as a %.*s precision.  Reject the message
> unless lockname_len <= DLM_LOCKID_NAME_MAX, num_locks <=
> DLM_MAX_MIGRATABLE_LOCKS (the bound the sender already asserts), and the
> payload is large enough to hold the claimed locks.  Conforming recovery
> and migration messages are unaffected.
> 
> Fixes: 6714d8e86bf4 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
> Cc: [email protected]
> Signed-off-by: Bryam Vargas <[email protected]>

Looks fine.
Reviewed-by: Joseph Qi <[email protected]>

> ---
>  fs/ocfs2/dlm/dlmrecovery.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
> index 128872bd945d..14d4c7c3eebd 100644
> --- a/fs/ocfs2/dlm/dlmrecovery.c
> +++ b/fs/ocfs2/dlm/dlmrecovery.c
> @@ -1357,6 +1357,15 @@ int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data,
>  	if (!dlm_grab(dlm))
>  		return -EINVAL;
>  
> +	if (mres->lockname_len > DLM_LOCKID_NAME_MAX ||
> +	    mres->num_locks > DLM_MAX_MIGRATABLE_LOCKS ||
> +	    be16_to_cpu(msg->data_len) < struct_size(mres, ml, mres->num_locks)) {
> +		mlog(ML_ERROR, "%s: invalid lockres migration message from %u\n",
> +		     dlm->name, mres->master);
> +		dlm_put(dlm);
> +		return -EINVAL;
> +	}
> +
>  	if (!dlm_joined(dlm)) {
>  		mlog(ML_ERROR, "Domain %s not joined! "
>  			  "lockres %.*s, master %u\n",
>