es58x_devlink.c triggers following format-truncation GCC warnings:
drivers/net/can/usb/etas_es58x/es58x_devlink.c: In function ‘es58x_devlink_info_get’:
drivers/net/can/usb/etas_es58x/es58x_devlink.c:201:41: warning: ‘%02u’ directive output may be truncated writing between 2 and 3 bytes into a region of size between 1 and 3 [-Wformat-truncation=]
201 | snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
| ^~~~
drivers/net/can/usb/etas_es58x/es58x_devlink.c:201:30: note: directive argument in the range [0, 255]
201 | snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
| ^~~~~~~~~~~~~~~~
drivers/net/can/usb/etas_es58x/es58x_devlink.c:201:3: note: ‘snprintf’ output between 9 and 12 bytes into a destination of size 9
201 | snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
202 | fw_ver->major, fw_ver->minor, fw_ver->revision);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/can/usb/etas_es58x/es58x_devlink.c:211:41: warning: ‘%02u’ directive output may be truncated writing between 2 and 3 bytes into a region of size between 1 and 3 [-Wformat-truncation=]
211 | snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
| ^~~~
drivers/net/can/usb/etas_es58x/es58x_devlink.c:211:30: note: directive argument in the range [0, 255]
211 | snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
| ^~~~~~~~~~~~~~~~
drivers/net/can/usb/etas_es58x/es58x_devlink.c:211:3: note: ‘snprintf’ output between 9 and 12 bytes into a destination of size 9
211 | snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
212 | bl_ver->major, bl_ver->minor, bl_ver->revision);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/can/usb/etas_es58x/es58x_devlink.c:221:38: warning: ‘%03u’ directive output may be truncated writing between 3 and 5 bytes into a region of size between 2 and 4 [-Wformat-truncation=]
221 | snprintf(buf, sizeof(buf), "%c%03u/%03u",
| ^~~~
drivers/net/can/usb/etas_es58x/es58x_devlink.c:221:30: note: directive argument in the range [0, 65535]
221 | snprintf(buf, sizeof(buf), "%c%03u/%03u",
| ^~~~~~~~~~~~~
drivers/net/can/usb/etas_es58x/es58x_devlink.c:221:3: note: ‘snprintf’ output between 9 and 13 bytes into a destination of size 9
221 | snprintf(buf, sizeof(buf), "%c%03u/%03u",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
222 | hw_rev->letter, hw_rev->major, hw_rev->minor);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This is not a actual bug because the scanf() parsing make sure that
the u8 are only two digits long and the u16 only three digits long.
Thus below declaration:
char buf[max(sizeof("xx.xx.xx"), sizeof("axxx/xxx"))];
allocates just what is needed to represent either of the versions.
This warning was known but ignored because, at the time of writing,
-Wformat-truncation was not present in the kernel, not even at W=3 [1].
Extend the es58x_{sw,hw}_version_is_set() functions to also check that
the versions numbers all fit in the expected number of digits. This
done, rename the functions to reflect the change and rewrite the
documentation. While doing so, also add a description of the return
value.
All those extra verification are paranoid checks but have the merit to
silence the new W=1 format-truncation warning [2].
[1] https://lore.kernel.org/all/CAMZ6Rq+K+6gbaZ35SOJcR9qQaTJ7KR0jW=XoDKFkobjhj8CHhw@xxxxxxxxxxxxxx/
[2] commit 6d4ab2e97dcf ("extrawarn: enable format and stringop overflow warnings in W=1")
Link: https://git.kernel.org/torvalds/c/6d4ab2e97dcf
Reported-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
Closes: https://lore.kernel.org/linux-can/20230914-carrousel-wrecker-720a08e173e9-mkl@xxxxxxxxxxxxxx/
Fixes: 9f06631c3f1f ("can: etas_es58x: export product information through devlink_ops::info_get()")
Signed-off-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>
---
.../net/can/usb/etas_es58x/es58x_devlink.c | 38 +++++++++++++------
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/drivers/net/can/usb/etas_es58x/es58x_devlink.c b/drivers/net/can/usb/etas_es58x/es58x_devlink.c
index 9fba29e2f57c..cb5b55a412d6 100644
--- a/drivers/net/can/usb/etas_es58x/es58x_devlink.c
+++ b/drivers/net/can/usb/etas_es58x/es58x_devlink.c
@@ -150,29 +150,43 @@ void es58x_parse_product_info(struct es58x_device *es58x_dev)
}
/**
- * es58x_sw_version_is_set() - Check if the version is a valid number.
+ * es58x_check_sw_version() - Check if the version is a valid number.
* @sw_ver: Version number of either the firmware or the bootloader.
*
- * If &es58x_sw_version.major, &es58x_sw_version.minor and
- * &es58x_sw_version.revision are all zero, the product string could
- * not be parsed and the version number is invalid.
+ * If any of the software version sub-numbers do not fit on two
+ * digits, a bug occurred and the version is corrupted.
+ *
+ * If all of the version sub-numbers are zero, the product string
+ * could not be parsed and the version number is invalid.
+ *
+ * Return: @true if the sotware version is valid and set, @false if
+ * it is not set or corrupted.
*/
-static inline bool es58x_sw_version_is_set(struct es58x_sw_version *sw_ver)
+static inline bool es58x_check_sw_version(struct es58x_sw_version *sw_ver)
{
- return sw_ver->major || sw_ver->minor || sw_ver->revision;
+ return (sw_ver->major < 100 && sw_ver->minor < 100 &&
+ sw_ver->revision < 100) &&
+ (sw_ver->major || sw_ver->minor || sw_ver->revision);
}
/**
- * es58x_hw_revision_is_set() - Check if the revision is a valid number.
+ * es58x_check_hw_revision() - Check if the revision is a valid number.
* @hw_rev: Revision number of the hardware.
*
+ * If any of the hardware version sub-numbers do not fit on three
+ * digits, a bug occurred and the version is corrupted.
+ *
* If &es58x_hw_revision.letter is the null character, the product
* string could not be parsed and the hardware revision number is
* invalid.
+ *
+ * Return: @true if the hardware version is valid and set, @false if
+ * it is not set or corrupted.
*/
-static inline bool es58x_hw_revision_is_set(struct es58x_hw_revision *hw_rev)
+static inline bool es58x_check_hw_revision(struct es58x_hw_revision *hw_rev)
{
- return hw_rev->letter != '\0';
+ return (hw_rev->major < 1000 && hw_rev->minor < 1000) &&
+ hw_rev->letter != '\0';
}
/**
@@ -197,7 +211,7 @@ static int es58x_devlink_info_get(struct devlink *devlink,
char buf[max(sizeof("xx.xx.xx"), sizeof("axxx/xxx"))];
int ret = 0;
- if (es58x_sw_version_is_set(fw_ver)) {
+ if (es58x_check_sw_version(fw_ver)) {
snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
fw_ver->major, fw_ver->minor, fw_ver->revision);
ret = devlink_info_version_running_put(req,
@@ -207,7 +221,7 @@ static int es58x_devlink_info_get(struct devlink *devlink,
return ret;
}
- if (es58x_sw_version_is_set(bl_ver)) {
+ if (es58x_check_sw_version(bl_ver)) {
snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
bl_ver->major, bl_ver->minor, bl_ver->revision);
ret = devlink_info_version_running_put(req,
@@ -217,7 +231,7 @@ static int es58x_devlink_info_get(struct devlink *devlink,
return ret;
}
- if (es58x_hw_revision_is_set(hw_rev)) {
+ if (es58x_check_hw_revision(hw_rev)) {
snprintf(buf, sizeof(buf), "%c%03u/%03u",
hw_rev->letter, hw_rev->major, hw_rev->minor);
ret = devlink_info_version_fixed_put(req,
--
2.25.1
