* [RFC PATCH v1 0/1] Fix hmat_adist_nb for coming CXL Type2 @ 2026-06-30 14:35 alejandro.lucero-palau 2026-06-30 14:35 ` [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block alejandro.lucero-palau 0 siblings, 1 reply; 6+ messages in thread From: alejandro.lucero-palau @ 2026-06-30 14:35 UTC (permalink / raw) To: linux-cxl, linux-acpi, rafael; +Cc: Alejandro Lucero 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block 2026-06-30 14:35 [RFC PATCH v1 0/1] Fix hmat_adist_nb for coming CXL Type2 alejandro.lucero-palau @ 2026-06-30 14:35 ` alejandro.lucero-palau 2026-06-30 13:51 ` sashiko-bot ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: alejandro.lucero-palau @ 2026-06-30 14:35 UTC (permalink / raw) To: linux-cxl, linux-acpi, rafael; +Cc: Alejandro Lucero 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block 2026-06-30 14:35 ` [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block alejandro.lucero-palau @ 2026-06-30 13:51 ` sashiko-bot 2026-06-30 16:18 ` Dave Jiang 2026-07-02 7:20 ` Richard Cheng 2 siblings, 0 replies; 6+ messages in thread From: sashiko-bot @ 2026-06-30 13:51 UTC (permalink / raw) To: alejandro.lucero-palau; +Cc: linux-cxl 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block 2026-06-30 14:35 ` [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block alejandro.lucero-palau 2026-06-30 13:51 ` sashiko-bot @ 2026-06-30 16:18 ` Dave Jiang 2026-07-02 7:20 ` Richard Cheng 2 siblings, 0 replies; 6+ messages in thread From: Dave Jiang @ 2026-06-30 16:18 UTC (permalink / raw) To: alejandro.lucero-palau, linux-cxl, linux-acpi, rafael; +Cc: Alejandro Lucero 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, > }; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block 2026-06-30 14:35 ` [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block alejandro.lucero-palau 2026-06-30 13:51 ` sashiko-bot 2026-06-30 16:18 ` Dave Jiang @ 2026-07-02 7:20 ` Richard Cheng 2026-07-06 6:57 ` Alejandro Lucero Palau 2 siblings, 1 reply; 6+ messages in thread From: Richard Cheng @ 2026-07-02 7:20 UTC (permalink / raw) To: alejandro.lucero-palau; +Cc: linux-cxl, linux-acpi, rafael, Alejandro Lucero 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block 2026-07-02 7:20 ` Richard Cheng @ 2026-07-06 6:57 ` Alejandro Lucero Palau 0 siblings, 0 replies; 6+ messages in thread From: Alejandro Lucero Palau @ 2026-07-06 6:57 UTC (permalink / raw) To: Richard Cheng, alejandro.lucero-palau; +Cc: linux-cxl, linux-acpi, rafael 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-06 6:57 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-30 14:35 [RFC PATCH v1 0/1] Fix hmat_adist_nb for coming CXL Type2 alejandro.lucero-palau 2026-06-30 14:35 ` [RFC PATCH v1 1/1] acpi/hmat: preserve hmat_adist_nb notifier block alejandro.lucero-palau 2026-06-30 13:51 ` sashiko-bot 2026-06-30 16:18 ` Dave Jiang 2026-07-02 7:20 ` Richard Cheng 2026-07-06 6:57 ` Alejandro Lucero Palau
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox