netdev - RE: [net V2 14/15] net/mlx5e: Check return value of snprintf writing to fw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <81cae734ee1b4cde9b380a9a31006c1a@AcuMS.aculab.com>
Date: Sun, 19 Nov 2023 10:46:57 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Saeed Mahameed' <saeed@...nel.org>, "David S. Miller"
	<davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>
CC: Saeed Mahameed <saeedm@...dia.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, Tariq Toukan <tariqt@...dia.com>, Rahul Rameshbabu
	<rrameshbabu@...dia.com>, Dragos Tatulea <dtatulea@...dia.com>
Subject: RE: [net V2 14/15] net/mlx5e: Check return value of snprintf writing
 to fw_version buffer

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@...dia.com>
> Reviewed-by: Dragos Tatulea <dtatulea@...dia.com>
> Signed-off-by: Saeed Mahameed <saeedm@...dia.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))
> +		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
> 

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)