netdev - Re: [ovs-dev] [PATCH net-next] net: Add options as a flexible array to struct ip_tunnel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2ef88acc-d4d7-4309-8c14-73ac107d1d07@ovn.org>
Date: Sun, 9 Feb 2025 17:21:53 +0100
From: Ilya Maximets <i.maximets@....org>
To: Gal Pressman <gal@...dia.com>, netdev@...r.kernel.org,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Andrew Lunn <andrew+netdev@...n.ch>
Cc: dev@...nvswitch.org, "Gustavo A. R. Silva" <gustavoars@...nel.org>,
 Jiri Pirko <jiri@...nulli.us>, Jamal Hadi Salim <jhs@...atatu.com>,
 Kees Cook <kees@...nel.org>, David Ahern <dsahern@...nel.org>,
 Yotam Gigi <yotam.gi@...il.com>, Tariq Toukan <tariqt@...dia.com>,
 linux-hardening@...r.kernel.org, Simon Horman <horms@...nel.org>,
 Cong Wang <xiyou.wangcong@...il.com>, Cosmin Ratiu <cratiu@...dia.com>,
 i.maximets@....org
Subject: Re: [ovs-dev] [PATCH net-next] net: Add options as a flexible array
 to struct ip_tunnel_info

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.
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'.
May cause crashes on some architectures as well as performance impact on
others.

Should the alignment attribute be also added to the field?

Best regards, Ilya Maximets.