On 8/14/24 15:31, Ilya Maximets wrote: > On 8/14/24 14:25, Dumitru Ceara wrote: >> On 8/14/24 14:24, Dumitru Ceara wrote: >>> Hi Ilya, >>> >>> On 8/14/24 14:09, Simon Horman wrote: >>>> On Wed, Aug 14, 2024 at 12:35:28PM +0200, Ilya Maximets wrote: >>>>> memcpy() is allowed to make alignment assumptions based on the argument >>>>> type. We have to cast before using it to avoid misaligned reads: >>> >>> I guess that's fine. But I'm not sure I understand how memcpy() is >>> allowed to make any assumption based on the argument type. Isn't >>> memcpy() supposed to be a function call? > > I guess, I need to re-phrase that into "compilers are allowed to make > alignment assumptions based on the pointer type passed to memcpy()". > > I can change that before applying the patch. >
Sounds good. > Also, even having misaligned pointers is generally not allowed in C... > > Some more info: > https://github.com/llvm/llvm-project/issues/90848#issuecomment-2090636114 > Do you mean that the line above is already undefined behavior, just not flagged as one? const struct rtnl_link_stats64 *lstats = nl_attr_get(a); > There are also similar issues reported in other projects with GCC. > Mainly ARM builds are the main issue as lower ARM architectures are > picky when it comes to misaligned memory accesses. > Ack, accesses I understand, what I'm not sure I understand completely is just having misaligned pointers (that are never dereferenced). >>> >>> At least the man page on my machine says: >>> >>> NAME >>> memcpy - copy memory area >>> >>> LIBRARY >>> Standard C library (libc, -lc) >>> >>> SYNOPSIS >>> #include <string.h> >>> >>> void *memcpy(void dest[restrict .n], const void src[restrict .n], >>> size_t n); >>> >> >> Forgot this part but I guess it was kind of obvious: :) >> >> DESCRIPTION >> The memcpy() function copies n bytes from memory area src to >> memory area dest. The memory areas must not overlap. Use memmove(3) if >> the memory areas do overlap. >> >> >>> Is it actually the compiler doing some special magic behind the scenes? >>> >>>>> >>>>> lib/netdev-linux.c:6738:41: runtime error: load of misaligned address >>>>> 0x51b000008db4 for type 'const struct rpl_rtnl_link_stats64 *', >>>>> which requires 8 byte alignment >>>>> 0x51b000008db4: note: pointer points here >>>>> cc 00 17 00 01 00 00 00 00 00 00 00 01 00 00 00 >>>>> ^ >>>>> 0 0xad2d8 in get_stats_via_netlink lib/netdev-linux.c:6738:17 >>>>> 1 0x9dcce in netdev_linux_get_stats lib/netdev-linux.c:2293:13 >>>>> 2 0xee5b8 in netdev_get_stats lib/netdev.c:1722:16 >>>>> 3 0xc07ad in iface_refresh_stats vswitchd/bridge.c:2795:5 >>>>> 4 0xbf4fc in iface_create vswitchd/bridge.c:2274:5 >>>>> 5 0xbf4fc in bridge_add_ports__ vswitchd/bridge.c:1225:21 >>>>> 6 0x989fc in bridge_add_ports vswitchd/bridge.c:1236:5 >>>>> 7 0x989fc in bridge_reconfigure vswitchd/bridge.c:952:9 >>>>> 8 0x9102a in bridge_run vswitchd/bridge.c:3439:9 >>>>> 9 0xc8976 in main vswitchd/ovs-vswitchd.c:137:9 >>>>> 10 0x2a1c9 in __libc_start_call_main >>>>> 11 0x2a28a in __libc_start_main >>>>> 12 0xb3ff4 in _start (vswitchd/ovs-vswitchd+0x734ff4) >>>>> >>>>> Reported by Clang 18 with UBsan on Ubuntu 24.04. >>>>> >>>>> Fixes: c8c49a9db9f2 ("netdev-linux: Properly access 32-bit aligned >>>>> rtnl_link_stats64 structs.") >>>>> Signed-off-by: Ilya Maximets <[email protected]> >>>> >>>> Reviewed-by: Simon Horman <[email protected]> >>>> >>> >>> Regards, >>> Dumitru >>> >>> >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
