From: Simon Horman <horms@kernel.org> Date: 2024-08-13 14:33:01
Hi,
This series addresses several string truncation issues that are flagged
by gcc-14. I do not have any reason to believe these are bugs, so I am
targeting this at net-next and have not provided Fixes tags.
---
Changes in v2:
- Added patch:
+ bnxt_en: Extend maximum length of version string by 1 byte
- Dropped the following patches.
- bnxt_en: check for irq name truncation
- bnxt_en: check for fw_ver_str truncation
+ The approach I had taken was to return error on truncation, but the
feedback I received was that it would be better to replace the last
three bytes with "..." or some other similar scheme. While simple
enough to implement, it does add some complexity, and I have so far
been unable to convince myself that it is warranted. So I have
decided to drop these patches for now.
- Link to v1: https://lore.kernel.org/r/20240705-bnxt-str-v1-0-bafc769ed89e@kernel.org
---
Simon Horman (2):
bnxt_en: Extend maximum length of version string by 1 byte
bnxt_en: avoid truncation of per rx run debugfs filename
drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c | 4 ++--
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
base-commit: dd1bf9f9df156b43e5122f90d97ac3f59a1a5621
From: Simon Horman <horms@kernel.org> Date: 2024-08-13 14:33:04
This corrects an out-by-one error in the maximum length of the package
version string. The size argument of snprintf includes space for the
trailing '\0' byte, so there is no need to allow extra space for it by
reducing the value of the size argument by 1.
Found by inspection.
Compile tested only.
Signed-off-by: Simon Horman <horms@kernel.org>
---
v2: New patch
---
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Simon Horman <horms@kernel.org> Date: 2024-08-13 14:33:06
Although it seems unlikely in practice - there would need to be
rx ring indexes greater than 10^10 - it is theoretically possible
for the filename of per rx ring debugfs files to be truncated.
This is because although a 16 byte buffer is provided, the length
of the filename is restricted to 10 bytes. Remove this restriction
and allow the entire buffer to be used.
Also reduce the buffer to 12 bytes, which is sufficient.
Given that the range of rx ring indexes likely much smaller than the
maximum range of a 32-bit signed integer, a smaller buffer could be
used, with some further changes. But this change seems simple, robust,
and has minimal stack overhead.
Flagged by gcc-14:
.../bnxt_debugfs.c: In function 'bnxt_debug_dev_init':
drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c:69:30: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 10 [-Wformat-truncation=]
69 | snprintf(qname, 10, "%d", ring_idx);
| ^~
In function 'debugfs_dim_ring_init',
inlined from 'bnxt_debug_dev_init' at .../bnxt_debugfs.c:87:4:
.../bnxt_debugfs.c:69:29: note: directive argument in the range [-2147483643, 2147483646]
69 | snprintf(qname, 10, "%d", ring_idx);
| ^~~~
.../bnxt_debugfs.c:69:9: note: 'snprintf' output between 2 and 12 bytes into a destination of size 10
69 | snprintf(qname, 10, "%d", ring_idx);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Compile tested only
Signed-off-by: Simon Horman <horms@kernel.org>
---
v2: No change
---
drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Michael Chan <michael.chan@broadcom.com> Date: 2024-08-13 17:01:30
On Tue, Aug 13, 2024 at 7:33 AM Simon Horman [off-list ref] wrote:
This corrects an out-by-one error in the maximum length of the package
version string. The size argument of snprintf includes space for the
trailing '\0' byte, so there is no need to allow extra space for it by
reducing the value of the size argument by 1.
Found by inspection.
Compile tested only.
Signed-off-by: Simon Horman <horms@kernel.org>
Thanks.
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
From: Michael Chan <michael.chan@broadcom.com> Date: 2024-08-13 17:02:46
On Tue, Aug 13, 2024 at 7:33 AM Simon Horman [off-list ref] wrote:
Although it seems unlikely in practice - there would need to be
rx ring indexes greater than 10^10 - it is theoretically possible
for the filename of per rx ring debugfs files to be truncated.
This is because although a 16 byte buffer is provided, the length
of the filename is restricted to 10 bytes. Remove this restriction
and allow the entire buffer to be used.
Also reduce the buffer to 12 bytes, which is sufficient.
Given that the range of rx ring indexes likely much smaller than the
maximum range of a 32-bit signed integer, a smaller buffer could be
used, with some further changes. But this change seems simple, robust,
and has minimal stack overhead.
Flagged by gcc-14:
.../bnxt_debugfs.c: In function 'bnxt_debug_dev_init':
drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c:69:30: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 10 [-Wformat-truncation=]
69 | snprintf(qname, 10, "%d", ring_idx);
| ^~
In function 'debugfs_dim_ring_init',
inlined from 'bnxt_debug_dev_init' at .../bnxt_debugfs.c:87:4:
.../bnxt_debugfs.c:69:29: note: directive argument in the range [-2147483643, 2147483646]
69 | snprintf(qname, 10, "%d", ring_idx);
| ^~~~
.../bnxt_debugfs.c:69:9: note: 'snprintf' output between 2 and 12 bytes into a destination of size 10
69 | snprintf(qname, 10, "%d", ring_idx);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Compile tested only
Signed-off-by: Simon Horman <horms@kernel.org>
Thanks.
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski [off-list ref]:
On Tue, 13 Aug 2024 15:32:54 +0100 you wrote:
Hi,
This series addresses several string truncation issues that are flagged
by gcc-14. I do not have any reason to believe these are bugs, so I am
targeting this at net-next and have not provided Fixes tags.
[...]