RE:[PATCH net v2] net: octeontx2-pf: Fix UB in shift operation

[Sunil Kovvuri Goutham]

>From: "Sergey V. Frolov" <Sergey.V.Frolov@xxxxxxxxxxxxx>
>
>In function otx2_get_egress_burst_cfg, when the parameter `burst` is
>255 and the max mantissa is 255 (0xFFULL), `burst_exp` is set to
>`ilog2(255) - 1`, which equals 6.
>
>This results in an unsigned wrap-around when calculating `(1ULL << (*burst_exp -
>7))`, since `*burst_exp - 7` becomes -1, which makes the shift operand 0xFFFFFFFF.
>This value is greater than the width of the left operand.
>
>According to standard 6.5.7 p.3:
>"The type of the result is that of the promoted left operand.
>If the value of the right operand is negative or is greater than or equal to the width
>of the promoted left operand, the behavior is undefined."
>
>Fix the off-by-one boundary condition.
>
>Add a WARN_ON(*burst_exp < 7) before the else branch as an explicit safeguard.
>This ensures that if max_mantissa ever changes in a way that reintroduces this
>condition, it will be immediately caught at runtime rather than silently triggering UB.
>
>Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
>Fixes: e638a83f167e ("octeontx2-pf: TC_MATCHALL egress ratelimiting offload")
>Signed-off-by: Sergey V. Frolov <Sergey.V.Frolov@xxxxxxxxxxxxx>
>Cc: stable@xxxxxxxxxxxxxxx
>---
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
>b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
>index 40162b08014d..652276cb314c 100644
>--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
>+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
>@@ -53,10 +53,12 @@ static void otx2_get_egress_burst_cfg(struct otx2_nic *nic,
>u32 burst,
> 	if (burst) {
> 		*burst_exp = ilog2(burst) ? ilog2(burst) - 1 : 0;
> 		tmp = burst - rounddown_pow_of_two(burst);
>-		if (burst < max_mantissa)
>+		if (burst <= max_mantissa) {
> 			*burst_mantissa = tmp * 2;
>-		else
>+		} else {
>+			WARN_ON(*burst_exp < 7);
> 			*burst_mantissa = tmp / (1ULL << (*burst_exp - 7));
>+		}
> 	} else {
> 		*burst_exp = MAX_BURST_EXPONENT;
> 		*burst_mantissa = max_mantissa;
>--
>2.34.1


Thanks for the patch.

Reviewed-by: Sunil Goutham <sgoutham@xxxxxxxxxxx>