linux-kernel - [PATCH v2] acpi: nfit: fix narrowing conversion in acpi_nfit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20250224113546.1441-1-m.masimov@mt-integration.ru>
Date: Mon, 24 Feb 2025 14:35:46 +0300
From: Murad Masimov <m.masimov@...integration.ru>
To: Dan Williams <dan.j.williams@...el.com>
CC: Vishal Verma <vishal.l.verma@...el.com>, Dave Jiang
	<dave.jiang@...el.com>, Ira Weiny <ira.weiny@...el.com>, "Rafael J. Wysocki"
	<rafael@...nel.org>, Len Brown <lenb@...nel.org>, <nvdimm@...ts.linux.dev>,
	<linux-acpi@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<lvc-project@...uxtesting.org>, Murad Masimov <m.masimov@...integration.ru>,
	<stable@...r.kernel.org>,
	<syzbot+c80d8dc0d9fa81a3cd8c@...kaller.appspotmail.com>
Subject: [PATCH v2] acpi: nfit: fix narrowing conversion in acpi_nfit_ctl

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@...r.kernel.org
Reported-by: syzbot+c80d8dc0d9fa81a3cd8c@...kaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=c80d8dc0d9fa81a3cd8c
Signed-off-by: Murad Masimov <m.masimov@...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