On 1/23/25 9:39 AM, Murad Masimov wrote:
> Syzkaller has reported a warning in to_nfit_bus_uuid(): "only secondary
> bus families can be translated". This warning is emited if the argument
> is equal to NVDIMM_BUS_FAMILY_NFIT == 0. 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, 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.
>
> All checks of the input value should be applied to the original variable
> call_pkg->nd_family.
>
> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
>
> Fixes: 6450ddbd5d8e ("ACPI: NFIT: Define runtime firmware activation commands")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: syzbot+c80d8dc0d9fa81a3cd8c@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=c80d8dc0d9fa81a3cd8c
> Signed-off-by: Murad Masimov <m.masimov@xxxxxxxxxxxxxxxxx>
While the change logically makes sense, the likelihood of nd_family > int_size is not ever likely. Given that NVDIMM_BUS_FAMILY_MAX is defined as 1, I don't think we care about values greater than that regardless of what is set in the upper 32bit of the u64. I'm leaning towards the fix is unnecessary.
> ---
> 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
>
