[PATCH 1/2] ocfs2/dlm: validate qr_numregions in dlm_match_regions()

[PATCH 1/2] ocfs2/dlm: validate qr_numregions in dlm_match_regions()

[Junrui Luo]

The qr_numregions field from a DLM_QUERY_REGION network message is used
directly as loop bounds in dlm_match_regions() without checking against
O2NM_MAX_REGIONS. Since qr_regions is sized for at most O2NM_MAX_REGIONS
(32) entries, a crafted message with qr_numregions > 32 causes
out-of-bounds reads past the qr_regions buffer.

Add a bounds check for qr_numregions before entering the loops.

Fixes: ea2034416b54 ("ocfs2/dlm: Add message DLM_QUERY_REGION")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 fs/ocfs2/dlm/dlmdomain.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c
index 70ca79e4bdc3..0a28cb5ded2a 100644
--- a/fs/ocfs2/dlm/dlmdomain.c
+++ b/fs/ocfs2/dlm/dlmdomain.c
@@ -980,6 +980,14 @@ static int dlm_match_regions(struct dlm_ctxt *dlm,
 		goto bail;
 	}
 
+	if (qr->qr_numregions > O2NM_MAX_REGIONS) {
+		mlog(ML_ERROR, "Domain %s: Joining node %d has invalid "
+		     "number of heartbeat regions %u\n",
+		     qr->qr_domain, qr->qr_node, qr->qr_numregions);
+		status = -EINVAL;
+		goto bail;
+	}
+
 	r = remote;
 	for (i = 0; i < qr->qr_numregions; ++i) {
 		mlog(0, "Region %.*s\n", O2HB_MAX_REGION_NAME_LEN, r);

-- 
2.52.0

[PATCH 2/2] ocfs2/dlm: fix off-by-one in dlm_match_regions() region comparison

[Junrui Luo]

The local-vs-remote region comparison loop uses '<=' instead of '<',
causing it to read one entry past the valid range of qr_regions. The
other loops in the same function correctly use '<'.

Fix the loop condition to use '<' for consistency and correctness.

Fixes: ea2034416b54 ("ocfs2/dlm: Add message DLM_QUERY_REGION")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 fs/ocfs2/dlm/dlmdomain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c
index 0a28cb5ded2a..dc9da9133c8e 100644
--- a/fs/ocfs2/dlm/dlmdomain.c
+++ b/fs/ocfs2/dlm/dlmdomain.c
@@ -1002,7 +1002,7 @@ static int dlm_match_regions(struct dlm_ctxt *dlm,
 	for (i = 0; i < localnr; ++i) {
 		foundit = 0;
 		r = remote;
-		for (j = 0; j <= qr->qr_numregions; ++j) {
+		for (j = 0; j < qr->qr_numregions; ++j) {
 			if (!memcmp(l, r, O2HB_MAX_REGION_NAME_LEN)) {
 				foundit = 1;
 				break;

-- 
2.52.0

[PATCH 0/2] ocfs2/dlm: fix two bugs in dlm_match_regions()

[Junrui Luo]

In dlm_match_regions(), the qr_numregions field from a DLM_QUERY_REGION
network message is used to drive loops over the qr_regions buffer without
sufficient validation. This series fixes two issues:

- Patch 1 adds a bounds check to reject messages where qr_numregions
  exceeds O2NM_MAX_REGIONS. The o2net layer only validates message
  byte length; it does not constrain field values, so a crafted message
  can set qr_numregions up to 255 and trigger out-of-bounds reads past
  the 1024-byte qr_regions buffer.

- Patch 2 fixes an off-by-one in the local-vs-remote comparison loop,
  which uses '<=' instead of '<', reading one entry past the valid range
  even when qr_numregions is within bounds.

Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
Junrui Luo (2):
      ocfs2/dlm: validate qr_numregions in dlm_match_regions()
      ocfs2/dlm: fix off-by-one in dlm_match_regions() region comparison

 fs/ocfs2/dlm/dlmdomain.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
---
base-commit: 0031c06807cfa8aa51a759ff8aa09e1aa48149af
change-id: 20260307-fixes-9d8b95012a49

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>

Re: [PATCH 0/2] ocfs2/dlm: fix two bugs in dlm_match_regions()

[Wengang Wang]

The two patches looks good to me.

Reviewed-by: Wengang Wang <wen.gang.wang@oracle.com <mailto:wen.gang.wang@oracle.com>>

Just unrelated to these patches, looking at function o2hb_get_all_regions,

2519 int o2hb_get_all_regions(char *region_uuids, u8 max_regions)
2520 {
2521         struct o2hb_region *reg;
2522         int numregs = 0;
2523         char *p;
2524
2525         spin_lock(&o2hb_live_lock);
2526
2527         p = region_uuids;
2528         list_for_each_entry(reg, &o2hb_all_regions, hr_all_item) {
2529                 if (reg->hr_item_dropped)
2530                         continue;
2531
2532                 mlog(0, "Region: %s\n", config_item_name(&reg->hr_item));
2533                 if (numregs < max_regions) {
2534                         memcpy(p, config_item_name(&reg->hr_item),
2535                                O2HB_MAX_REGION_NAME_LEN);
2536                         p += O2HB_MAX_REGION_NAME_LEN;
2537                 }
2538                 numregs++;
2539         }
2540
2541         spin_unlock(&o2hb_live_lock);
2542
2543         return numregs;
2544 }

It seems that ‘numregs’ can mismatch with ‘region_uuids' (line 2538 VS line 2534) due to the limitation of ‘max_regions’
though calling o2hb_get_all_regions() with max_regions being O2NM_MAX_REGIONS looks good.

Thanks,
Wengang


> On Mar 6, 2026, at 11:22 PM, Junrui Luo <moonafterrain@outlook.com> wrote:
> 
> In dlm_match_regions(), the qr_numregions field from a DLM_QUERY_REGION
> network message is used to drive loops over the qr_regions buffer without
> sufficient validation. This series fixes two issues:
> 
> - Patch 1 adds a bounds check to reject messages where qr_numregions
>  exceeds O2NM_MAX_REGIONS. The o2net layer only validates message
>  byte length; it does not constrain field values, so a crafted message
>  can set qr_numregions up to 255 and trigger out-of-bounds reads past
>  the 1024-byte qr_regions buffer.
> 
> - Patch 2 fixes an off-by-one in the local-vs-remote comparison loop,
>  which uses '<=' instead of '<', reading one entry past the valid range
>  even when qr_numregions is within bounds.
> 
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> ---
> Junrui Luo (2):
>      ocfs2/dlm: validate qr_numregions in dlm_match_regions()
>      ocfs2/dlm: fix off-by-one in dlm_match_regions() region comparison
> 
> fs/ocfs2/dlm/dlmdomain.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
> ---
> base-commit: 0031c06807cfa8aa51a759ff8aa09e1aa48149af
> change-id: 20260307-fixes-9d8b95012a49
> 
> Best regards,
> -- 
> Junrui Luo <moonafterrain@outlook.com>
> 
> 

Re: [PATCH 0/2] ocfs2/dlm: fix two bugs in dlm_match_regions()

[Wengang Wang]

Sorry, my email client added more to the "Reviewed-by” part, correcting it as (hope the correction looks good). 

Reviewed-by: Wengang Wang <wen.gang.wang@oracle.com>

Thanks,
Wengang

> On Mar 7, 2026, at 8:06 AM, Wengang Wang <wen.gang.wang@oracle.com> wrote:
> 
> The two patches looks good to me.
> 
> Reviewed-by: Wengang Wang <wen.gang.wang@oracle.com <mailto:wen.gang.wang@oracle.com>>
> 
> Just unrelated to these patches, looking at function o2hb_get_all_regions,
> 
> 2519 int o2hb_get_all_regions(char *region_uuids, u8 max_regions)
> 2520 {
> 2521         struct o2hb_region *reg;
> 2522         int numregs = 0;
> 2523         char *p;
> 2524
> 2525         spin_lock(&o2hb_live_lock);
> 2526
> 2527         p = region_uuids;
> 2528         list_for_each_entry(reg, &o2hb_all_regions, hr_all_item) {
> 2529                 if (reg->hr_item_dropped)
> 2530                         continue;
> 2531
> 2532                 mlog(0, "Region: %s\n", config_item_name(&reg->hr_item));
> 2533                 if (numregs < max_regions) {
> 2534                         memcpy(p, config_item_name(&reg->hr_item),
> 2535                                O2HB_MAX_REGION_NAME_LEN);
> 2536                         p += O2HB_MAX_REGION_NAME_LEN;
> 2537                 }
> 2538                 numregs++;
> 2539         }
> 2540
> 2541         spin_unlock(&o2hb_live_lock);
> 2542
> 2543         return numregs;
> 2544 }
> 
> It seems that ‘numregs’ can mismatch with ‘region_uuids' (line 2538 VS line 2534) due to the limitation of ‘max_regions’
> though calling o2hb_get_all_regions() with max_regions being O2NM_MAX_REGIONS looks good.
> 
> Thanks,
> Wengang
> 
> 
>> On Mar 6, 2026, at 11:22 PM, Junrui Luo <moonafterrain@outlook.com> wrote:
>> 
>> In dlm_match_regions(), the qr_numregions field from a DLM_QUERY_REGION
>> network message is used to drive loops over the qr_regions buffer without
>> sufficient validation. This series fixes two issues:
>> 
>> - Patch 1 adds a bounds check to reject messages where qr_numregions
>> exceeds O2NM_MAX_REGIONS. The o2net layer only validates message
>> byte length; it does not constrain field values, so a crafted message
>> can set qr_numregions up to 255 and trigger out-of-bounds reads past
>> the 1024-byte qr_regions buffer.
>> 
>> - Patch 2 fixes an off-by-one in the local-vs-remote comparison loop,
>> which uses '<=' instead of '<', reading one entry past the valid range
>> even when qr_numregions is within bounds.
>> 
>> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
>> ---
>> Junrui Luo (2):
>>     ocfs2/dlm: validate qr_numregions in dlm_match_regions()
>>     ocfs2/dlm: fix off-by-one in dlm_match_regions() region comparison
>> 
>> fs/ocfs2/dlm/dlmdomain.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>> ---
>> base-commit: 0031c06807cfa8aa51a759ff8aa09e1aa48149af
>> change-id: 20260307-fixes-9d8b95012a49
>> 
>> Best regards,
>> -- 
>> Junrui Luo <moonafterrain@outlook.com>
>> 
>> 
> 

Re: [PATCH 0/2] ocfs2/dlm: fix two bugs in dlm_match_regions()

[Joseph Qi]

On 3/8/26 12:06 AM, Wengang Wang wrote:
> The two patches looks good to me.
> 
> Reviewed-by: Wengang Wang <wen.gang.wang@oracle.com <mailto:wen.gang.wang@oracle.com>>
> 
> Just unrelated to these patches, looking at function o2hb_get_all_regions,
> 
> 2519 int o2hb_get_all_regions(char *region_uuids, u8 max_regions)
> 2520 {
> 2521         struct o2hb_region *reg;
> 2522         int numregs = 0;
> 2523         char *p;
> 2524
> 2525         spin_lock(&o2hb_live_lock);
> 2526
> 2527         p = region_uuids;
> 2528         list_for_each_entry(reg, &o2hb_all_regions, hr_all_item) {
> 2529                 if (reg->hr_item_dropped)
> 2530                         continue;
> 2531
> 2532                 mlog(0, "Region: %s\n", config_item_name(&reg->hr_item));
> 2533                 if (numregs < max_regions) {
> 2534                         memcpy(p, config_item_name(&reg->hr_item),
> 2535                                O2HB_MAX_REGION_NAME_LEN);
> 2536                         p += O2HB_MAX_REGION_NAME_LEN;
> 2537                 }
> 2538                 numregs++;
> 2539         }
> 2540
> 2541         spin_unlock(&o2hb_live_lock);
> 2542
> 2543         return numregs;
> 2544 }
> 
> It seems that ‘numregs’ can mismatch with ‘region_uuids' (line 2538 VS line 2534) due to the limitation of ‘max_regions’
> though calling o2hb_get_all_regions() with max_regions being O2NM_MAX_REGIONS looks good.
> 

Yes, it looks wried here.
So why not fix it by moving 'numregs++' into if clause?

Thanks,
Joseph

Re: [PATCH 1/2] ocfs2/dlm: validate qr_numregions in dlm_match_regions()

[Joseph Qi]

On 3/7/26 3:21 PM, Junrui Luo wrote:
> The qr_numregions field from a DLM_QUERY_REGION network message is used
> directly as loop bounds in dlm_match_regions() without checking against
> O2NM_MAX_REGIONS. Since qr_regions is sized for at most O2NM_MAX_REGIONS
> (32) entries, a crafted message with qr_numregions > 32 causes
> out-of-bounds reads past the qr_regions buffer.
> 
> Add a bounds check for qr_numregions before entering the loops.
> 
> Fixes: ea2034416b54 ("ocfs2/dlm: Add message DLM_QUERY_REGION")
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> ---
>  fs/ocfs2/dlm/dlmdomain.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c
> index 70ca79e4bdc3..0a28cb5ded2a 100644
> --- a/fs/ocfs2/dlm/dlmdomain.c
> +++ b/fs/ocfs2/dlm/dlmdomain.c
> @@ -980,6 +980,14 @@ static int dlm_match_regions(struct dlm_ctxt *dlm,
>  		goto bail;
>  	}
>  
> +	if (qr->qr_numregions > O2NM_MAX_REGIONS) {
> +		mlog(ML_ERROR, "Domain %s: Joining node %d has invalid "
> +		     "number of heartbeat regions %u\n",
> +		     qr->qr_domain, qr->qr_node, qr->qr_numregions);
> +		status = -EINVAL;
> +		goto bail;
> +	}
> +
>  	r = remote;
>  	for (i = 0; i < qr->qr_numregions; ++i) {
>  		mlog(0, "Region %.*s\n", O2HB_MAX_REGION_NAME_LEN, r);
> 

Re: [PATCH 2/2] ocfs2/dlm: fix off-by-one in dlm_match_regions() region comparison

[Joseph Qi]

On 3/7/26 3:21 PM, Junrui Luo wrote:
> The local-vs-remote region comparison loop uses '<=' instead of '<',
> causing it to read one entry past the valid range of qr_regions. The
> other loops in the same function correctly use '<'.
> 
> Fix the loop condition to use '<' for consistency and correctness.
> 
> Fixes: ea2034416b54 ("ocfs2/dlm: Add message DLM_QUERY_REGION")
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
>  fs/ocfs2/dlm/dlmdomain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c
> index 0a28cb5ded2a..dc9da9133c8e 100644
> --- a/fs/ocfs2/dlm/dlmdomain.c
> +++ b/fs/ocfs2/dlm/dlmdomain.c
> @@ -1002,7 +1002,7 @@ static int dlm_match_regions(struct dlm_ctxt *dlm,
>  	for (i = 0; i < localnr; ++i) {
>  		foundit = 0;
>  		r = remote;
> -		for (j = 0; j <= qr->qr_numregions; ++j) {
> +		for (j = 0; j < qr->qr_numregions; ++j) {
>  			if (!memcmp(l, r, O2HB_MAX_REGION_NAME_LEN)) {
>  				foundit = 1;
>  				break;
>