Re: [PATCH] staging: greybus: fw-management: validate firmware_tag is NUL-terminated

[Dan Carpenter <error27@gmail.com>]

On Mon, Nov 02, 2026 at 05:12:21PM +0300, Maritu Hone wrote:
> fw_mgmt_ioctl() copies struct fw_mgmt_ioc_get_backend_version,
> struct fw_mgmt_ioc_intf_load_and_validate and
> struct fw_mgmt_ioc_backend_fw_update directly from userspace via
> copy_from_user(). Each of these contains a fixed-size
> firmware_tag[GB_FIRMWARE_U_TAG_MAX_SIZE] array that is never checked
> for NUL-termination before being used.
> 
> While the array is eventually copied into a Greybus request via
> strscpy_pad(), which safely bounds its read and always NUL-terminates
> its destination, three error paths print the original, unterminated
> userspace buffer directly with "%s":
> 
>   fw_mgmt_backend_fw_version_operation():
>     dev_err(..., "failed to get version of %s backend firmware (%d)\n",
>             fw_info->firmware_tag, ret);

The strscpy_pad() function returns -E2BIG if the string is too large
so if "fw_info->firmware_tag" is too large then it will never reach this
printk().

>     dev_err(..., "Firmware with tag %s is not supported by Interface\n",
>             fw_info->firmware_tag);
> 
>   fw_mgmt_backend_fw_update_operation():
>     dev_err(..., "backend %s firmware update request failed (%d)\n",
>             tag, ret);

Same for this one as well so these are all false positives.

> 
> If a caller supplies exactly GB_FIRMWARE_U_TAG_MAX_SIZE non-NUL bytes,
> "%s" has no terminator inside the array and reads past it into
> adjacent kernel stack memory until it happens to find a zero byte,
> leaking that memory into dmesg.
> 
> Reject the ioctl up front when firmware_tag is not NUL-terminated,
> mirroring the existing check already used for the equivalent
> Greybus-module-supplied field in fw_download_find_firmware().
> 
> Fixes: 013e66537273 ("greybus: fw-management: Add firmware-management protocol driver")
> Signed-off-by: Maritu Hone <marehone12@gmail.com>

If this were a real bug then this wouldn't be the correct Fixes
tag.

regards,
dan carpenter