Currently, load_microcode_amd() iterates over all NUMA nodes, retrieves their CPU masks and unconditonally accesses per-CPU data for the first CPU of each mask.
According to Documentation/admin-guide/mm/numaperf.rst: "Some memory may share the same node as a CPU, and others are provided as memory only nodes." Therefore, some node CPU masks may be empty and wouldn't have a "first CPU".
On a machine with far memory (and therefore CPU-less NUMA nodes): - cpumask_of_node(nid) is 0 - cpumask_first(0) is CONFIG_NR_CPUS - cpu_data(CONFIG_NR_CPUS) accesses the cpu_info per-CPU array at an index that is 1 out of bounds
This does not have any security implications since flashing microcode is a privileged operation but I believe this has reliability implications by potentially corrupting memory while flashing a microcode update.
When booting with CONFIG_UBSAN_BOUNDS=y on an AMD machine that flashes a microcode update. I get the following splat:
UBSAN: array-index-out-of-bounds in arch/x86/kernel/cpu/microcode/amd.c:X:Y index 512 is out of range for type 'unsigned long[512]' [...] Call Trace: dump_stack+0xdb/0x143 __ubsan_handle_out_of_bounds+0xf5/0x120 load_microcode_amd+0x58f/0x6b0 request_microcode_amd+0x17c/0x250 reload_store+0x174/0x2b0 kernfs_fop_write_iter+0x227/0x2d0 vfs_write+0x322/0x510 ksys_write+0xb5/0x160 do_syscall_64+0x6b/0xa0 entry_SYSCALL_64_after_hwframe+0x67/0xd1
This changes the iteration to only loop on NUMA nodes which have CPUs before attempting to update their microcodes.
Fixes: 7ff6edf4fef3 ("x86/microcode/AMD: Fix mixed steppings support") Signed-off-by: Florent Revest revest@chromium.org Cc: stable@vger.kernel.org --- arch/x86/kernel/cpu/microcode/amd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c index 95ac1c6a84fbe..d1e74bfe130f8 100644 --- a/arch/x86/kernel/cpu/microcode/amd.c +++ b/arch/x86/kernel/cpu/microcode/amd.c @@ -1068,7 +1068,7 @@ static enum ucode_state load_microcode_amd(u8 family, const u8 *data, size_t siz if (ret != UCODE_OK) return ret;
- for_each_node(nid) { + for_each_node_with_cpus(nid) { cpu = cpumask_first(cpumask_of_node(nid)); c = &cpu_data(cpu);
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: e3e89178a9f4a80092578af3ff3c8478f9187d59 Gitweb: https://git.kernel.org/tip/e3e89178a9f4a80092578af3ff3c8478f9187d59 Author: Florent Revest revest@chromium.org AuthorDate: Mon, 10 Mar 2025 15:42:43 +01:00 Committer: Borislav Petkov (AMD) bp@alien8.de CommitterDate: Mon, 10 Mar 2025 16:02:54 +01:00
x86/microcode/AMD: Fix out-of-bounds on systems with CPU-less NUMA nodes
Currently, load_microcode_amd() iterates over all NUMA nodes, retrieves their CPU masks and unconditionally accesses per-CPU data for the first CPU of each mask.
According to Documentation/admin-guide/mm/numaperf.rst:
"Some memory may share the same node as a CPU, and others are provided as memory only nodes."
Therefore, some node CPU masks may be empty and wouldn't have a "first CPU".
On a machine with far memory (and therefore CPU-less NUMA nodes): - cpumask_of_node(nid) is 0 - cpumask_first(0) is CONFIG_NR_CPUS - cpu_data(CONFIG_NR_CPUS) accesses the cpu_info per-CPU array at an index that is 1 out of bounds
This does not have any security implications since flashing microcode is a privileged operation but I believe this has reliability implications by potentially corrupting memory while flashing a microcode update.
When booting with CONFIG_UBSAN_BOUNDS=y on an AMD machine that flashes a microcode update. I get the following splat:
UBSAN: array-index-out-of-bounds in arch/x86/kernel/cpu/microcode/amd.c:X:Y index 512 is out of range for type 'unsigned long[512]' [...] Call Trace: dump_stack __ubsan_handle_out_of_bounds load_microcode_amd request_microcode_amd reload_store kernfs_fop_write_iter vfs_write ksys_write do_syscall_64 entry_SYSCALL_64_after_hwframe
Change the loop to go over only NUMA nodes which have CPUs before determining whether the first CPU on the respective node needs microcode update.
[ bp: Massage commit message, fix typo. ]
Fixes: 7ff6edf4fef3 ("x86/microcode/AMD: Fix mixed steppings support") Signed-off-by: Florent Revest revest@chromium.org Signed-off-by: Borislav Petkov (AMD) bp@alien8.de Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20250310144243.861978-1-revest@chromium.org --- arch/x86/kernel/cpu/microcode/amd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c index c69b1bc..138689b 100644 --- a/arch/x86/kernel/cpu/microcode/amd.c +++ b/arch/x86/kernel/cpu/microcode/amd.c @@ -1074,7 +1074,7 @@ static enum ucode_state load_microcode_amd(u8 family, const u8 *data, size_t siz if (ret != UCODE_OK) return ret;
- for_each_node(nid) { + for_each_node_with_cpus(nid) { cpu = cpumask_first(cpumask_of_node(nid)); c = &cpu_data(cpu);
[ Sasha's backport helper bot ]
Hi,
Summary of potential issues: ❌ Build failures detected ⚠️ Found matching upstream commit but patch is missing proper reference to it
Found matching upstream commit: e3e89178a9f4a80092578af3ff3c8478f9187d59
Note: The patch differs from the upstream commit: --- 1: e3e89178a9f4a ! 1: dc33caa61204c x86/microcode/AMD: Fix out-of-bounds on systems with CPU-less NUMA nodes @@ Metadata ## Commit message ## x86/microcode/AMD: Fix out-of-bounds on systems with CPU-less NUMA nodes
- Currently, load_microcode_amd() iterates over all NUMA nodes, retrieves their - CPU masks and unconditionally accesses per-CPU data for the first CPU of each - mask. + Currently, load_microcode_amd() iterates over all NUMA nodes, retrieves + their CPU masks and unconditonally accesses per-CPU data for the first + CPU of each mask.
- According to Documentation/admin-guide/mm/numaperf.rst: - - "Some memory may share the same node as a CPU, and others are provided as - memory only nodes." - - Therefore, some node CPU masks may be empty and wouldn't have a "first CPU". + According to Documentation/admin-guide/mm/numaperf.rst: "Some memory may + share the same node as a CPU, and others are provided as memory only + nodes." Therefore, some node CPU masks may be empty and wouldn't have a + "first CPU".
On a machine with far memory (and therefore CPU-less NUMA nodes): - cpumask_of_node(nid) is 0 @@ Commit message index that is 1 out of bounds
This does not have any security implications since flashing microcode is - a privileged operation but I believe this has reliability implications by - potentially corrupting memory while flashing a microcode update. - - When booting with CONFIG_UBSAN_BOUNDS=y on an AMD machine that flashes - a microcode update. I get the following splat: - - UBSAN: array-index-out-of-bounds in arch/x86/kernel/cpu/microcode/amd.c:X:Y - index 512 is out of range for type 'unsigned long[512]' - [...] - Call Trace: - dump_stack - __ubsan_handle_out_of_bounds - load_microcode_amd - request_microcode_amd - reload_store - kernfs_fop_write_iter - vfs_write - ksys_write - do_syscall_64 - entry_SYSCALL_64_after_hwframe - - Change the loop to go over only NUMA nodes which have CPUs before determining - whether the first CPU on the respective node needs microcode update. - - [ bp: Massage commit message, fix typo. ] + a privileged operation but I believe this has reliability implications + by potentially corrupting memory while flashing a microcode update. + + When booting with CONFIG_UBSAN_BOUNDS=y on an AMD machine that flashes a + microcode update. I get the following splat: + + UBSAN: array-index-out-of-bounds in arch/x86/kernel/cpu/microcode/amd.c:X:Y + index 512 is out of range for type 'unsigned long[512]' + [...] + Call Trace: + dump_stack+0xdb/0x143 + __ubsan_handle_out_of_bounds+0xf5/0x120 + load_microcode_amd+0x58f/0x6b0 + request_microcode_amd+0x17c/0x250 + reload_store+0x174/0x2b0 + kernfs_fop_write_iter+0x227/0x2d0 + vfs_write+0x322/0x510 + ksys_write+0xb5/0x160 + do_syscall_64+0x6b/0xa0 + entry_SYSCALL_64_after_hwframe+0x67/0xd1 + + This changes the iteration to only loop on NUMA nodes which have CPUs + before attempting to update their microcodes.
Fixes: 7ff6edf4fef3 ("x86/microcode/AMD: Fix mixed steppings support") Signed-off-by: Florent Revest revest@chromium.org - Signed-off-by: Borislav Petkov (AMD) bp@alien8.de Cc: stable@vger.kernel.org - Link: https://lore.kernel.org/r/20250310144243.861978-1-revest@chromium.org
## arch/x86/kernel/cpu/microcode/amd.c ## @@ arch/x86/kernel/cpu/microcode/amd.c: static enum ucode_state load_microcode_amd(u8 family, const u8 *data, size_t siz ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.13.y | Success | Success | | stable/linux-6.12.y | Success | Success | | stable/linux-6.6.y | Success | Success | | stable/linux-6.1.y | Failed | N/A | | stable/linux-5.15.y | Failed | N/A | | stable/linux-5.10.y | Failed | N/A | | stable/linux-5.4.y | Failed | N/A |
Build Errors: Patch failed to apply on stable/linux-6.1.y. Reject:
diff a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c (rejected hunks) @@ -1068,7 +1068,7 @@ static enum ucode_state load_microcode_amd(u8 family, const u8 *data, size_t siz if (ret != UCODE_OK) return ret;
- for_each_node(nid) { + for_each_node_with_cpus(nid) { cpu = cpumask_first(cpumask_of_node(nid)); c = &cpu_data(cpu);
Patch failed to apply on stable/linux-5.15.y. Reject:
diff a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c (rejected hunks) @@ -1068,7 +1068,7 @@ static enum ucode_state load_microcode_amd(u8 family, const u8 *data, size_t siz if (ret != UCODE_OK) return ret;
- for_each_node(nid) { + for_each_node_with_cpus(nid) { cpu = cpumask_first(cpumask_of_node(nid)); c = &cpu_data(cpu);
Patch failed to apply on stable/linux-5.10.y. Reject:
diff a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c (rejected hunks) @@ -1068,7 +1068,7 @@ static enum ucode_state load_microcode_amd(u8 family, const u8 *data, size_t siz if (ret != UCODE_OK) return ret;
- for_each_node(nid) { + for_each_node_with_cpus(nid) { cpu = cpumask_first(cpumask_of_node(nid)); c = &cpu_data(cpu);
Patch failed to apply on stable/linux-5.4.y. Reject:
diff a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c (rejected hunks) @@ -1068,7 +1068,7 @@ static enum ucode_state load_microcode_amd(u8 family, const u8 *data, size_t siz if (ret != UCODE_OK) return ret;
- for_each_node(nid) { + for_each_node_with_cpus(nid) { cpu = cpumask_first(cpumask_of_node(nid)); c = &cpu_data(cpu);
linux-stable-mirror@lists.linaro.org
-
Florent Revest -
Sasha Levin -
tip-bot2 for Florent Revest