On Wed, Jun 12, 2024 at 9:50 AM Ales Musil <[email protected]> wrote:

>
>
> On Wed, Jun 12, 2024 at 3:32 PM Mike Pattrick <[email protected]> wrote:
>
>> When compiling with '-fsanitize=address,undefined', the "ovs-ofctl
>> ct-flush" test will yield the following undefined behavior flagged
>> by UBSan. This patch uses memcpy to move the 128bit value into the
>> stack before reading it.
>>
>> lib/ofp-prop.c:277:14: runtime error: load of misaligned address
>> for type 'union ovs_be128', which requires 8 byte alignment
>>               ^
>>     #0 0x7735d4 in ofpprop_parse_u128 lib/ofp-prop.c:277
>>     #1 0x6c6c83 in ofp_ct_match_decode lib/ofp-ct.c:529
>>     #2 0x76f3b5 in ofp_print_nxt_ct_flush lib/ofp-print.c:959
>>     #3 0x76f3b5 in ofp_to_string__ lib/ofp-print.c:1206
>>     #4 0x76f3b5 in ofp_to_string lib/ofp-print.c:1264
>>     #5 0x770c0d in ofp_print lib/ofp-print.c:1308
>>     #6 0x484a9d in ofctl_ofp_print utilities/ovs-ofctl.c:4899
>>     #7 0x4ddb77 in ovs_cmdl_run_command__ lib/command-line.c:247
>>     #8 0x47f6b3 in main utilities/ovs-ofctl.c:186
>>
>> Signed-off-by: Mike Pattrick <[email protected]>
>> ---
>>
>
> Hi Mike,
>
> this is interesting, do you have an idea why it didn't fail in CI by now?
> Also AFAIR the ofprops is supposed to be aligned to 8 bytes so unless the
> buffer itself isn't allocated at an address that is not itself 8 bytes
> aligned it shouldn't happen. In that case we might actually have a problem
> with other sizes.
>

Report is seen with gcc + ubsan, but not clang + ubsan. It is possible that
this is only seen due the test, this warning wasn't seen live.

Cheers,
M


>
>
>>  lib/ofp-prop.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/ofp-prop.c b/lib/ofp-prop.c
>> index 0a685750c..ed6365414 100644
>> --- a/lib/ofp-prop.c
>> +++ b/lib/ofp-prop.c
>> @@ -271,10 +271,12 @@ enum ofperr
>>  ofpprop_parse_u128(const struct ofpbuf *property, ovs_u128 *value)
>>  {
>>      ovs_be128 *p = property->msg;
>> +    ovs_be128 aligned;
>>      if (ofpbuf_msgsize(property) != sizeof *p) {
>>          return OFPERR_OFPBPC_BAD_LEN;
>>      }
>> -    *value = ntoh128(*p);
>> +    memcpy(&aligned, p, sizeof aligned);
>> +    *value = ntoh128(aligned);
>>      return 0;
>>  }
>>
>> --
>> 2.39.3
>>
>> _______________________________________________
>> dev mailing list
>> [email protected]
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
> Thanks,
> Ales
>
> --
>
> Ales Musil
>
> Senior Software Engineer - OVN Core
>
> Red Hat EMEA <https://www.redhat.com>
>
> [email protected]
> <https://red.ht/sig>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to