Re: [PATCH] acpi: nfit: fix narrowing conversion in acpi_nfit_ctl

[Dave Jiang <dave.jiang@intel.com>]

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@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>

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
>