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

Sunil Kovvuri Goutham <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <CH3PR18MB6027EE89FA778B95B8194131C6D32@CH3PR18MB6027.namprd18.prod.outlook.com>
>From: "Sergey V. Frolov" <[email protected]>
>
>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 <[email protected]>
>Cc: [email protected]
>---
> 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 <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.