net/ipv4/ip_gre.c | 18 ++++++++++++++++-- net/ipv6/ip6_gre.c | 18 ++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-)
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.
In the normal case the ip_tunnel_info_opts_set() helper is used
which would initialize options_len properly, 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
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>
---
net/ipv4/ip_gre.c | 18 ++++++++++++++++--
net/ipv6/ip6_gre.c | 18 ++++++++++++++++--
2 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 761a53c6a89a..285a656c9e41 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -330,6 +330,22 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
if (!tun_dst)
return PACKET_REJECT;
+ /* 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.
+ *
+ * Link: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-counted_005fby-variable-attribute
+ */
+ 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 +360,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..eb840a11b93b 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -535,6 +535,22 @@ static int ip6erspan_rcv(struct sk_buff *skb,
if (!tun_dst)
return PACKET_REJECT;
+ /* 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.
+ *
+ * Link: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-counted_005fby-variable-attribute
+ */
+ 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 +559,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 +566,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.43.0
On 12/11/2025 11:32 PM, Frode Nordahl wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> 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.
>
> In the normal case the ip_tunnel_info_opts_set() helper is used
> which would initialize options_len properly, 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
>
> 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")
Should this be [PATCH net]?
It seems like this should be intended for the net tree.
> Signed-off-by: Frode Nordahl <fnordahl@ubuntu.com>
> ---
> net/ipv4/ip_gre.c | 18 ++++++++++++++++--
> net/ipv6/ip6_gre.c | 18 ++++++++++++++++--
> 2 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 761a53c6a89a..285a656c9e41 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -330,6 +330,22 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
> if (!tun_dst)
> return PACKET_REJECT;
>
> + /* 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.
> + *
> + * Link: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-counted_005fby-variable-attribute
Nit, but I wonder if the Link in the commit message is good enough? Same
comment below.
Thanks,
Brett
> + */
> + 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 +360,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..eb840a11b93b 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -535,6 +535,22 @@ static int ip6erspan_rcv(struct sk_buff *skb,
> if (!tun_dst)
> return PACKET_REJECT;
>
> + /* 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.
> + *
> + * Link: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-counted_005fby-variable-attribute
> + */
> + 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 +559,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 +566,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.43.0
>
>
On 12/12/25 18:23, Creeley, Brett wrote:
>
> On 12/11/2025 11:32 PM, Frode Nordahl wrote:
>> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>>
>>
>> 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.
>>
>> In the normal case the ip_tunnel_info_opts_set() helper is used
>> which would initialize options_len properly, 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
>>
>> 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")
>
> Should this be [PATCH net]?
>
> It seems like this should be intended for the net tree.
Indeed, thank you for pointing it out!
>> Signed-off-by: Frode Nordahl <fnordahl@ubuntu.com>
>> ---
>> net/ipv4/ip_gre.c | 18 ++++++++++++++++--
>> net/ipv6/ip6_gre.c | 18 ++++++++++++++++--
>> 2 files changed, 32 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> index 761a53c6a89a..285a656c9e41 100644
>> --- a/net/ipv4/ip_gre.c
>> +++ b/net/ipv4/ip_gre.c
>> @@ -330,6 +330,22 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
>> if (!tun_dst)
>> return PACKET_REJECT;
>>
>> + /* 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.
>> + *
>> + * Link: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-counted_005fby-variable-attribute
>
> Nit, but I wonder if the Link in the commit message is good enough? Same
> comment below.
Yes, in retrospect the comments and links in-line became a bit too
verbose, I'll trim them down in the next iteration.
> Thanks,
Thank you for taking the time to review, much appreciated!
--
Frode Nordahl
> Brett
>
>> + */
>> + 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 +360,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..eb840a11b93b 100644
>> --- a/net/ipv6/ip6_gre.c
>> +++ b/net/ipv6/ip6_gre.c
>> @@ -535,6 +535,22 @@ static int ip6erspan_rcv(struct sk_buff *skb,
>> if (!tun_dst)
>> return PACKET_REJECT;
>>
>> + /* 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.
>> + *
>> + * Link: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-counted_005fby-variable-attribute
>> + */
>> + 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 +559,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 +566,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.43.0
>>
>>
On Fri, Dec 12, 2025 at 07:32:01AM +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.
>
> In the normal case the ip_tunnel_info_opts_set() helper is used
> which would initialize options_len properly, 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
>
> 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>
Hi Frode,
Thanks for your patch (and nice to see you recently in Prague :).
Overall this looks good to me but I have some minor feedback.
Firstly, the cited patch seems to cover more than erspan.
So I'm wondering if you took at look at other cases where
this might occur? No problem either way, but if so it might
be worth mentioning in the commit message.
Regarding the comments in the code. I am wondering if the are necessary
as the information is also contained in the commit message. And if the
source documented every such case then things could get rather verbose.
If you do feel strongly about it keeping it then could I ask that
(other than the URL) it is line-wrapped trimmed to 80 columns wide or less,
as is still preferred for Networking (but confusingly not all Kernel) code.
As a fix for code present in net this should be targeted at that tree.
It's best to do so explicitly like this:
Subject: [PATCH net] ...
And it's probably also best to CC stable@vger.kernel.org.
That practice isn't as widespread as perhaps it should be for Networking code.
But it does seem worth mentioning.
...
On 12/12/25 16:13, Simon Horman wrote:
> On Fri, Dec 12, 2025 at 07:32:01AM +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.
>>
>> In the normal case the ip_tunnel_info_opts_set() helper is used
>> which would initialize options_len properly, 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
>>
>> 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>
>
> Hi Frode,
>
> Thanks for your patch (and nice to see you recently in Prague :).
Thank you for taking the time to review, much appreciated (I enjoyed the
recent conference in Prague and our exchanges there!).
> Overall this looks good to me but I have some minor feedback.
>
>
> Firstly, the cited patch seems to cover more than erspan.
> So I'm wondering if you took at look at other cases where
> this might occur? No problem either way, but if so it might
> be worth mentioning in the commit message.
I did some quick searches which formed the basis of the statement of the
normal case being to use the ip_tunnel_info_opts_set(), I could expand a
bit upon that statement.
> Regarding the comments in the code. I am wondering if the are necessary
> as the information is also contained in the commit message. And if the
> source documented every such case then things could get rather verbose.
>
> If you do feel strongly about it keeping it then could I ask that
> (other than the URL) it is line-wrapped trimmed to 80 columns wide or less,
> as is still preferred for Networking (but confusingly not all Kernel) code.
Yes, I guess it became a bit verbose. The thought was that it would be
very easy to miss this important detail for anyone (including future me)
spelunking into this part of the code.
I'll trim it down to a single line, which should be enough to give the
urge to look at the commit message.
> As a fix for code present in net this should be targeted at that tree.
> It's best to do so explicitly like this:
>
> Subject: [PATCH net] ...
Ack.
> And it's probably also best to CC stable@vger.kernel.org.
> That practice isn't as widespread as perhaps it should be for Networking code.
> But it does seem worth mentioning.
Ack, the intention was indeed to Cc them, I only put them into the
e-mail header and the stable kernel bot pointed out that the Cc also
needs to be in the commit message.
--
Frode Nordahl
On Fri, Dec 12, 2025 at 07:21:49PM +0100, Frode Nordahl wrote:
> On 12/12/25 16:13, Simon Horman wrote:
> > On Fri, Dec 12, 2025 at 07:32:01AM +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.
> > >
> > > In the normal case the ip_tunnel_info_opts_set() helper is used
> > > which would initialize options_len properly, 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
> > >
> > > 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>
> >
> > Hi Frode,
> >
> > Thanks for your patch (and nice to see you recently in Prague :).
>
> Thank you for taking the time to review, much appreciated (I enjoyed the
> recent conference in Prague and our exchanges there!).
>
> > Overall this looks good to me but I have some minor feedback.
> >
> >
> > Firstly, the cited patch seems to cover more than erspan.
> > So I'm wondering if you took at look at other cases where
> > this might occur? No problem either way, but if so it might
> > be worth mentioning in the commit message.
>
> I did some quick searches which formed the basis of the statement of the
> normal case being to use the ip_tunnel_info_opts_set(), I could expand a bit
> upon that statement.
Understood. Whichever way you want to go on this is fine by me.
> > Regarding the comments in the code. I am wondering if the are necessary
> > as the information is also contained in the commit message. And if the
> > source documented every such case then things could get rather verbose.
> >
> > If you do feel strongly about it keeping it then could I ask that
> > (other than the URL) it is line-wrapped trimmed to 80 columns wide or less,
> > as is still preferred for Networking (but confusingly not all Kernel) code.
>
> Yes, I guess it became a bit verbose. The thought was that it would be very
> easy to miss this important detail for anyone (including future me)
> spelunking into this part of the code.
>
> I'll trim it down to a single line, which should be enough to give the urge
> to look at the commit message.
Thanks.
> > As a fix for code present in net this should be targeted at that tree.
> > It's best to do so explicitly like this:
> >
> > Subject: [PATCH net] ...
>
> Ack.
>
> > And it's probably also best to CC stable@vger.kernel.org.
> > That practice isn't as widespread as perhaps it should be for Networking code.
> > But it does seem worth mentioning.
>
> Ack, the intention was indeed to Cc them, I only put them into the e-mail
> header and the stable kernel bot pointed out that the Cc also needs to be in
> the commit message.
>
> --
> Frode Nordahl
>
© 2016 - 2025 Red Hat, Inc.