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] [day] [month] [year] [list]

Date:   Thu, 14 Sep 2017 11:39:20 -0700
From:   Saeed Mahameed <saeedm@....mellanox.co.il>
To:     Leon Romanovsky <leonro@...lanox.com>
Cc:     Arnd Bergmann <arnd@...db.de>, Ilan Tayari <ilant@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Matan Barak <matanb@...lanox.com>,
        Boris Pismenny <borisp@...lanox.com>,
        Linux Netdev List <netdev@...r.kernel.org>,
        linux-rdma@...r.kernel.org,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] net/mlx5: fpga: avoid uninitialized return codes

On Thu, Sep 14, 2017 at 5:54 AM, Leon Romanovsky <leonro@...lanox.com> wrote:
> 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.

Thanks Arnd, Looks good.

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

Or simply trust the caller :), and avoid redundant code.

>         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