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).

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

Reply via email to