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]
Message-Id: <20181017.215322.873219355646098517.davem@davemloft.net>
Date:   Wed, 17 Oct 2018 21:53:22 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     natechancellor@...il.com
Cc:     Dept-GELinuxNICDev@...ium.com, netdev@...r.kernel.org,
        sudarsana.kalluru@...ium.com, Tomer.Tayar@...ium.com,
        Michal.Kalderon@...ium.com, Ariel.Elior@...ium.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: qla3xxx: Remove overflowing shift statement

From: Nathan Chancellor <natechancellor@...il.com>
Date: Fri, 12 Oct 2018 19:14:58 -0700

> Clang currently warns:
> 
> drivers/net/ethernet/qlogic/qla3xxx.c:384:24: warning: signed shift
> result (0xF00000000) requires 37 bits to represent, but 'int' only has
> 32 bits [-Wshift-overflow]
>                     ((ISP_NVRAM_MASK << 16) | qdev->eeprom_cmd_data));
>                       ~~~~~~~~~~~~~~ ^  ~~
> 1 warning generated.
> 
> The warning is certainly accurate since ISP_NVRAM_MASK is defined as
> (0x000F << 16) which is then shifted by 16, resulting in 64424509440,
> well above UINT_MAX.
> 
> Given that this is the only location in this driver where ISP_NVRAM_MASK
> is shifted again, it seems likely that ISP_NVRAM_MASK was originally
> defined without a shift and during the move of the shift to the
> definition, this statement wasn't properly removed (since ISP_NVRAM_MASK
> is used in the statenent right above this). Only the maintainers can
> confirm this since this statment has been here since the driver was
> first added to the kernel.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/127
> Signed-off-by: Nathan Chancellor <natechancellor@...il.com>

If the qlogic/cavium folks aren't even going to look at and comment
on this, that is not fair to you and your change.

Applied, thanks Nathan.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ