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

[Maritu Hone]

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);
    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);

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@xxxxxxxxx>
---
 drivers/staging/greybus/fw-management.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/staging/greybus/fw-management.c b/drivers/staging/greybus/fw-management.c
index 5d01da6b6eb9..2ee609dd67fb 100644
--- a/drivers/staging/greybus/fw-management.c
+++ b/drivers/staging/greybus/fw-management.c
@@ -419,6 +419,11 @@ static int fw_mgmt_ioctl(struct fw_mgmt *fw_mgmt, unsigned int cmd,
 				   sizeof(backend_fw_info)))
 			return -EFAULT;
 
+		if (strnlen(backend_fw_info.firmware_tag,
+			    GB_FIRMWARE_U_TAG_MAX_SIZE) ==
+		    GB_FIRMWARE_U_TAG_MAX_SIZE)
+			return -EINVAL;
+
 		ret = fw_mgmt_backend_fw_version_operation(fw_mgmt,
 							   &backend_fw_info);
 		if (ret)
@@ -433,6 +438,11 @@ static int fw_mgmt_ioctl(struct fw_mgmt *fw_mgmt, unsigned int cmd,
 		if (copy_from_user(&intf_load, buf, sizeof(intf_load)))
 			return -EFAULT;
 
+		if (strnlen(intf_load.firmware_tag,
+			    GB_FIRMWARE_U_TAG_MAX_SIZE) ==
+		    GB_FIRMWARE_U_TAG_MAX_SIZE)
+			return -EINVAL;
+
 		ret = fw_mgmt_load_and_validate_operation(fw_mgmt,
 				intf_load.load_method, intf_load.firmware_tag);
 		if (ret)
@@ -457,6 +467,11 @@ static int fw_mgmt_ioctl(struct fw_mgmt *fw_mgmt, unsigned int cmd,
 				   sizeof(backend_update)))
 			return -EFAULT;
 
+		if (strnlen(backend_update.firmware_tag,
+			    GB_FIRMWARE_U_TAG_MAX_SIZE) ==
+		    GB_FIRMWARE_U_TAG_MAX_SIZE)
+			return -EINVAL;
+
 		ret = fw_mgmt_backend_fw_update_operation(fw_mgmt,
 							  backend_update.firmware_tag);
 		if (ret)
-- 
2.43.0