[PATCH] qede: Prevent possible snprintf() truncation by bounding %s string format

[PATCH] qede: Prevent possible snprintf() truncation by bounding %s string format

[Baran Tuna]

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

Re: [PATCH] qede: Prevent possible snprintf() truncation by bounding %s string format

[Breno Leitao]

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

Re: [PATCH] qede: Prevent possible snprintf() truncation by bounding %s string format

[Baran TUna]

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

Re: [PATCH] qede: Prevent possible snprintf() truncation by bounding %s string format

[David Laight]

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