linux-kernel - [PATCH] scsi: mpi3mr: fix invalid 64-bit phy bitmask calculation

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

Message-ID: <20250220094431.38185-1-pchelkin@ispras.ru>
Date: Thu, 20 Feb 2025 12:44:31 +0300
From: Fedor Pchelkin <pchelkin@...ras.ru>
To: Sathya Prakash Veerichetty <sathya.prakash@...adcom.com>,
	Kashyap Desai <kashyap.desai@...adcom.com>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	Ranjan Kumar <ranjan.kumar@...adcom.com>
Cc: Fedor Pchelkin <pchelkin@...ras.ru>,
	Sumit Saxena <sumit.saxena@...adcom.com>,
	Sreekanth Reddy <sreekanth.reddy@...adcom.com>,
	"James E.J. Bottomley" <James.Bottomley@...senPartnership.com>,
	mpi3mr-linuxdrv.pdl@...adcom.com,
	linux-scsi@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	lvc-project@...uxtesting.org,
	stable@...r.kernel.org
Subject: [PATCH] scsi: mpi3mr: fix invalid 64-bit phy bitmask calculation

ffs() operates on arguments of type 'int', not generally considered to be
64-bit values.

Shifts like (1 << i) can also only be helpful for calculations that are
expected to have a range equal to the width of type 'int'. When the left
operand is of type 'int', valid values of the shift argument should not
exceed the width of this type (almost always 32 bits), otherwise it is
considered as undefined behavior.

Since there is a need for manipulating the phy mask bits higher than that,
perform the calculations directly in 64 bits.

Found by Linux Verification Center (linuxtesting.org).

Fixes: cb5b60894602 ("scsi: mpi3mr: Increase maximum number of PHYs to 64 from 32")
Cc: stable@...r.kernel.org
Signed-off-by: Fedor Pchelkin <pchelkin@...ras.ru>
---
 drivers/scsi/mpi3mr/mpi3mr_transport.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
index 0ba9e6a6a13c..f0da8c0dc55d 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_transport.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c
@@ -7,6 +7,7 @@
  *
  */
 
+#include <linux/bits.h>
 #include <linux/vmalloc.h>
 
 #include "mpi3mr.h"
@@ -608,10 +609,12 @@ static void mpi3mr_delete_sas_phy(struct mpi3mr_ioc *mrioc,
 	mr_sas_port->num_phys--;
 
 	if (host_node) {
-		mr_sas_port->phy_mask &= ~(1 << mr_sas_phy->phy_id);
+		mr_sas_port->phy_mask &= ~BIT_ULL(mr_sas_phy->phy_id);
 
-		if (mr_sas_port->lowest_phy == mr_sas_phy->phy_id)
-			mr_sas_port->lowest_phy = ffs(mr_sas_port->phy_mask) - 1;
+		if (mr_sas_port->lowest_phy == mr_sas_phy->phy_id &&
+		    mr_sas_port->phy_mask)
+			mr_sas_port->lowest_phy =
+				__ffs64(mr_sas_port->phy_mask) - 1;
 	}
 	sas_port_delete_phy(mr_sas_port->port, mr_sas_phy->phy);
 	mr_sas_phy->phy_belongs_to_port = 0;
@@ -639,10 +642,11 @@ static void mpi3mr_add_sas_phy(struct mpi3mr_ioc *mrioc,
 	list_add_tail(&mr_sas_phy->port_siblings, &mr_sas_port->phy_list);
 	mr_sas_port->num_phys++;
 	if (host_node) {
-		mr_sas_port->phy_mask |= (1 << mr_sas_phy->phy_id);
+		mr_sas_port->phy_mask |= BIT_ULL(mr_sas_phy->phy_id);
 
 		if (mr_sas_phy->phy_id < mr_sas_port->lowest_phy)
-			mr_sas_port->lowest_phy = ffs(mr_sas_port->phy_mask) - 1;
+			mr_sas_port->lowest_phy =
+				__ffs64(mr_sas_port->phy_mask) - 1;
 	}
 	sas_port_add_phy(mr_sas_port->port, mr_sas_phy->phy);
 	mr_sas_phy->phy_belongs_to_port = 1;
@@ -1396,7 +1400,7 @@ static struct mpi3mr_sas_port *mpi3mr_sas_port_add(struct mpi3mr_ioc *mrioc,
 		    &mr_sas_port->phy_list);
 		mr_sas_port->num_phys++;
 		if (mr_sas_node->host_node)
-			mr_sas_port->phy_mask |= (1 << i);
+			mr_sas_port->phy_mask |= BIT_ULL(i);
 	}
 
 	if (!mr_sas_port->num_phys) {
@@ -1406,7 +1410,7 @@ static struct mpi3mr_sas_port *mpi3mr_sas_port_add(struct mpi3mr_ioc *mrioc,
 	}
 
 	if (mr_sas_node->host_node)
-		mr_sas_port->lowest_phy = ffs(mr_sas_port->phy_mask) - 1;
+		mr_sas_port->lowest_phy = __ffs64(mr_sas_port->phy_mask) - 1;
 
 	if (mr_sas_port->remote_identify.device_type == SAS_END_DEVICE) {
 		tgtdev = mpi3mr_get_tgtdev_by_addr(mrioc,
@@ -1738,7 +1742,7 @@ mpi3mr_refresh_sas_ports(struct mpi3mr_ioc *mrioc)
 		found = 0;
 		for (j = 0; j < host_port_count; j++) {
 			if (h_port[j].handle == attached_handle) {
-				h_port[j].phy_mask |= (1 << i);
+				h_port[j].phy_mask |= BIT_ULL(i);
 				found = 1;
 				break;
 			}
@@ -1765,7 +1769,7 @@ mpi3mr_refresh_sas_ports(struct mpi3mr_ioc *mrioc)
 		port_idx = host_port_count;
 		h_port[port_idx].sas_address = le64_to_cpu(sasinf->sas_address);
 		h_port[port_idx].handle = attached_handle;
-		h_port[port_idx].phy_mask = (1 << i);
+		h_port[port_idx].phy_mask = BIT_ULL(i);
 		h_port[port_idx].iounit_port_id = sas_io_unit_pg0->phy_data[i].io_unit_port;
 		h_port[port_idx].lowest_phy = sasinf->phy_num;
 		h_port[port_idx].used = 0;
-- 
2.48.1