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.

> Signed-off-by: Mike Pattrick <[email protected]>
> ---
>  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.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to