diff mbox series

[1/1] net: phy: Fix signed shift overflow

Message ID 20240104042623.103398-1-heinrich.schuchardt@canonical.com
State Accepted
Commit f5dbfc82a90b2333bfee76906df883ba86a85f56
Delegated to: Ramon Fried
Headers show
Series [1/1] net: phy: Fix signed shift overflow | expand

Commit Message

Heinrich Schuchardt Jan. 4, 2024, 4:26 a.m. UTC 
From: Eugeniu Rosca <erosca@de.adit-jv.com>

Booting R-Car Gen3 arm64 U-Boot with CONFIG_UBSAN=y resulted in:

 =====================================================================
 UBSAN: Undefined behaviour in drivers/net/phy/phy.c:728:19
 left shift of 1 by 31 places cannot be represented in type 'int'
 =====================================================================

Fix it by appending the UL suffix to the numeric literal. While at it,
convert the type of "addr" variable from signed to unsigned, to protect
against shifting the numeric literal by a negative value (which would
lead to yet another undefined behavior).

Fixes: 1adb406b0141 ("phy: add phy_find_by_mask/phy_connect_dev")
Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>

* Using U-suffix for integer is sufficient.
* ffs() of non-zero value cannot be 0. But addr being unsigned is
* preferable.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 drivers/net/phy/phy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Tom Rini March 28, 2024, 3:09 p.m. UTC | #1 
On Thu, Jan 04, 2024 at 05:26:23AM +0100, Heinrich Schuchardt wrote:

> From: Eugeniu Rosca <erosca@de.adit-jv.com>
> 
> Booting R-Car Gen3 arm64 U-Boot with CONFIG_UBSAN=y resulted in:
> 
>  =====================================================================
>  UBSAN: Undefined behaviour in drivers/net/phy/phy.c:728:19
>  left shift of 1 by 31 places cannot be represented in type 'int'
>  =====================================================================
> 
> Fix it by appending the UL suffix to the numeric literal. While at it,
> convert the type of "addr" variable from signed to unsigned, to protect
> against shifting the numeric literal by a negative value (which would
> lead to yet another undefined behavior).
> 
> Fixes: 1adb406b0141 ("phy: add phy_find_by_mask/phy_connect_dev")
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> 
> * Using U-suffix for integer is sufficient.
> * ffs() of non-zero value cannot be 0. But addr being unsigned is
> * preferable.
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

Applied to u-boot/next, thanks!
diff mbox series

Patch

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 63b3e46f10..f39ca942f6 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -642,12 +642,12 @@  static struct phy_device *search_for_existing_phy(struct mii_dev *bus,
 {
 	/* If we have one, return the existing device, with new interface */
 	while (phy_mask) {
-		int addr = ffs(phy_mask) - 1;
+		unsigned int addr = ffs(phy_mask) - 1;
 
 		if (bus->phymap[addr])
 			return bus->phymap[addr];
 
-		phy_mask &= ~(1 << addr);
+		phy_mask &= ~(1U << addr);
 	}
 	return NULL;
 }