* [PATCH] qede: Prevent possible snprintf() truncation by bounding %s string format
@ 2026-07-01 14:47 Baran Tuna
2026-07-01 15:27 ` Breno Leitao
2026-07-01 19:33 ` David Laight
0 siblings, 2 replies; 4+ messages in thread
From: Baran Tuna @ 2026-07-01 14:47 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Baran Tuna, Breno Leitao,
open list:QLOGIC QL4xxx ETHERNET DRIVER, open list
GCC warning shows that formatted strings may
exceed the fixed-size destination buffers.
Bounding the %s string format
so the maximum formatted output always fits.
This eliminates the -Wformat-truncation warning.
Signed-off-by: Baran Tuna <barant@fastmail.com>
---
drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index 647f30a16a94..5428f53150a0 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -618,10 +618,10 @@ static void qede_get_drvinfo(struct net_device *ndev,
if ((strlen(storm) + strlen("[storm]")) <
sizeof(info->version))
snprintf(info->version, sizeof(info->version),
- "[storm %s]", storm);
+ "[storm %.16s]", storm);
else
snprintf(info->version, sizeof(info->version),
- "%s", storm);
+ "%.16s", storm);
if (edev->dev_info.common.mbi_version) {
snprintf(mbi, ETHTOOL_FWVERS_LEN, "%d.%d.%d",
@@ -632,10 +632,10 @@ static void qede_get_drvinfo(struct net_device *ndev,
(edev->dev_info.common.mbi_version &
QED_MBI_VERSION_0_MASK) >> QED_MBI_VERSION_0_OFFSET);
snprintf(info->fw_version, sizeof(info->fw_version),
- "mbi %s [mfw %s]", mbi, mfw);
+ "mbi %.10s [mfw %.10s]", mbi, mfw);
} else {
snprintf(info->fw_version, sizeof(info->fw_version),
- "mfw %s", mfw);
+ "mfw %.16s", mfw);
}
strscpy(info->bus_info, pci_name(edev->pdev), sizeof(info->bus_info));
--
2.54.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] qede: Prevent possible snprintf() truncation by bounding %s string format
2026-07-01 14:47 [PATCH] qede: Prevent possible snprintf() truncation by bounding %s string format Baran Tuna
@ 2026-07-01 15:27 ` Breno Leitao
2026-07-01 16:23 ` Baran TUna
2026-07-01 19:33 ` David Laight
1 sibling, 1 reply; 4+ messages in thread
From: Breno Leitao @ 2026-07-01 15:27 UTC (permalink / raw)
To: Baran Tuna
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, open list:QLOGIC QL4xxx ETHERNET DRIVER, open list
On Wed, Jul 01, 2026 at 05:47:11PM +0300, Baran Tuna wrote:
> GCC warning shows that formatted strings may
> exceed the fixed-size destination buffers.
>
> Bounding the %s string format
> so the maximum formatted output always fits.
>
> This eliminates the -Wformat-truncation warning.
>
> Signed-off-by: Baran Tuna <barant@fastmail.com>
> ---
> drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> index 647f30a16a94..5428f53150a0 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> @@ -618,10 +618,10 @@ static void qede_get_drvinfo(struct net_device *ndev,
> if ((strlen(storm) + strlen("[storm]")) <
> sizeof(info->version))
> snprintf(info->version, sizeof(info->version),
> - "[storm %s]", storm);
> + "[storm %.16s]", storm);
Where is this 16 coming from?
Also, isn't the if above checking for no overflow? I.e,
we got here only if strlen(storm) + strlen("[storm]") < sizeof(info->version))
For whoever else is reviwewing this, this the buffers:
#define ETHTOOL_FWVERS_LEN 32
char version[32];
char storm[ETHTOOL_FWVERS_LEN];
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] qede: Prevent possible snprintf() truncation by bounding %s string format
2026-07-01 15:27 ` Breno Leitao
@ 2026-07-01 16:23 ` Baran TUna
0 siblings, 0 replies; 4+ messages in thread
From: Baran TUna @ 2026-07-01 16:23 UTC (permalink / raw)
To: Breno Leitao
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, open list:QLOGIC QL4xxx ETHERNET DRIVER, open list
The current solution is pretty arbitrary.
Numbers are coming from a simple calculation, to make sure output always
fits.
I will take a further look and send a patch if there is a more
generalized solution.
On 7/1/26 6:27 PM, Breno Leitao wrote:
> On Wed, Jul 01, 2026 at 05:47:11PM +0300, Baran Tuna wrote:
>> GCC warning shows that formatted strings may
>> exceed the fixed-size destination buffers.
>>
>> Bounding the %s string format
>> so the maximum formatted output always fits.
>>
>> This eliminates the -Wformat-truncation warning.
>>
>> Signed-off-by: Baran Tuna <barant@fastmail.com>
>> ---
>> drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
>> index 647f30a16a94..5428f53150a0 100644
>> --- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
>> +++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
>> @@ -618,10 +618,10 @@ static void qede_get_drvinfo(struct net_device *ndev,
>> if ((strlen(storm) + strlen("[storm]")) <
>> sizeof(info->version))
>> snprintf(info->version, sizeof(info->version),
>> - "[storm %s]", storm);
>> + "[storm %.16s]", storm);
> Where is this 16 coming from?
>
> Also, isn't the if above checking for no overflow? I.e,
> we got here only if strlen(storm) + strlen("[storm]") < sizeof(info->version))
>
> For whoever else is reviwewing this, this the buffers:
>
> #define ETHTOOL_FWVERS_LEN 32
> char version[32];
> char storm[ETHTOOL_FWVERS_LEN];
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] qede: Prevent possible snprintf() truncation by bounding %s string format
2026-07-01 14:47 [PATCH] qede: Prevent possible snprintf() truncation by bounding %s string format Baran Tuna
2026-07-01 15:27 ` Breno Leitao
@ 2026-07-01 19:33 ` David Laight
1 sibling, 0 replies; 4+ messages in thread
From: David Laight @ 2026-07-01 19:33 UTC (permalink / raw)
To: Baran Tuna
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Breno Leitao,
open list:QLOGIC QL4xxx ETHERNET DRIVER, open list
On Wed, 1 Jul 2026 17:47:11 +0300
Baran Tuna <barant@fastmail.com> wrote:
> GCC warning shows that formatted strings may
> exceed the fixed-size destination buffers.
>
> Bounding the %s string format
> so the maximum formatted output always fits.
>
> This eliminates the -Wformat-truncation warning.
>
> Signed-off-by: Baran Tuna <barant@fastmail.com>
> ---
> drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> index 647f30a16a94..5428f53150a0 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> @@ -618,10 +618,10 @@ static void qede_get_drvinfo(struct net_device *ndev,
> if ((strlen(storm) + strlen("[storm]")) <
> sizeof(info->version))
> snprintf(info->version, sizeof(info->version),
> - "[storm %s]", storm);
> + "[storm %.16s]", storm);
> else
> snprintf(info->version, sizeof(info->version),
> - "%s", storm);
> + "%.16s", storm);
That looks wrong.
The code is using two different formats based on the length
of 'storm' but you are truncating it to the same length in
both cases.
I think this will work:
if (snprintf(info->version, sizeof(info->version),
"[storm %s]", storm) >= sizeof(info->strorm))
strscpy(info->version, storm);
-- David
>
> if (edev->dev_info.common.mbi_version) {
> snprintf(mbi, ETHTOOL_FWVERS_LEN, "%d.%d.%d",
> @@ -632,10 +632,10 @@ static void qede_get_drvinfo(struct net_device *ndev,
> (edev->dev_info.common.mbi_version &
> QED_MBI_VERSION_0_MASK) >> QED_MBI_VERSION_0_OFFSET);
> snprintf(info->fw_version, sizeof(info->fw_version),
> - "mbi %s [mfw %s]", mbi, mfw);
> + "mbi %.10s [mfw %.10s]", mbi, mfw);
> } else {
> snprintf(info->fw_version, sizeof(info->fw_version),
> - "mfw %s", mfw);
> + "mfw %.16s", mfw);
> }
>
> strscpy(info->bus_info, pci_name(edev->pdev), sizeof(info->bus_info));
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-01 19:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-01 14:47 [PATCH] qede: Prevent possible snprintf() truncation by bounding %s string format Baran Tuna
2026-07-01 15:27 ` Breno Leitao
2026-07-01 16:23 ` Baran TUna
2026-07-01 19:33 ` David Laight
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox