On 10/26/22 13:19, Ilya Maximets wrote:
> Offsets within 'rewrite' action are not 4-byte aligned, so has to
> be accessed carefully.
>
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior lib/tc.c:1132:17 in
>
> lib/tc.c:1132:17: runtime error: store to misaligned address 0x7fba215b2025
> for type 'ovs_be32' (aka 'unsigned int'), which requires 4 byte alignment
>
> 0 0xd78857 in nl_parse_act_pedit lib/tc.c:1132:24
> 1 0xd68103 in nl_parse_single_action lib/tc.c:1936:15
> 2 0xd624ee in nl_parse_flower_actions lib/tc.c:2024:19
> 3 0xd624ee in nl_parse_flower_options lib/tc.c:2139:12
> 4 0xd5f082 in parse_netlink_to_tc_flower lib/tc.c:2187:12
> 5 0xd6a2a1 in tc_replace_flower lib/tc.c:3776:19
> 6 0xd2ae8f in netdev_tc_flow_put lib/netdev-offload-tc.c:2350:11
> 7 0x951d07 in netdev_flow_put lib/netdev-offload.c:318:14
> 8 0xcbb81a in parse_flow_put lib/dpif-netlink.c:2297:11
> 9 0xcbb81a in try_send_to_netdev lib/dpif-netlink.c:2384:15
> 10 0xcbb81a in dpif_netlink_operate lib/dpif-netlink.c:2455:23
> 11 0x8678ae in dpif_operate lib/dpif.c:1372:13
> 12 0x6bcc89 in handle_upcalls ofproto/ofproto-dpif-upcall.c:1674:5
> 13 0x6bcc89 in recv_upcalls ofproto/ofproto-dpif-upcall.c:905:9
> 14 0x6b7f9a in udpif_upcall_handler ofproto/ofproto-dpif-upcall.c:801:13
> 15 0xb54c5a in ovsthread_wrapper lib/ovs-thread.c:422:12
> 16 0x7fba2f2081ce in start_thread (/lib64/libpthread.so.0+0x81ce)
> 17 0x7fba2de39dd2 in clone (/lib64/libc.so.6+0x39dd2)
>
> Fixes: 8ada482bbe19 ("tc: Add header rewrite using tc pedit action")
> Signed-off-by: Ilya Maximets <[email protected]>
> ---
>
> Found while running 'make check-offloads' on 6.0 kernel.
>
> lib/tc.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
Hmm, this patch makes sparse angry. I'll fix that in v2.
>
> diff --git a/lib/tc.c b/lib/tc.c
> index 94044cde6..a997055c4 100644
> --- a/lib/tc.c
> +++ b/lib/tc.c
> @@ -1115,7 +1115,7 @@ nl_parse_act_pedit(struct nlattr *options, struct
> tc_flower *flower)
> ovs_be32 *dst = (void *) (rewrite_key + diff);
> ovs_be32 *dst_m = (void *) (rewrite_mask + diff);
> ovs_be32 mask, mask_word, data_word;
> - uint32_t zero_bits;
> + uint32_t zero_bits, val;
>
> mask_word = htonl(ntohl(keys->mask) << m->boundary_shift);
> data_word = htonl(ntohl(keys->val) << m->boundary_shift);
> @@ -1129,8 +1129,13 @@ nl_parse_act_pedit(struct nlattr *options, struct
> tc_flower *flower)
> mask &= htonl(UINT32_MAX << zero_bits);
> }
>
> - *dst_m |= mask;
> - *dst |= data_word & mask;
> + val = get_unaligned_u32(dst_m);
> + val |= mask;
> + put_unaligned_u32(dst_m, val);
> +
> + val = get_unaligned_u32(dst);
> + val |= data_word & mask;
> + put_unaligned_u32(dst, val);
> }
> }
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
