linux-kernel - [PATCH] net: qla3xxx: Remove overflowing shift statement

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

Message-Id: <20181013021457.13608-1-natechancellor@gmail.com>
Date:   Fri, 12 Oct 2018 19:14:58 -0700
From:   Nathan Chancellor <natechancellor@...il.com>
To:     Dept-GELinuxNICDev@...ium.com,
        "David S. Miller" <davem@...emloft.net>
Cc:     netdev@...r.kernel.org,
        Sudarsana Reddy Kalluru <sudarsana.kalluru@...ium.com>,
        Tomer Tayar <Tomer.Tayar@...ium.com>,
        Michal Kalderon <Michal.Kalderon@...ium.com>,
        Ariel Elior <Ariel.Elior@...ium.com>,
        linux-kernel@...r.kernel.org,
        Nathan Chancellor <natechancellor@...il.com>
Subject: [PATCH] net: qla3xxx: Remove overflowing shift statement

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

I sent this diff in a RFC email to the email in the MAINTAINERS file
with no response. Given that this driver has not received any actual
Cavium/QLogic treatment since at least the moving of this file in
commit aa43c2158d5a ("qlogic: Move the QLogic drivers") back in 2011,
I'd be curious to see who is actually using this driver but I'll leave
that for another time.

I've added a few of the active 'drivers/net/ethernet/qlogic' Cavium
developers (hope they don't mind) to see if this makes any sense since I
don't have the hardware to test and I know they're active.

 drivers/net/ethernet/qlogic/qla3xxx.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qla3xxx.c b/drivers/net/ethernet/qlogic/qla3xxx.c
index b48f76182049..10b075bc5959 100644
--- a/drivers/net/ethernet/qlogic/qla3xxx.c
+++ b/drivers/net/ethernet/qlogic/qla3xxx.c
@@ -380,8 +380,6 @@ static void fm93c56a_select(struct ql3_adapter *qdev)

 	qdev->eeprom_cmd_data = AUBURN_EEPROM_CS_1;
 	ql_write_nvram_reg(qdev, spir, ISP_NVRAM_MASK | qdev->eeprom_cmd_data);
-	ql_write_nvram_reg(qdev, spir,
-			   ((ISP_NVRAM_MASK << 16) | qdev->eeprom_cmd_data));
 }

 /*
-- 
2.19.1