On Wed, Feb 19, 2025 at 04:32:56PM +0200, Gal Pressman 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().
>
> The layout of struct ip_tunnel_info remains the same with this patch.
> Before this patch, there was an implicit padding at the end of the
> struct, options would be written at 'info + 1' which is after the
> padding.
> This will remain the same as this patch explicitly aligns 'options'.
> The alignment is needed as the options are later casted to different
> structs, and might result in unaligned memory access.
>
> Pahole output before this patch:
> struct ip_tunnel_info {
> struct ip_tunnel_key key; /* 0 64 */
>
> /* XXX last struct has 1 byte of padding */
>
> /* --- cacheline 1 boundary (64 bytes) --- */
> struct ip_tunnel_encap encap; /* 64 8 */
> struct dst_cache dst_cache; /* 72 16 */
> u8 options_len; /* 88 1 */
> u8 mode; /* 89 1 */
>
> /* size: 96, cachelines: 2, members: 5 */
> /* padding: 6 */
> /* paddings: 1, sum paddings: 1 */
> /* last cacheline: 32 bytes */
> };
>
> Pahole output after this patch:
> struct ip_tunnel_info {
> struct ip_tunnel_key key; /* 0 64 */
>
> /* XXX last struct has 1 byte of padding */
>
> /* --- cacheline 1 boundary (64 bytes) --- */
> struct ip_tunnel_encap encap; /* 64 8 */
> struct dst_cache dst_cache; /* 72 16 */
> u8 options_len; /* 88 1 */
> u8 mode; /* 89 1 */
>
> /* XXX 6 bytes hole, try to pack */
>
> u8 options[] __attribute__((__aligned__(16))); /*
> 96 0 */
>
> /* size: 96, cachelines: 2, members: 6 */
> /* sum members: 90, holes: 1, sum holes: 6 */
> /* paddings: 1, sum paddings: 1 */
> /* forced alignments: 1, forced holes: 1, sum forced holes: 6 */
> /* last cacheline: 32 bytes */
> } __attribute__((__aligned__(16)));
>
> [1] Commit 13cfd6a6d7ac ("net: Silence false field-spanning write warning in
> metadata_dst memcpy")
>
> Link:
> https://lore.kernel.org/all/[email protected]/
> Suggested-by: Kees Cook <[email protected]>
> Reviewed-by: Cosmin Ratiu <[email protected]>
> Reviewed-by: Tariq Toukan <[email protected]>
> Signed-off-by: Gal Pressman <[email protected]>
Thanks for these updates and the pahole output. :)
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
