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]> > > > > > - return (size << 16) | random_uint16(); > > > + return ((uint32_t) size << 16) | random_uint16(); > > > } > > > > > > /* Updates 'evg', an eviction_group within 'table', following a change > > > that > > > -- > > > 2.54.0 > > > > > > _______________________________________________ > > > dev mailing list > > > [email protected] > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
