From: Rahul Rameshbabu <rrameshbabu@nvidia•com>
To: David Laight <David.Laight@ACULAB•COM>
Cc: 'Saeed Mahameed' <saeed@kernel•org>,
"David S. Miller" <davem@davemloft•net>,
Jakub Kicinski <kuba@kernel•org>,
Paolo Abeni <pabeni@redhat•com>,
Eric Dumazet <edumazet@google•com>,
Saeed Mahameed <saeedm@nvidia•com>,
"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
Tariq Toukan <tariqt@nvidia•com>,
Dragos Tatulea <dtatulea@nvidia•com>
Subject: Re: [net V2 14/15] net/mlx5e: Check return value of snprintf writing to fw_version buffer
Date: Sun, 19 Nov 2023 10:54:00 -0800 [thread overview]
Message-ID: <87v89xbmlz.fsf@nvidia.com> (raw)
In-Reply-To: <81cae734ee1b4cde9b380a9a31006c1a@AcuMS.aculab.com> (David Laight's message of "Sun, 19 Nov 2023 10:46:57 +0000")
On Sun, 19 Nov, 2023 10:46:57 +0000 David Laight <David.Laight@ACULAB•COM> wrote:
> From: Saeed Mahameed
>> Sent: 14 November 2023 21:59
>>
>> Treat the operation as an error case when the return value is equivalent to
>> the size of the name buffer. Failed to write null terminator to the name
>> buffer, making the string malformed and should not be used. Provide a
>> string with only the firmware version when forming the string with the
>> board id fails.
>
> Nak.
>
> RTFM snprintf().
>
>>
>> Without check, will trigger -Wformat-truncation with W=1.
>>
>> drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c: In function 'mlx5e_ethtool_get_drvinfo':
>> drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c:49:31: warning: '%.16s' directive output may
>> be truncated writing up to 16 bytes into a region of size between 13 and 22 [-Wformat-truncation=]
>> 49 | "%d.%d.%04d (%.16s)",
>> | ^~~~~
>> drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c:48:9: note: 'snprintf' output between 12 and
>> 37 bytes into a destination of size 32
>> 48 | snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> 49 | "%d.%d.%04d (%.16s)",
>> | ~~~~~~~~~~~~~~~~~~~~~
>> 50 | fw_rev_maj(mdev), fw_rev_min(mdev), fw_rev_sub(mdev),
>> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> 51 | mdev->board_id);
>> | ~~~~~~~~~~~~~~~
>>
>> Fixes: 84e11edb71de ("net/mlx5e: Show board id in ethtool driver information")
>> Link:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6d4ab2e97dcfbcd748ae7176
>> 1a9d8e5e41cc732c
>> Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia•com>
>> Reviewed-by: Dragos Tatulea <dtatulea@nvidia•com>
>> Signed-off-by: Saeed Mahameed <saeedm@nvidia•com>
>> ---
>> .../net/ethernet/mellanox/mlx5/core/en_ethtool.c | 13 +++++++++----
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> index 215261a69255..792a0ea544cd 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> @@ -43,12 +43,17 @@ void mlx5e_ethtool_get_drvinfo(struct mlx5e_priv *priv,
>> struct ethtool_drvinfo *drvinfo)
>> {
>> struct mlx5_core_dev *mdev = priv->mdev;
>> + int count;
>>
>> strscpy(drvinfo->driver, KBUILD_MODNAME, sizeof(drvinfo->driver));
>> - snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
>> - "%d.%d.%04d (%.16s)",
>> - fw_rev_maj(mdev), fw_rev_min(mdev), fw_rev_sub(mdev),
>> - mdev->board_id);
>> + count = snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
>> + "%d.%d.%04d (%.16s)", fw_rev_maj(mdev),
>> + fw_rev_min(mdev), fw_rev_sub(mdev), mdev->board_id);
>> + if (count == sizeof(drvinfo->fw_version))
This should be >= now that I think about it.
From the kernel docs: If the return is greater than or equal to size,
the resulting string is truncated.
The return value *can* be greater than the size parameter expressing
what the length would have been.
https://docs.kernel.org/core-api/kernel-api.html#c.snprintf
>> + snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
>> + "%d.%d.%04d", fw_rev_maj(mdev),
>> + fw_rev_min(mdev), fw_rev_sub(mdev));
>> +
>> strscpy(drvinfo->bus_info, dev_name(mdev->device),
>> sizeof(drvinfo->bus_info));
>> }
>> --
>> 2.41.0
>>
--
Thanks,
Rahul Rameshbabu
next prev parent reply other threads:[~2023-11-19 18:54 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-14 21:58 [pull request][net V2 00/15] mlx5 fixes 2023-11-13 Saeed Mahameed
2023-11-14 21:58 ` [net V2 01/15] Revert "net/mlx5: DR, Supporting inline WQE when possible" Saeed Mahameed
2023-11-16 6:40 ` patchwork-bot+netdevbpf
2023-11-14 21:58 ` [net V2 02/15] net/mlx5: Free used cpus mask when an IRQ is released Saeed Mahameed
2023-11-14 21:58 ` [net V2 03/15] net/mlx5: DR, Allow old devices to use multi destination FTE Saeed Mahameed
2023-11-14 21:58 ` [net V2 04/15] net/mlx5: Decouple PHC .adjtime and .adjphase implementations Saeed Mahameed
2023-11-14 21:58 ` [net V2 05/15] net/mlx5e: fix double free of encap_header Saeed Mahameed
2023-11-16 6:35 ` Jakub Kicinski
2023-11-14 21:58 ` [net V2 06/15] net/mlx5e: fix double free of encap_header in update funcs Saeed Mahameed
2023-11-14 21:58 ` [net V2 07/15] net/mlx5e: Fix pedit endianness Saeed Mahameed
2023-11-14 21:58 ` [net V2 08/15] net/mlx5e: Don't modify the peer sent-to-vport rules for IPSec offload Saeed Mahameed
2023-11-14 21:58 ` [net V2 09/15] net/mlx5e: Avoid referencing skb after free-ing in drop path of mlx5e_sq_xmit_wqe Saeed Mahameed
2023-11-14 21:58 ` [net V2 10/15] net/mlx5e: Track xmit submission to PTP WQ after populating metadata map Saeed Mahameed
2023-11-14 21:58 ` [net V2 11/15] net/mlx5e: Update doorbell for port timestamping CQ before the software counter Saeed Mahameed
2023-11-14 21:58 ` [net V2 12/15] net/mlx5: Increase size of irq name buffer Saeed Mahameed
2023-11-14 21:58 ` [net V2 13/15] net/mlx5e: Reduce the size of icosq_str Saeed Mahameed
2023-11-14 21:58 ` [net V2 14/15] net/mlx5e: Check return value of snprintf writing to fw_version buffer Saeed Mahameed
2023-11-19 10:46 ` David Laight
2023-11-19 18:54 ` Rahul Rameshbabu [this message]
2023-11-14 21:58 ` [net V2 15/15] net/mlx5e: Check return value of snprintf writing to fw_version buffer for representors Saeed Mahameed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87v89xbmlz.fsf@nvidia.com \
--to=rrameshbabu@nvidia$(echo .)com \
--cc=David.Laight@ACULAB$(echo .)COM \
--cc=davem@davemloft$(echo .)net \
--cc=dtatulea@nvidia$(echo .)com \
--cc=edumazet@google$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=saeed@kernel$(echo .)org \
--cc=saeedm@nvidia$(echo .)com \
--cc=tariqt@nvidia$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox