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]>