netdev - Re: [PATCH] net/mlx5: fpga: avoid uninitialized return codes

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: <20170914125429.GZ3405@mtr-leonro.local>
Date:   Thu, 14 Sep 2017 15:54:29 +0300
From:   Leon Romanovsky <leonro@...lanox.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Ilan Tayari <ilant@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Matan Barak <matanb@...lanox.com>,
        Boris Pismenny <borisp@...lanox.com>, netdev@...r.kernel.org,
        linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net/mlx5: fpga: avoid uninitialized return codes

On Thu, Sep 14, 2017 at 01:06:18PM +0200, Arnd Bergmann wrote:
> calling mlx5_fpga_mem_{read,write}_i2c() with a zero length on
> older compiler version such as gcc-4.6 results in a warning that
> the return code is not initialized:
>
> drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c:147:6: error: ‘err’ may be used uninitialized in this function [-Werror=uninitialized]
> drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c:126:6: error: ‘err’ may be used uninitialized in this function [-Werror=uninitialized]
>
> On newer compilers, the 'err' variable is optimized away in this
> code path and assumed to be zero when the loop completes, so we
> don't get this warning.
>
> I'm changing the function here to instead return -EINVAL for the
> case, under the assumption that it was never meant to be called
> with a zero length argument.

I agree with you that size can't be zero and this patch will fix the
warning, but if it is possible, I will prefer to have this check is
written explicitly and not implicitly.

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
index 3c11d6e2160a..0e85bebe1dad 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
@@ -69,6 +69,9 @@ static int mlx5_fpga_mem_read_i2c(struct mlx5_fpga_device *fdev, size_t size,
 	if (!fdev->mdev)
 		return -ENOTCONN;

+	if (!size)
+		return -EINVAL;
+
 	while (bytes_done < size) {
 		actual_size = min(max_size, (size - bytes_done));

Thanks

>
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82203
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
> index 3c11d6e2160a..914fb9d77a1a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
> @@ -64,7 +64,7 @@ static int mlx5_fpga_mem_read_i2c(struct mlx5_fpga_device *fdev, size_t size,
>  	size_t max_size = MLX5_FPGA_ACCESS_REG_SIZE_MAX;
>  	size_t bytes_done = 0;
>  	u8 actual_size;
> -	int err;
> +	int err = -EINVAL;
>
>  	if (!fdev->mdev)
>  		return -ENOTCONN;
> @@ -93,7 +93,7 @@ static int mlx5_fpga_mem_write_i2c(struct mlx5_fpga_device *fdev, size_t size,
>  	size_t max_size = MLX5_FPGA_ACCESS_REG_SIZE_MAX;
>  	size_t bytes_done = 0;
>  	u8 actual_size;
> -	int err;
> +	int err = -EINVAL;
>
>  	if (!fdev->mdev)
>  		return -ENOTCONN;
> --
> 2.9.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)