Re: [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block

Re: [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Integer overflow during the ACPI HMAT locality table size calculation bypasses bounds checking and allows out-of-bounds memory reads.
--

commit 69d5452dda77c729836d9ff08f4b38f9cff8d91b
Author: Alejandro Lucero <alucerop@amd.com>

acpi/hmat: preserve hmat_adist_nb notifier block

Remove __meminitdata from hmat_adist_nb declaration preserving it even
when CONFIG_MEMORY_HOTPLUG is not set. Otherwise further notifier
registrations using register_mt_adistance_algorithm() will find a
notifier chain likely corrupted. This is the case when a CXL Type2
driver will load after kernel initialization and a CXL region created from
its committed HDM decoders registers an mt_adistance_algorithm notifier.

> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
> index 9792dc3947561..3006789ae31fe 100644
> --- a/drivers/acpi/numa/hmat.c
> +++ b/drivers/acpi/numa/hmat.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a regression where an integer
overflow during the ACPI HMAT locality table size calculation could bypass
bounds checking.

When parsing the locality table, total_size is declared as a 32-bit unsigned
integer:

drivers/acpi/numa/hmat.c:hmat_parse_locality() {
	...
	unsigned int init, targ, total_size, ipds, tpds;
	...
	ipds = hmat_loc->number_of_initiator_Pds;
	tpds = hmat_loc->number_of_target_Pds;
	total_size = sizeof(*hmat_loc) + sizeof(*entries) * ipds * tpds +
		     sizeof(*inits) * ipds + sizeof(*targs) * tpds;
	if (hmat_loc->header.length < total_size) {
		pr_notice("Unexpected locality header length:%u, minimum required:%u\n",
			 hmat_loc->header.length, total_size);
		return -EINVAL;
	}
	...

If abnormally large values are provided for ipds and tpds, could the
intermediate arithmetic be truncated when assigned to total_size?

If this wraps around 32-bit boundaries, wouldn't it result in a small
total_size that bypasses the hmat_loc->header.length check?

Later loops use the unbounded ipds and tpds variables:

drivers/acpi/numa/hmat.c:hmat_parse_locality() {
	...
	for (init = 0; init < ipds; init++) {
		alloc_memory_initiator(inits[init]);
		for (targ = 0; targ < tpds; targ++) {
			value = hmat_normalize(entries[init * tpds + targ],
					       hmat_loc->entry_base_unit,
					       type);
	...
}

Does this lead to out-of-bounds memory reads if the total_size calculation
overflows?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630143507.30193-1-alejandro.lucero-palau@amd.com?part=1

[RFC PATCH v1 0/1] Fix hmat_adist_nb for coming CXL Type2

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

While working on adding CXL Type2 support I found a problem with
register_mt_adistance_algorithm() which is used inside
cxl_region_probe(). It turns out my kernel config had not
CONFIG_MEMORY_HOTPLUG enabled leading to the hmat_adist_nb notifier
block deleted after kernel initialization without doing any notifer
block unregister, leaving the notifier chain unaware of this. A CXL
Type2 driver can be load after kernel initialization and with a kernel
not enabling memory hotplug, the notifier registration crashes if the
released memory from hmat_adist_nb is reused.

I'm not sure but I do not think this can happen for Type3 devices
because the related CXL driver is used only during kernel initialization
if memory hotplug not enabled. Type2, aka CXL accelerator drivers, is
coming and the problem will arise with certainty.

I have contemplated to modify how the identified __meminit and
__meminitdat are defined in linux/init.h, extending the case for being
empty definitions if CONFIG_CXL_MEM=y, but I think it makes sense to
only avoid the use of __meminit for hmat_adist_nb.

FWIW, a Type2 device should not require CONFIG_MEMORY_HOTPLUG and its
memory is currently initialized by the BIOS. Maybe not having memory
hotplug enabled is unusual and even more with CXL, so another option
could be to enable memory hotplug if CXL mem is enabled.

Alejandro Lucero (1):
  acpi/hmat: preserve hmat_adist_nb notifier block

 drivers/acpi/numa/hmat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
-- 
2.34.1

[RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block

[alejandro.lucero-palau]

From: Alejandro Lucero <alucerop@amd.com>

Remove __meminitdata from hmat_adist_nb declaration preserving it even
when CONFIG_MEMORY_HOTPLUG is not set. Otherwise further notifier
registrations using register_mt_adistance_algorithm() will find a
notifier chain likely corrupted. This is the case when a CXL Type2
driver will load after kernel initialization and a CXL region created from
its committed HDM decoders registers an mt_adistance_algorithm notifier.

Signed-off-by: Alejandro Lucero <alucerop@amd.com>
---
 drivers/acpi/numa/hmat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
index 9792dc394756..3006789ae31f 100644
--- a/drivers/acpi/numa/hmat.c
+++ b/drivers/acpi/numa/hmat.c
@@ -995,7 +995,7 @@ static int hmat_calculate_adistance(struct notifier_block *self,
 	return NOTIFY_STOP;
 }
 
-static struct notifier_block hmat_adist_nb __meminitdata = {
+static struct notifier_block hmat_adist_nb = {
 	.notifier_call = hmat_calculate_adistance,
 	.priority = 100,
 };
-- 
2.34.1

Re: [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block

[Dave Jiang]

On 6/30/26 7:35 AM, alejandro.lucero-palau@amd.com wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> Remove __meminitdata from hmat_adist_nb declaration preserving it even
> when CONFIG_MEMORY_HOTPLUG is not set. Otherwise further notifier
> registrations using register_mt_adistance_algorithm() will find a
> notifier chain likely corrupted. This is the case when a CXL Type2
> driver will load after kernel initialization and a CXL region created from
> its committed HDM decoders registers an mt_adistance_algorithm notifier.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> ---
>  drivers/acpi/numa/hmat.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
> index 9792dc394756..3006789ae31f 100644
> --- a/drivers/acpi/numa/hmat.c
> +++ b/drivers/acpi/numa/hmat.c
> @@ -995,7 +995,7 @@ static int hmat_calculate_adistance(struct notifier_block *self,
>  	return NOTIFY_STOP;
>  }
>  
> -static struct notifier_block hmat_adist_nb __meminitdata = {
> +static struct notifier_block hmat_adist_nb = {

Maybe do something similar to __init_or_acpilib in include/linux/acpi.h?

>  	.notifier_call = hmat_calculate_adistance,
>  	.priority = 100,
>  };

Re: [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block

[Richard Cheng]

On Tue, Jun 30, 2026 at 03:35:07PM +0800, alejandro.lucero-palau@amd.com wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> Remove __meminitdata from hmat_adist_nb declaration preserving it even
> when CONFIG_MEMORY_HOTPLUG is not set. Otherwise further notifier
> registrations using register_mt_adistance_algorithm() will find a
> notifier chain likely corrupted. This is the case when a CXL Type2
> driver will load after kernel initialization and a CXL region created from
> its committed HDM decoders registers an mt_adistance_algorithm notifier.
> 
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> ---
>  drivers/acpi/numa/hmat.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
> index 9792dc394756..3006789ae31f 100644
> --- a/drivers/acpi/numa/hmat.c
> +++ b/drivers/acpi/numa/hmat.c
> @@ -995,7 +995,7 @@ static int hmat_calculate_adistance(struct notifier_block *self,
>  	return NOTIFY_STOP;
>  }
>  
> -static struct notifier_block hmat_adist_nb __meminitdata = {
> +static struct notifier_block hmat_adist_nb = {
>  	.notifier_call = hmat_calculate_adistance,
>  	.priority = 100,
>  };
> -- 
> 2.34.1
> 
>

I think this is not limited to CXL type 2 device.
cxl_region_probe() already calls register_mt_adistance_algorithm(), and CXL_REGION
depends only on SPARSEMEM, not MEMORY_HOTPLUG, so with NUMA=y, MEMORY_HOTPLUG=n, CXL_ACPI=y and CXL_REGION=y,
any runtime CXL region probe walks the chain. I would suggest some tweak for commit message.
Also adding Fixes: 3718c02dbd4c ("acpi, hmat: calculate abstract distance with HMAT")

And maybe keep unconditional removal rather than a consumer-keyed attribute.
register_mt_adistance_algorithm() is callable by any runtime module,
not just CXL, so a CONFIG_CXL_MEM-keyed condition would  still leave the hole
for other callers.

What do you think ?

Best regards,
Richard Cheng.

Re: [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block

[Alejandro Lucero Palau]

On 7/2/26 08:20, Richard Cheng wrote:
> On Tue, Jun 30, 2026 at 03:35:07PM +0800, alejandro.lucero-palau@amd.com wrote:
>> From: Alejandro Lucero <alucerop@amd.com>
>>
>> Remove __meminitdata from hmat_adist_nb declaration preserving it even
>> when CONFIG_MEMORY_HOTPLUG is not set. Otherwise further notifier
>> registrations using register_mt_adistance_algorithm() will find a
>> notifier chain likely corrupted. This is the case when a CXL Type2
>> driver will load after kernel initialization and a CXL region created from
>> its committed HDM decoders registers an mt_adistance_algorithm notifier.
>>
>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>> ---
>>   drivers/acpi/numa/hmat.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
>> index 9792dc394756..3006789ae31f 100644
>> --- a/drivers/acpi/numa/hmat.c
>> +++ b/drivers/acpi/numa/hmat.c
>> @@ -995,7 +995,7 @@ static int hmat_calculate_adistance(struct notifier_block *self,
>>   	return NOTIFY_STOP;
>>   }
>>   
>> -static struct notifier_block hmat_adist_nb __meminitdata = {
>> +static struct notifier_block hmat_adist_nb = {
>>   	.notifier_call = hmat_calculate_adistance,
>>   	.priority = 100,
>>   };
>> -- 
>> 2.34.1
>>
>>
> I think this is not limited to CXL type 2 device.
> cxl_region_probe() already calls register_mt_adistance_algorithm(), and CXL_REGION
> depends only on SPARSEMEM, not MEMORY_HOTPLUG, so with NUMA=y, MEMORY_HOTPLUG=n, CXL_ACPI=y and CXL_REGION=y,
> any runtime CXL region probe walks the chain. I would suggest some tweak for commit message.
> Also adding Fixes: 3718c02dbd4c ("acpi, hmat: calculate abstract distance with HMAT")


I think you are right. I did say it was not affecting Type3 because I 
was assuming only Type3 regions created during CXL core initialization 
and cxl_mem "active" before after-initialization kernel memory release. 
I will add what you suggest.


>
> And maybe keep unconditional removal rather than a consumer-keyed attribute.
> register_mt_adistance_algorithm() is callable by any runtime module,
> not just CXL, so a CONFIG_CXL_MEM-keyed condition would  still leave the hole
> for other callers.


Not sure what you mean here. The way I understand your comment leads to 
contradictions ...


"keep unconditional removal" suggest to me unregistering hmat_adist_nb 
but "keep" confuses me as it is not unregister now only memory released 
based on __meminitdata ... but maybe you mean exactly what I did and 
avoid any change to __meminitdata declaration based on a config 
attribute, what is what Dave is suggesting to explore.


Please, could you clarify this?

Thank you,

Alejandro.


>
> What do you think ?
>
> Best regards,
> Richard Cheng.