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 patch checks that a NUMA node has CPUs before attempting to update its first CPU's microcode.
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 | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c index 95ac1c6a84fbe..7c06425edc1b5 100644 --- a/arch/x86/kernel/cpu/microcode/amd.c +++ b/arch/x86/kernel/cpu/microcode/amd.c @@ -1059,6 +1059,7 @@ static enum ucode_state _load_microcode_amd(u8 family, const u8 *data, size_t si
static enum ucode_state load_microcode_amd(u8 family, const u8 *data, size_t size) { + const struct cpumask *mask; struct cpuinfo_x86 *c; unsigned int nid, cpu; struct ucode_patch *p; @@ -1069,7 +1070,11 @@ static enum ucode_state load_microcode_amd(u8 family, const u8 *data, size_t siz return ret;
for_each_node(nid) { - cpu = cpumask_first(cpumask_of_node(nid)); + mask = cpumask_of_node(nid); + if (cpumask_empty(mask)) + continue; + + cpu = cpumask_first(mask); c = &cpu_data(cpu);
p = find_patch(cpu);