[PATCH] net: xgene: fix signedness bug in xgene_enet_get_fpsel()

[PATCH] net: xgene: fix signedness bug in xgene_enet_get_fpsel()

[Anas Iqbal]

xgene_enet_get_fpsel() returns a u8 but can compute a negative
value when xgene_enet_ring_bufnum(id) is less than
RING_BUFNUM_BUFPOOL. This leads to an implicit conversion of a
negative value to u8, resulting in a large unintended value.

This can cause incorrect behavior when the result is used in
bit operations such as BIT(), potentially leading to undefined
behavior.

Fix this by validating the value before subtraction to avoid
underflow.

Fixes: 2c839337520b ("drivers: net: xgene: Add helper function")
Signed-off-by: Anas Iqbal <mohd.abd.6602@gmail.com>
---
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.h | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
index 2f534f9d4416..fe563c396773 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
@@ -405,10 +405,16 @@ static inline bool xgene_enet_is_bufpool(u16 id)
 
 static inline u8 xgene_enet_get_fpsel(u16 id)
 {
-	if (xgene_enet_is_bufpool(id))
-		return xgene_enet_ring_bufnum(id) - RING_BUFNUM_BUFPOOL;
+	u16 val;
 
-	return 0;
+	if (!xgene_enet_is_bufpool(id))
+		return 0;
+
+	val = xgene_enet_ring_bufnum(id);
+	if (val < RING_BUFNUM_BUFPOOL)
+		return 0;
+
+	return val - RING_BUFNUM_BUFPOOL;
 }
 
 static inline u16 xgene_enet_get_numslots(u16 id, u32 size)
-- 
2.43.0

Re: [PATCH] net: xgene: fix signedness bug in xgene_enet_get_fpsel()

[Simon Horman]

On Thu, Mar 19, 2026 at 09:11:06AM +0000, Anas Iqbal wrote:
> xgene_enet_get_fpsel() returns a u8 but can compute a negative
> value when xgene_enet_ring_bufnum(id) is less than
> RING_BUFNUM_BUFPOOL. This leads to an implicit conversion of a
> negative value to u8, resulting in a large unintended value.
> 
> This can cause incorrect behavior when the result is used in
> bit operations such as BIT(), potentially leading to undefined
> behavior.
> 
> Fix this by validating the value before subtraction to avoid
> underflow.
> 
> Fixes: 2c839337520b ("drivers: net: xgene: Add helper function")
> Signed-off-by: Anas Iqbal <mohd.abd.6602@gmail.com>
> ---
>  drivers/net/ethernet/apm/xgene/xgene_enet_hw.h | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
> index 2f534f9d4416..fe563c396773 100644
> --- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
> +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
> @@ -405,10 +405,16 @@ static inline bool xgene_enet_is_bufpool(u16 id)
>  
>  static inline u8 xgene_enet_get_fpsel(u16 id)
>  {
> -	if (xgene_enet_is_bufpool(id))
> -		return xgene_enet_ring_bufnum(id) - RING_BUFNUM_BUFPOOL;

Hi,

It seems to me that the existing xgene_enet_is_bufpool() condition
protects against underflow. Am I missing something?

Re: [PATCH] net: xgene: fix signedness bug in xgene_enet_get_fpsel()

[Anas Iqbal]

Hi Simon,

Thanks for taking a look.

You are right — xgene_enet_is_bufpool() checks
(id & RING_BUFNUM_MASK) >= 0x20, and
xgene_enet_ring_bufnum() returns the same masked value.
So the condition guarantees that the subtraction cannot
underflow.

This appears to be a false positive from Smatch. I missed checking all the conditions.
Sorry for the inconvenience caused. I will drop this patch.

Thanks for pointing it out.

Regards,
Anas