On 5/22/26 3:45 PM, Mike Pattrick via dev wrote: > On Thu, May 21, 2026 at 11:19 AM Timothy Redaelli <[email protected]> > wrote: > >> On Wed, 20 May 2026 13:46:49 -0400 >> Mike Pattrick <[email protected]> wrote: >> >>> On Tue, May 19, 2026 at 9:20 AM Timothy Redaelli via dev < >>> [email protected]> wrote: >>> >>>> size is uint16_t, promoted to int. When size is UINT16_MAX (0xFFFF), >>>> the expression (size << 16) shifts a 1 into the sign bit of the >>>> resulting int, which is undefined behavior in C. >>>> >>>> Cast to uint32_t before shifting to avoid UB. >>>> >>>> Found by OpenScanHub Coverity (INTEGER_OVERFLOW). >>>> Signed-off-by: Timothy Redaelli <[email protected]> >>>> --- >>>> ofproto/ofproto.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c >>>> index ec6d60a44..9b47869c2 100644 >>>> --- a/ofproto/ofproto.c >>>> +++ b/ofproto/ofproto.c >>>> @@ -9075,7 +9075,7 @@ static uint32_t >>>> eviction_group_priority(size_t n_rules) >>>> { >>>> uint16_t size = MIN(UINT16_MAX, n_rules); >>>> >>> >>> Instead of casting, couldn't the size type just be declared as uint32_t? >> >> It could, but I think the uint16_t makes the intent clearer, since the >> value is clamped to 16 bits, and the type says so. >> Changing to uint32_t would work but hides that. >> >> The cast at the shift is also more in line with what we do elsewhere >> (e.g. UINT32_C(1) << i in dpif-netlink.c). >> > > Ok, that's fair. > > Acked-by: Mike Pattrick <[email protected]>
Thanks, Timothy and Mike! Applied. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
