Hi Ilya, thanks for the review. On 09/02/2025 18:21, Ilya Maximets wrote: > On 2/9/25 11:18, Gal Pressman via dev wrote: >> Remove the hidden assumption that options are allocated at the end of >> the struct, and teach the compiler about them using a flexible array. >> >> With this, we can revert the unsafe_memcpy() call we have in >> tun_dst_unclone() [1], and resolve the false field-spanning write >> warning caused by the memcpy() in ip_tunnel_info_opts_set(). >> >> Note that this patch changes the layout of struct ip_tunnel_info since >> there is padding at the end of the struct. >> Before this, options would be written at 'info + 1' which is after the >> padding. >> After this patch, options are written right after 'mode' field (into the >> padding). > > This doesn't sound like a safe thing to do. 'info + 1' ensures that the > options are aligned the same way as the struct ip_tunnel_info itself.
What is special about the alignment of struct ip_tunnel_info? What are you assuming it to be, and how is it related to whatever alignment the options need? > In many places in the code, the options are cast into a specific tunnel > options type that may require sufficient alignment. And the alignment can > no longer be guaranteed once the options are put directly after the 'mode'. What guaranteed it was aligned before? A hidden assumption that a u64 is hidden somewhere in ip_tunnel_info? > May cause crashes on some architectures as well as performance impact on > others. > > Should the alignment attribute be also added to the field? Align to what? To the first field of every potential options type? To eight bytes? _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
