* [PATCH net v2] erspan: Initialize options_len before referencing options.
@ 2025-12-13 10:13 Frode Nordahl
2025-12-15 14:58 ` Simon Horman
0 siblings, 1 reply; 2+ messages in thread
From: Frode Nordahl @ 2025-12-13 10:13 UTC (permalink / raw)
To: David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Tariq Toukan, Cosmin Ratiu,
Gal Pressman, Kees Cook
Cc: Frode Nordahl, stable, netdev, linux-kernel
The struct ip_tunnel_info has a flexible array member named
options that is protected by a counted_by(options_len)
attribute.
The compiler will use this information to enforce runtime bounds
checking deployed by FORTIFY_SOURCE string helpers.
As laid out in the GCC documentation, the counter must be
initialized before the first reference to the flexible array
member.
After scanning through the files that use struct ip_tunnel_info
and also refer to options or options_len, it appears the normal
case is to use the ip_tunnel_info_opts_set() helper.
Said helper would initialize options_len properly before copying
data into options, however in the GRE ERSPAN code a partial
update is done, preventing the use of the helper function.
Before this change the handling of ERSPAN traffic in GRE tunnels
would cause a kernel panic when the kernel is compiled with
GCC 15+ and having FORTIFY_SOURCE configured:
memcpy: detected buffer overflow: 4 byte write of buffer size 0
Call Trace:
<IRQ>
__fortify_panic+0xd/0xf
erspan_rcv.cold+0x68/0x83
? ip_route_input_slow+0x816/0x9d0
gre_rcv+0x1b2/0x1c0
gre_rcv+0x8e/0x100
? raw_v4_input+0x2a0/0x2b0
ip_protocol_deliver_rcu+0x1ea/0x210
ip_local_deliver_finish+0x86/0x110
ip_local_deliver+0x65/0x110
? ip_rcv_finish_core+0xd6/0x360
ip_rcv+0x186/0x1a0
Cc: stable@vger.kernel.org
Link: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-counted_005fby-variable-attribute
Reported-at: https://launchpad.net/bugs/2129580
Fixes: bb5e62f2d547 ("net: Add options as a flexible array to struct ip_tunnel_info")
Signed-off-by: Frode Nordahl <fnordahl@ubuntu.com>
---
v2:
- target correct netdev tree and properly cc stable in commit message.
- replace repeated long in-line comments and link with a single line.
- document search for any similar offenses in the code base in commit
message.
v1: https://lore.kernel.org/all/20251212073202.13153-1-fnordahl@ubuntu.com/
net/ipv4/ip_gre.c | 6 ++++--
net/ipv6/ip6_gre.c | 6 ++++--
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 761a53c6a89a..8178c44a3cdd 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -330,6 +330,10 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
if (!tun_dst)
return PACKET_REJECT;
+ /* MUST set options_len before referencing options */
+ info = &tun_dst->u.tun_info;
+ info->options_len = sizeof(*md);
+
/* skb can be uncloned in __iptunnel_pull_header, so
* old pkt_md is no longer valid and we need to reset
* it
@@ -344,10 +348,8 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
memcpy(md2, pkt_md, ver == 1 ? ERSPAN_V1_MDSIZE :
ERSPAN_V2_MDSIZE);
- info = &tun_dst->u.tun_info;
__set_bit(IP_TUNNEL_ERSPAN_OPT_BIT,
info->key.tun_flags);
- info->options_len = sizeof(*md);
}
skb_reset_mac_header(skb);
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index c82a75510c0e..4603554d4c7f 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -535,6 +535,10 @@ static int ip6erspan_rcv(struct sk_buff *skb,
if (!tun_dst)
return PACKET_REJECT;
+ /* MUST set options_len before referencing options */
+ info = &tun_dst->u.tun_info;
+ info->options_len = sizeof(*md);
+
/* skb can be uncloned in __iptunnel_pull_header, so
* old pkt_md is no longer valid and we need to reset
* it
@@ -543,7 +547,6 @@ static int ip6erspan_rcv(struct sk_buff *skb,
skb_network_header_len(skb);
pkt_md = (struct erspan_metadata *)(gh + gre_hdr_len +
sizeof(*ershdr));
- info = &tun_dst->u.tun_info;
md = ip_tunnel_info_opts(info);
md->version = ver;
md2 = &md->u.md2;
@@ -551,7 +554,6 @@ static int ip6erspan_rcv(struct sk_buff *skb,
ERSPAN_V2_MDSIZE);
__set_bit(IP_TUNNEL_ERSPAN_OPT_BIT,
info->key.tun_flags);
- info->options_len = sizeof(*md);
ip6_tnl_rcv(tunnel, skb, tpi, tun_dst, log_ecn_error);
--
2.51.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH net v2] erspan: Initialize options_len before referencing options.
2025-12-13 10:13 [PATCH net v2] erspan: Initialize options_len before referencing options Frode Nordahl
@ 2025-12-15 14:58 ` Simon Horman
0 siblings, 0 replies; 2+ messages in thread
From: Simon Horman @ 2025-12-15 14:58 UTC (permalink / raw)
To: Frode Nordahl
Cc: David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Tariq Toukan, Cosmin Ratiu, Gal Pressman, Kees Cook,
stable, netdev, linux-kernel
On Sat, Dec 13, 2025 at 10:13:36AM +0000, Frode Nordahl wrote:
> The struct ip_tunnel_info has a flexible array member named
> options that is protected by a counted_by(options_len)
> attribute.
>
> The compiler will use this information to enforce runtime bounds
> checking deployed by FORTIFY_SOURCE string helpers.
>
> As laid out in the GCC documentation, the counter must be
> initialized before the first reference to the flexible array
> member.
>
> After scanning through the files that use struct ip_tunnel_info
> and also refer to options or options_len, it appears the normal
> case is to use the ip_tunnel_info_opts_set() helper.
>
> Said helper would initialize options_len properly before copying
> data into options, however in the GRE ERSPAN code a partial
> update is done, preventing the use of the helper function.
>
> Before this change the handling of ERSPAN traffic in GRE tunnels
> would cause a kernel panic when the kernel is compiled with
> GCC 15+ and having FORTIFY_SOURCE configured:
>
> memcpy: detected buffer overflow: 4 byte write of buffer size 0
>
> Call Trace:
> <IRQ>
> __fortify_panic+0xd/0xf
> erspan_rcv.cold+0x68/0x83
> ? ip_route_input_slow+0x816/0x9d0
> gre_rcv+0x1b2/0x1c0
> gre_rcv+0x8e/0x100
> ? raw_v4_input+0x2a0/0x2b0
> ip_protocol_deliver_rcu+0x1ea/0x210
> ip_local_deliver_finish+0x86/0x110
> ip_local_deliver+0x65/0x110
> ? ip_rcv_finish_core+0xd6/0x360
> ip_rcv+0x186/0x1a0
>
> Cc: stable@vger.kernel.org
> Link: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-counted_005fby-variable-attribute
> Reported-at: https://launchpad.net/bugs/2129580
> Fixes: bb5e62f2d547 ("net: Add options as a flexible array to struct ip_tunnel_info")
> Signed-off-by: Frode Nordahl <fnordahl@ubuntu.com>
> ---
> v2:
> - target correct netdev tree and properly cc stable in commit message.
> - replace repeated long in-line comments and link with a single line.
> - document search for any similar offenses in the code base in commit
> message.
> v1: https://lore.kernel.org/all/20251212073202.13153-1-fnordahl@ubuntu.com/
Thanks for the updates.
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-12-15 14:58 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-13 10:13 [PATCH net v2] erspan: Initialize options_len before referencing options Frode Nordahl
2025-12-15 14:58 ` Simon Horman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).