* [PATCH v3 0/2] x86/amd_node: Fixes for virtualized systems @ 2026-08-25 21:48 Jason Andryuk 2026-08-25 21:48 ` [PATCH v3 1/2] x86/amd_node: Avoid divide by zero on " Jason Andryuk 2026-08-25 21:48 ` [PATCH v3 2/2] x86/amd_node: Remove smn_exclusive Jason Andryuk 0 siblings, 2 replies; 12+ messages in thread From: Jason Andryuk @ 2026-08-25 21:48 UTC (permalink / raw) To: Borislav Petkov, Dave Hansen, H. Peter Anvin, Ingo Molnar, Mario Limonciello, Thomas Gleixner, x86, Yazen Ghannam Cc: Jason Andryuk, linux-kernel, Penny Zheng For v3, Add newline to warning. R-b Yazen. For v2, swap the patch order. On my test system, I see SMN accesses from: amd_pmf_probe() - read amd_pmc_probe() - read acp_acp70_audio_probe() - write Jason Andryuk (2): x86/amd_node: Avoid divide by zero on virtualized systems x86/amd_node: Remove smn_exclusive arch/x86/kernel/amd_node.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) -- 2.55.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 1/2] x86/amd_node: Avoid divide by zero on virtualized systems 2026-08-25 21:48 [PATCH v3 0/2] x86/amd_node: Fixes for virtualized systems Jason Andryuk @ 2026-08-25 21:48 ` Jason Andryuk 2026-08-26 4:45 ` Borislav Petkov 2026-08-31 17:35 ` [tip: x86/urgent] " tip-bot2 for Jason Andryuk 2026-08-25 21:48 ` [PATCH v3 2/2] x86/amd_node: Remove smn_exclusive Jason Andryuk 1 sibling, 2 replies; 12+ messages in thread From: Jason Andryuk @ 2026-08-25 21:48 UTC (permalink / raw) To: Borislav Petkov, Dave Hansen, H. Peter Anvin, Ingo Molnar, Mario Limonciello, Thomas Gleixner, x86, Yazen Ghannam Cc: Jason Andryuk, linux-kernel, Penny Zheng, stable On a virtualized system, the number of nodes does not have a relationship to the number of roots. A Xen PVH dom0 can calculate roots_per_node as 0, which crashes with a divide by zero in: if (count++ % roots_per_node) The issue is seen with Xen, but it could affect other systems. Change roots_per_node == 0 to 1 to allow the system to boot. Print a warning when this is performed for non-virtualized systems. Fixes: 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching") Cc: stable@vger.kernel.org Suggested-by: Borislav Petkov <bp@alien8.de> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> --- v3: Add newline to warning. R-b: Yazen v2: Print as a warning instead of error Use "!" instead of "== 0". X86_FEATURE_XENPV is only for PV, but this is observed with a PVH dom0. --- arch/x86/kernel/amd_node.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c index 0be01725a2a4..408b9fd48349 100644 --- a/arch/x86/kernel/amd_node.c +++ b/arch/x86/kernel/amd_node.c @@ -287,6 +287,11 @@ static int __init amd_smn_init(void) return -ENOMEM; roots_per_node = num_roots / num_nodes; + if (!roots_per_node) { + if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR)) + pr_warn(FW_BUG "Error detecting roots per node.\n"); + roots_per_node = 1; + } count = 0; node = 0; -- 2.55.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] x86/amd_node: Avoid divide by zero on virtualized systems 2026-08-25 21:48 ` [PATCH v3 1/2] x86/amd_node: Avoid divide by zero on " Jason Andryuk @ 2026-08-26 4:45 ` Borislav Petkov 2026-08-26 12:15 ` Jason Andryuk 2026-08-31 17:35 ` [tip: x86/urgent] " tip-bot2 for Jason Andryuk 1 sibling, 1 reply; 12+ messages in thread From: Borislav Petkov @ 2026-08-26 4:45 UTC (permalink / raw) To: Jason Andryuk Cc: Dave Hansen, H. Peter Anvin, Ingo Molnar, Mario Limonciello, Thomas Gleixner, x86, Yazen Ghannam, linux-kernel, Penny Zheng, stable On Tue, Aug 25, 2026 at 05:48:02PM -0400, Jason Andryuk wrote: > On a virtualized system, the number of nodes does not have a > relationship to the number of roots. A Xen PVH dom0 can calculate > roots_per_node as 0, which crashes with a divide by zero in: > > if (count++ % roots_per_node) > > The issue is seen with Xen, but it could affect other systems. > > Change roots_per_node == 0 to 1 to allow the system to boot. Print a > warning when this is performed for non-virtualized systems. > > Fixes: 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching") > Cc: stable@vger.kernel.org > Suggested-by: Borislav Petkov <bp@alien8.de> > Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com> > Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> > --- > v3: > Add newline to warning. > R-b: Yazen > > v2: > Print as a warning instead of error > Use "!" instead of "== 0". > > X86_FEATURE_XENPV is only for PV, but this is observed with a PVH dom0. > --- > arch/x86/kernel/amd_node.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c > index 0be01725a2a4..408b9fd48349 100644 > --- a/arch/x86/kernel/amd_node.c > +++ b/arch/x86/kernel/amd_node.c > @@ -287,6 +287,11 @@ static int __init amd_smn_init(void) > return -ENOMEM; > > roots_per_node = num_roots / num_nodes; Hold on - the only way this is fixing something on Xen - and you say that roots_per_node becomes 0 - which means that num_roots is 0. But a couple of lines above, we have if (!num_roots) return -ENODEV; So we would've exited already. num_nodes can't be 0 because you'd explode even with this patch. What am I missing? Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] x86/amd_node: Avoid divide by zero on virtualized systems 2026-08-26 4:45 ` Borislav Petkov @ 2026-08-26 12:15 ` Jason Andryuk 2026-08-26 15:55 ` Borislav Petkov 0 siblings, 1 reply; 12+ messages in thread From: Jason Andryuk @ 2026-08-26 12:15 UTC (permalink / raw) To: Borislav Petkov Cc: Dave Hansen, H. Peter Anvin, Ingo Molnar, Mario Limonciello, Thomas Gleixner, x86, Yazen Ghannam, linux-kernel, Penny Zheng, stable On 2026-08-26 00:45, Borislav Petkov wrote: > On Tue, Aug 25, 2026 at 05:48:02PM -0400, Jason Andryuk wrote: >> On a virtualized system, the number of nodes does not have a >> relationship to the number of roots. A Xen PVH dom0 can calculate >> roots_per_node as 0, which crashes with a divide by zero in: >> >> if (count++ % roots_per_node) >> >> The issue is seen with Xen, but it could affect other systems. >> >> Change roots_per_node == 0 to 1 to allow the system to boot. Print a >> warning when this is performed for non-virtualized systems. >> >> Fixes: 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching") >> Cc: stable@vger.kernel.org >> Suggested-by: Borislav Petkov <bp@alien8.de> >> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com> >> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> >> --- >> v3: >> Add newline to warning. >> R-b: Yazen >> >> v2: >> Print as a warning instead of error >> Use "!" instead of "== 0". >> >> X86_FEATURE_XENPV is only for PV, but this is observed with a PVH dom0. >> --- >> arch/x86/kernel/amd_node.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c >> index 0be01725a2a4..408b9fd48349 100644 >> --- a/arch/x86/kernel/amd_node.c >> +++ b/arch/x86/kernel/amd_node.c >> @@ -287,6 +287,11 @@ static int __init amd_smn_init(void) >> return -ENOMEM; >> >> roots_per_node = num_roots / num_nodes; > > Hold on - the only way this is fixing something on Xen - and you say that > roots_per_node becomes 0 - which means that num_roots is 0. > > But a couple of lines above, we have > > if (!num_roots) > return -ENODEV; > > So we would've exited already. > > num_nodes can't be 0 because you'd explode even with this patch. > > What am I missing? num_roots is 1, but num_nodes is 2. Integer division gives 0 for roots_per_node. Regards, Jason ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] x86/amd_node: Avoid divide by zero on virtualized systems 2026-08-26 12:15 ` Jason Andryuk @ 2026-08-26 15:55 ` Borislav Petkov 0 siblings, 0 replies; 12+ messages in thread From: Borislav Petkov @ 2026-08-26 15:55 UTC (permalink / raw) To: Jason Andryuk Cc: Dave Hansen, H. Peter Anvin, Ingo Molnar, Mario Limonciello, Thomas Gleixner, x86, Yazen Ghannam, linux-kernel, Penny Zheng, stable On Wed, Aug 26, 2026 at 08:15:49AM -0400, Jason Andryuk wrote: > num_roots is 1, but num_nodes is 2. Integer division gives 0 for > roots_per_node. Aha, Xen has some topology knowledge and it manages to produce 2 nodes. I'll add that to the commit message. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 12+ messages in thread
* [tip: x86/urgent] x86/amd_node: Avoid divide by zero on virtualized systems 2026-08-25 21:48 ` [PATCH v3 1/2] x86/amd_node: Avoid divide by zero on " Jason Andryuk 2026-08-26 4:45 ` Borislav Petkov @ 2026-08-31 17:35 ` tip-bot2 for Jason Andryuk 1 sibling, 0 replies; 12+ messages in thread From: tip-bot2 for Jason Andryuk @ 2026-08-31 17:35 UTC (permalink / raw) To: linux-tip-commits Cc: Borislav Petkov, Jason Andryuk, Yazen Ghannam, stable, x86, linux-kernel The following commit has been merged into the x86/urgent branch of tip: Commit-ID: 72bd92bd8190d7869ecb462649ca40f297822a33 Gitweb: https://git.kernel.org/tip/72bd92bd8190d7869ecb462649ca40f297822a33 Author: Jason Andryuk <jason.andryuk@amd.com> AuthorDate: Tue, 25 Aug 2026 17:48:02 -04:00 Committer: Borislav Petkov (AMD) <bp@alien8.de> CommitterDate: Sun, 30 Aug 2026 18:25:42 -07:00 x86/amd_node: Avoid divide by zero on virtualized systems On a virtualized system, the number of nodes does not have a relationship to the number of roots. A Xen PVH dom0 can calculate roots_per_node as 0, which crashes with a divide by zero in: if (count++ % roots_per_node) because the underlying topology code on Xen ends up making num_nodes 2 and num_roots 1 and the integer division result is 0. The issue is seen with Xen, but it could affect other systems. Set roots_per_node to 1 in this case. Print a firmware bug when this is performed for non-virtualized systems. [ bp: Massage commit message. ] Fixes: 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching") Suggested-by: Borislav Petkov <bp@alien8.de> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com> Cc: stable@vger.kernel.org Link: https://patch.msgid.link/20260825214805.39148-2-jason.andryuk@amd.com --- arch/x86/kernel/amd_node.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c index 0be0172..408b9fd 100644 --- a/arch/x86/kernel/amd_node.c +++ b/arch/x86/kernel/amd_node.c @@ -287,6 +287,11 @@ static int __init amd_smn_init(void) return -ENOMEM; roots_per_node = num_roots / num_nodes; + if (!roots_per_node) { + if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR)) + pr_warn(FW_BUG "Error detecting roots per node.\n"); + roots_per_node = 1; + } count = 0; node = 0; ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/2] x86/amd_node: Remove smn_exclusive 2026-08-25 21:48 [PATCH v3 0/2] x86/amd_node: Fixes for virtualized systems Jason Andryuk 2026-08-25 21:48 ` [PATCH v3 1/2] x86/amd_node: Avoid divide by zero on " Jason Andryuk @ 2026-08-25 21:48 ` Jason Andryuk 2026-08-31 2:30 ` Borislav Petkov 2026-09-02 21:32 ` [tip: x86/urgent] x86/amd_node: Prevent potential NULL pointer dereference tip-bot2 for Jason Andryuk 1 sibling, 2 replies; 12+ messages in thread From: Jason Andryuk @ 2026-08-25 21:48 UTC (permalink / raw) To: Borislav Petkov, Dave Hansen, H. Peter Anvin, Ingo Molnar, Mario Limonciello, Thomas Gleixner, x86, Yazen Ghannam Cc: Jason Andryuk, linux-kernel, Penny Zheng, stable, Mario Limonciello (AMD) amd_smn_read/write() are exported functions around __amd_smn_rw(), so they are always available even if amd_smn_init() fails. smn_exclusive would prevent access to __amd_smn_rw(), but it is placed too late. If amd_smn_init() failed, amd_roots is NULL and __amd_smn_rw() will fault over it. Replace smn_exclusive with directly checking amd_roots to avoid the NULL pointer dereference. commit 83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access") added smn_exclusive which indicated the calls to pci_request_config_region_exclusive() succeeded to prevent userspace access. commit 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching") re-ordered initialization so pci_request_config_region_exclusive() is called earlier and a failure exits amd_smn_init() before allocating amd_roots. Setting smn_exclusive moved to the end of amd_smn_init(), after amd_roots is allocated. smn_exclusive became redundant to amd_roots and can be removed. Add a comment stating how amd_roots replaces smn_exclusive's purpose. Fixes: 77466b798d59 ("x86/amd_node: Remove dependency on AMD_NB") Cc: stable@vger.kernel.org Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> --- Fixes is the introduction of amd_roots, which could be a NULL deref. v3: No change v2: R-b Yazen and Mario Add comment about amd_root taking smn_exclusive's purpose to indicate userspace access is prevented. Expand commit message --- arch/x86/kernel/amd_node.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c index 408b9fd48349..833e5eda5d91 100644 --- a/arch/x86/kernel/amd_node.c +++ b/arch/x86/kernel/amd_node.c @@ -38,7 +38,6 @@ static struct pci_dev **amd_roots; /* Protect the PCI config register pairs used for SMN. */ static DEFINE_MUTEX(smn_mutex); -static bool smn_exclusive; #define SMN_INDEX_OFFSET 0x60 #define SMN_DATA_OFFSET 0x64 @@ -91,11 +90,15 @@ static int __amd_smn_rw(u8 i_off, u8 d_off, u16 node, u32 address, u32 *value, b if (node >= amd_num_nodes()) return err; - root = amd_roots[node]; - if (!root) + /* + * non-NULL amd_roots indicates pci_request_config_region_exclusive() + * succeeded and userspace cannot access the registers. + */ + if (!amd_roots) return err; - if (!smn_exclusive) + root = amd_roots[node]; + if (!root) return err; guard(mutex)(&smn_mutex); @@ -313,8 +316,6 @@ static int __init amd_smn_init(void) debugfs_create_file("value", 0600, debugfs_dir, NULL, &smn_value_fops); } - smn_exclusive = true; - return 0; } -- 2.55.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] x86/amd_node: Remove smn_exclusive 2026-08-25 21:48 ` [PATCH v3 2/2] x86/amd_node: Remove smn_exclusive Jason Andryuk @ 2026-08-31 2:30 ` Borislav Petkov 2026-08-31 17:52 ` Jason Andryuk 2026-09-02 21:32 ` [tip: x86/urgent] x86/amd_node: Prevent potential NULL pointer dereference tip-bot2 for Jason Andryuk 1 sibling, 1 reply; 12+ messages in thread From: Borislav Petkov @ 2026-08-31 2:30 UTC (permalink / raw) To: Jason Andryuk Cc: Dave Hansen, H. Peter Anvin, Ingo Molnar, Mario Limonciello, Thomas Gleixner, x86, Yazen Ghannam, linux-kernel, Penny Zheng, stable, Mario Limonciello (AMD) On Tue, Aug 25, 2026 at 05:48:03PM -0400, Jason Andryuk wrote: > - root = amd_roots[node]; > - if (!root) > + /* > + * non-NULL amd_roots indicates pci_request_config_region_exclusive() > + * succeeded and userspace cannot access the registers. > + */ > + if (!amd_roots) So then this should be if (amd_roots) return err; ? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] x86/amd_node: Remove smn_exclusive 2026-08-31 2:30 ` Borislav Petkov @ 2026-08-31 17:52 ` Jason Andryuk 2026-09-01 0:27 ` Borislav Petkov 0 siblings, 1 reply; 12+ messages in thread From: Jason Andryuk @ 2026-08-31 17:52 UTC (permalink / raw) To: Borislav Petkov Cc: Dave Hansen, H. Peter Anvin, Ingo Molnar, Mario Limonciello, Thomas Gleixner, x86, Yazen Ghannam, linux-kernel, Penny Zheng, stable, Mario Limonciello (AMD) On 2026-08-30 22:30, Borislav Petkov wrote: > On Tue, Aug 25, 2026 at 05:48:03PM -0400, Jason Andryuk wrote: >> - root = amd_roots[node]; >> - if (!root) >> + /* >> + * non-NULL amd_roots indicates pci_request_config_region_exclusive() >> + * succeeded and userspace cannot access the registers. >> + */ >> + if (!amd_roots) > > So then this should be > > if (amd_roots) > return err; > > ? No, the code and the comment are both correct as posted. It's just that I wrote the comment reversed from the code to state the positive property we want. Flipping it would be: "NULL amd_roots means pci_request_config_region_exclusive() failed and userspace can access the registers." But I'd like to reconsider this comment. There is an existing comment before pci_request_config_region_exclusive() in amd_smn_init() stating its purpose. The commit message explains how smn_exclusive is no longer needed. That seems enough to make the code change without adding a comment. I wasn't sure how to phrase the comment because it feels out of place to me. Because the other side is that it is a valid NULL check to prevent dereferencing amd_roots. That is the important part. Maybe the commit messages should have been "Avoid NULL deref in __amd_smn_rw()" to highlight that? When you look at the code as a whole, smn_exclusive isn't doing anything, which is why I removed it. I'll change this whichever way you want. It's not a big deal, which is why I added the comment like you requested originally. Regards, Jason ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] x86/amd_node: Remove smn_exclusive 2026-08-31 17:52 ` Jason Andryuk @ 2026-09-01 0:27 ` Borislav Petkov 2026-09-01 11:59 ` Jason Andryuk 0 siblings, 1 reply; 12+ messages in thread From: Borislav Petkov @ 2026-09-01 0:27 UTC (permalink / raw) To: Jason Andryuk Cc: Dave Hansen, H. Peter Anvin, Ingo Molnar, Mario Limonciello, Thomas Gleixner, x86, Yazen Ghannam, linux-kernel, Penny Zheng, stable, Mario Limonciello (AMD) On Mon, Aug 31, 2026 at 01:52:08PM -0400, Jason Andryuk wrote: > No, the code and the comment are both correct as posted. It's just that I > wrote the comment reversed from the code to state the positive property we > want. So if amd_smn_init() succeeds, that means we have registered the PCI config regions of the PCI roots, including the SMN ones, as IORESOURCE_EXCLUSIVE and they won't be exported to userspace through /dev/mem and what not. So, any caller of __amd_smn_rw() should fail because it is coming from kernel space and it can expect that userspace could interfere with the hw and thus not a good idea. So yes, this is what got me confused and the (!amd_roots) check is correct. > I'll change this whichever way you want. It's not a big deal, which is why > I added the comment like you requested originally. I think I already "intercepted" your suggestion, see here: From 1a1c4addc5a14e9ef2bb7d351a62ff4563d0f10a Mon Sep 17 00:00:00 2001 From: Jason Andryuk <jason.andryuk@amd.com> Date: Tue, 25 Aug 2026 17:48:03 -0400 Subject: [PATCH] x86/amd_node: Prevent potential NULL pointer dereference amd_smn_read/write() are exported functions around __amd_smn_rw(), so they are always available even if amd_smn_init() fails. In that case, amd_roots is NULL and __amd_smn_rw() will access uninitialized memory. Then, commit 83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access") added smn_exclusive which indicated the calls to pci_request_config_region_exclusive() succeeded to prevent concurrent userspace access. Commit 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching") re-ordered initialization so pci_request_config_region_exclusive() is called earlier and a failure exits amd_smn_init() before allocating amd_roots. Setting smn_exclusive moved to the end of amd_smn_init(), after amd_roots is allocated. It became redundant and can be removed. Replace smn_exclusive with directly checking amd_roots to avoid a potential NULL pointer dereference. [ bp: Reorg commit message, touchup comment. ] Fixes: 77466b798d59 ("x86/amd_node: Remove dependency on AMD_NB") Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org> Cc: stable@vger.kernel.org Link: https://patch.msgid.link/20260825214805.39148-3-jason.andryuk@amd.com --- arch/x86/kernel/amd_node.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c index 408b9fd48349..762585775b5a 100644 --- a/arch/x86/kernel/amd_node.c +++ b/arch/x86/kernel/amd_node.c @@ -38,7 +38,6 @@ static struct pci_dev **amd_roots; /* Protect the PCI config register pairs used for SMN. */ static DEFINE_MUTEX(smn_mutex); -static bool smn_exclusive; #define SMN_INDEX_OFFSET 0x60 #define SMN_DATA_OFFSET 0x64 @@ -91,11 +90,16 @@ static int __amd_smn_rw(u8 i_off, u8 d_off, u16 node, u32 address, u32 *value, b if (node >= amd_num_nodes()) return err; - root = amd_roots[node]; - if (!root) + /* + * Uninitialized amd_roots indicates pci_request_config_region_exclusive() + * didn't run or failed and thus the kernel cannot rely on having + * exclusive access to SMN registers so prevent that. + */ + if (!amd_roots) return err; - if (!smn_exclusive) + root = amd_roots[node]; + if (!root) return err; guard(mutex)(&smn_mutex); @@ -313,8 +317,6 @@ static int __init amd_smn_init(void) debugfs_create_file("value", 0600, debugfs_dir, NULL, &smn_value_fops); } - smn_exclusive = true; - return 0; } -- 2.53.0 -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/2] x86/amd_node: Remove smn_exclusive 2026-09-01 0:27 ` Borislav Petkov @ 2026-09-01 11:59 ` Jason Andryuk 0 siblings, 0 replies; 12+ messages in thread From: Jason Andryuk @ 2026-09-01 11:59 UTC (permalink / raw) To: Borislav Petkov Cc: Dave Hansen, H. Peter Anvin, Ingo Molnar, Mario Limonciello, Thomas Gleixner, x86, Yazen Ghannam, linux-kernel, Penny Zheng, stable, Mario Limonciello (AMD) On 2026-08-31 20:27, Borislav Petkov wrote: > On Mon, Aug 31, 2026 at 01:52:08PM -0400, Jason Andryuk wrote: >> No, the code and the comment are both correct as posted. It's just that I >> wrote the comment reversed from the code to state the positive property we >> want. > > So if amd_smn_init() succeeds, that means we have registered the PCI config > regions of the PCI roots, including the SMN ones, as IORESOURCE_EXCLUSIVE and > they won't be exported to userspace through /dev/mem and what not. > > So, any caller of __amd_smn_rw() should fail because it is coming from kernel > space and it can expect that userspace could interfere with the hw and thus > not a good idea. > > So yes, this is what got me confused and the (!amd_roots) check is correct. > >> I'll change this whichever way you want. It's not a big deal, which is why >> I added the comment like you requested originally. > > I think I already "intercepted" your suggestion, see here: > > From 1a1c4addc5a14e9ef2bb7d351a62ff4563d0f10a Mon Sep 17 00:00:00 2001 > From: Jason Andryuk <jason.andryuk@amd.com> > Date: Tue, 25 Aug 2026 17:48:03 -0400 > Subject: [PATCH] x86/amd_node: Prevent potential NULL pointer dereference > > amd_smn_read/write() are exported functions around __amd_smn_rw(), so > they are always available even if amd_smn_init() fails. In that case, > amd_roots is NULL and __amd_smn_rw() will access uninitialized memory. > > Then, commit > > 83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access") > > added smn_exclusive which indicated the calls to > pci_request_config_region_exclusive() succeeded to prevent concurrent > userspace access. > > Commit > > 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching") > > re-ordered initialization so pci_request_config_region_exclusive() is called > earlier and a failure exits amd_smn_init() before allocating amd_roots. > Setting smn_exclusive moved to the end of amd_smn_init(), after amd_roots is > allocated. It became redundant and can be removed. > > Replace smn_exclusive with directly checking amd_roots to avoid a potential > NULL pointer dereference. > > [ bp: Reorg commit message, touchup comment. ] > > Fixes: 77466b798d59 ("x86/amd_node: Remove dependency on AMD_NB") > Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> > Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de> > Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com> > Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org> > Cc: stable@vger.kernel.org > Link: https://patch.msgid.link/20260825214805.39148-3-jason.andryuk@amd.com Looks good to me. Thanks, Jason ^ permalink raw reply [flat|nested] 12+ messages in thread
* [tip: x86/urgent] x86/amd_node: Prevent potential NULL pointer dereference 2026-08-25 21:48 ` [PATCH v3 2/2] x86/amd_node: Remove smn_exclusive Jason Andryuk 2026-08-31 2:30 ` Borislav Petkov @ 2026-09-02 21:32 ` tip-bot2 for Jason Andryuk 1 sibling, 0 replies; 12+ messages in thread From: tip-bot2 for Jason Andryuk @ 2026-09-02 21:32 UTC (permalink / raw) To: linux-tip-commits Cc: Jason Andryuk, Borislav Petkov (AMD), Yazen Ghannam, Mario Limonciello (AMD), stable, x86, linux-kernel The following commit has been merged into the x86/urgent branch of tip: Commit-ID: 60714fb1d494e11d1eb54b0aef45e250c51279a8 Gitweb: https://git.kernel.org/tip/60714fb1d494e11d1eb54b0aef45e250c51279a8 Author: Jason Andryuk <jason.andryuk@amd.com> AuthorDate: Tue, 25 Aug 2026 17:48:03 -04:00 Committer: Borislav Petkov (AMD) <bp@alien8.de> CommitterDate: Wed, 02 Sep 2026 13:52:15 -07:00 x86/amd_node: Prevent potential NULL pointer dereference amd_smn_read/write() are exported functions around __amd_smn_rw(), so they are always available even if amd_smn_init() fails. In that case, amd_roots is NULL and __amd_smn_rw() will access uninitialized memory. Then, commit 83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access") added smn_exclusive which indicated the calls to pci_request_config_region_exclusive() succeeded to prevent concurrent userspace access. Commit 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching") re-ordered initialization so pci_request_config_region_exclusive() is called earlier and a failure exits amd_smn_init() before allocating amd_roots. Setting smn_exclusive moved to the end of amd_smn_init(), after amd_roots is allocated. It became redundant and can be removed. Replace smn_exclusive with directly checking amd_roots to avoid a potential NULL pointer dereference. [ bp: Reorg commit message, touchup comment. ] Fixes: 77466b798d59 ("x86/amd_node: Remove dependency on AMD_NB") Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org> Cc: stable@vger.kernel.org Link: https://patch.msgid.link/20260825214805.39148-3-jason.andryuk@amd.com --- arch/x86/kernel/amd_node.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c index 408b9fd..7625857 100644 --- a/arch/x86/kernel/amd_node.c +++ b/arch/x86/kernel/amd_node.c @@ -38,7 +38,6 @@ static struct pci_dev **amd_roots; /* Protect the PCI config register pairs used for SMN. */ static DEFINE_MUTEX(smn_mutex); -static bool smn_exclusive; #define SMN_INDEX_OFFSET 0x60 #define SMN_DATA_OFFSET 0x64 @@ -91,11 +90,16 @@ static int __amd_smn_rw(u8 i_off, u8 d_off, u16 node, u32 address, u32 *value, b if (node >= amd_num_nodes()) return err; - root = amd_roots[node]; - if (!root) + /* + * Uninitialized amd_roots indicates pci_request_config_region_exclusive() + * didn't run or failed and thus the kernel cannot rely on having + * exclusive access to SMN registers so prevent that. + */ + if (!amd_roots) return err; - if (!smn_exclusive) + root = amd_roots[node]; + if (!root) return err; guard(mutex)(&smn_mutex); @@ -313,8 +317,6 @@ static int __init amd_smn_init(void) debugfs_create_file("value", 0600, debugfs_dir, NULL, &smn_value_fops); } - smn_exclusive = true; - return 0; } ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-09-02 21:32 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-08-25 21:48 [PATCH v3 0/2] x86/amd_node: Fixes for virtualized systems Jason Andryuk 2026-08-25 21:48 ` [PATCH v3 1/2] x86/amd_node: Avoid divide by zero on " Jason Andryuk 2026-08-26 4:45 ` Borislav Petkov 2026-08-26 12:15 ` Jason Andryuk 2026-08-26 15:55 ` Borislav Petkov 2026-08-31 17:35 ` [tip: x86/urgent] " tip-bot2 for Jason Andryuk 2026-08-25 21:48 ` [PATCH v3 2/2] x86/amd_node: Remove smn_exclusive Jason Andryuk 2026-08-31 2:30 ` Borislav Petkov 2026-08-31 17:52 ` Jason Andryuk 2026-09-01 0:27 ` Borislav Petkov 2026-09-01 11:59 ` Jason Andryuk 2026-09-02 21:32 ` [tip: x86/urgent] x86/amd_node: Prevent potential NULL pointer dereference tip-bot2 for Jason Andryuk
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®