Syzkaller has reported a warning in to_nfit_bus_uuid():
================================================================== only secondary bus families can be translated WARNING: CPU: 0 PID: 15821 at drivers/acpi/nfit/core.c:80 to_nfit_bus_uuid+0x6f/0x90 drivers/acpi/nfit/core.c:79 Modules linked in: CPU: 0 UID: 0 PID: 15821 Comm: syz-executor579 Not tainted 6.11.0-rc7-syzkaller-00020-g8d8d276ba2fb #0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014 RIP: 0010:to_nfit_bus_uuid+0x6f/0x90 drivers/acpi/nfit/core.c:79 Call Trace: <TASK> acpi_nfit_ctl+0x8a9/0x24a0 drivers/acpi/nfit/core.c:489 __nd_ioctl drivers/nvdimm/bus.c:1186 [inline] nd_ioctl+0x184d/0x1fe0 drivers/nvdimm/bus.c:1264 vfs_ioctl fs/ioctl.c:51 [inline] __do_sys_ioctl fs/ioctl.c:907 [inline] __se_sys_ioctl+0xfc/0x170 fs/ioctl.c:893 do_syscall_x64 arch/x86/entry/common.c:52 [inline] do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83 entry_SYSCALL_64_after_hwframe+0x77/0x7f ==================================================================
This warning is triggered if the argument passed in to_nfit_bus_uuid() is equal to 0. It is important that this function expects that the argument is between 1 and NVDIMM_BUS_FAMILY_MAX. Therefore, it must be checked beforehand. However, in acpi_nfit_ctl() validity checks made before calling to_nfit_bus_uuid() are erroneous.
Function acpi_nfit_ctl() first verifies that a user-provided value call_pkg->nd_family of type u64 is not equal to 0. Then the value is converted to int (narrowing conversion), and only after that is compared to NVDIMM_BUS_FAMILY_MAX. This can lead to passing an invalid argument to acpi_nfit_ctl(), if call_pkg->nd_family is non-zero, while the lower 32 bits are zero.
Moreover, the same way zero can be passed in to_nfit_bus_uuid(), negative value also may occur. That wouldn't trigger the warning, but could lead to a wild-memory-access in test_bit(). This is achieved with a slightly modified version of the reproducer generated by Syzkaller. The crash report is as follows:
================================================================== BUG: KASAN: wild-memory-access in acpi_nfit_ctl (./arch/x86/include/asm/bitops.h:227 (discriminator 6) ./arch/x86/include/asm/bitops.h:239 (discriminator 6) ./include/asm-generic/bitops/instrumented-non-atomic.h:142 (discriminator 6) drivers/acpi/nfit/core.c:489 (discriminator 6)) Read of size 8 at addr 1fff888141379358 by task repro/681503
CPU: 0 UID: 0 PID: 681503 Comm: repro Not tainted 6.13.0-04858-g21266b8df522 #30 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 Call Trace: <TASK> dump_stack_lvl (lib/dump_stack.c:123) kasan_report (mm/kasan/report.c:604) kasan_check_range (mm/kasan/generic.c:183 mm/kasan/generic.c:189) acpi_nfit_ctl (./arch/x86/include/asm/bitops.h:227 (discriminator 6) ./arch/x86/include/asm/bitops.h:239 (discriminator 6) ./include/asm-generic/bitops/instrumented-non-atomic.h:142 (discriminator 6) drivers/acpi/nfit/core.c:489 (discriminator 6)) nd_ioctl (drivers/nvdimm/bus.c:1187 drivers/nvdimm/bus.c:1264) __x64_sys_ioctl (fs/ioctl.c:52 fs/ioctl.c:906 fs/ioctl.c:892 fs/ioctl.c:892) do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130) ==================================================================
All checks of the input value should be applied to the original variable call_pkg->nd_family. This approach is better suited for the stable branches and is much safer than replacing the type of the variable from int to u32 or u64 throughout the code, as this could potentially introduce new bugs.
Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
Fixes: 6450ddbd5d8e ("ACPI: NFIT: Define runtime firmware activation commands") Cc: stable@vger.kernel.org Reported-by: syzbot+c80d8dc0d9fa81a3cd8c@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=c80d8dc0d9fa81a3cd8c Signed-off-by: Murad Masimov m.masimov@mt-integration.ru --- v2: Add more details to the commit message.
drivers/acpi/nfit/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index a5d47819b3a4..ae035b93da08 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -485,7 +485,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, cmd_mask = nd_desc->cmd_mask; if (cmd == ND_CMD_CALL && call_pkg->nd_family) { family = call_pkg->nd_family; - if (family > NVDIMM_BUS_FAMILY_MAX || + if (call_pkg->nd_family > NVDIMM_BUS_FAMILY_MAX || !test_bit(family, &nd_desc->bus_family_mask)) return -EINVAL; family = array_index_nospec(family, -- 2.39.2