On Tue, Mar 28, 2023 at 6:02 AM Ilya Maximets <i.maxim...@ovn.org> wrote:
>
> On 3/27/23 22:32, Mike Pattrick wrote:
> > UB Sanitizer report:
> >
> > lib/netdev-offload-tc.c:1276:19: runtime error: load of misaligned
> > address 0x7f74e801976c for type 'union ovs_u128', which requires 8 byte
> > alignment
> >
> >     #0 in netdev_tc_flow_dump_next lib/netdev-offload-tc.c:1276
> >     #1 in netdev_flow_dump_next lib/netdev-offload.c:303
> >     #2 in dpif_netlink_flow_dump_next lib/dpif-netlink.c:1921
> >     [...]
> >
>
> Thanks for the fix, Mike!
>
> How did you catch it?  UBsan doesn't report that for me while
> running a check-offloads testsuite.

I compiled with gcc 11.3.1 (20221121) if that helps. Other than that,
with current master:

# ./configure CFLAGS="-fsanitize=undefined"
# make -j 50
# make check-offloads TESTSUITEFLAGS="2"
## ------------------------------ ##
## openvswitch 3.1.90 test suite. ##
## ------------------------------ ##
  2: offloads - ping between two ports - offloads enabled FAILED
(system-offloads-traffic.at:72)

However, clang version 15.0.7 doesn't identify this spot.

>
> > Signed-off-by: Mike Pattrick <m...@redhat.com>
> > ---
> >  lib/netdev-offload-tc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> > index 4fb9d9f21..506b74ce7 100644
> > --- a/lib/netdev-offload-tc.c
> > +++ b/lib/netdev-offload-tc.c
> > @@ -1273,7 +1273,7 @@ netdev_tc_flow_dump_next(struct netdev_flow_dump 
> > *dump,
> >          }
> >
> >          if (flower.act_cookie.len) {
> > -            *ufid = *((ovs_u128 *) flower.act_cookie.data);
> > +            memcpy(ufid, flower.act_cookie.data, sizeof(ovs_u128));
>
> Please, don't use sizeof(type).  It's prone to errors and also
> against the coding style.  'sizeof *ufid' should be used instead.
>
> What is the actual alignment of this structure?  If it's 4-bytes,
> then we should use get_32aligned_u128() instead to be more clear
> on what is going on here.

I'll resubmit with this correction and Eelco's excellent suggestion.


Cheers,
M

>
> Best regards, Ilya Maximets.
>

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to