public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] ipv6: Apply max_dst_opts_cnt to ip6_tnl_parse_tlv_enc_lim
@ 2026-04-17 22:03 Daniel Borkmann
  2026-04-18 10:59 ` Justin Iurman
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Borkmann @ 2026-04-17 22:03 UTC (permalink / raw)
  To: kuba; +Cc: edumazet, dsahern, tom, willemdebruijn.kernel, idosch, pabeni,
	netdev

Commit 47d3d7ac656a ("ipv6: Implement limits on Hop-by-Hop and
Destination options") added net.ipv6.max_{hbh,dst}_opts_{cnt,len}
and applied them in ip6_parse_tlv(), the generic TLV walker
invoked from ipv6_destopt_rcv() and ipv6_parse_hopopts().

ip6_tnl_parse_tlv_enc_lim() does not go through ip6_parse_tlv();
it has its own hand-rolled TLV scanner inside its NEXTHDR_DEST
branch which looks for IPV6_TLV_TNL_ENCAP_LIMIT. That inner
loop is bounded only by optlen, which can be up to 2048 bytes.
Stuffing the Destination Options header with 2046 Pad1 (type=0)
entries advances the scanner a single byte at a time, yielding
~2000 TLV iterations per extension header.

Reuse max_dst_opts_cnt to bound the TLV iterations, matching
the semantics from 47d3d7ac656a.

Fixes: 47d3d7ac656a ("ipv6: Implement limits on Hop-by-Hop and Destination options")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 net/ipv6/ip6_tunnel.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 907c6a2af331..0ab76f93c136 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -430,11 +430,16 @@ __u16 ip6_tnl_parse_tlv_enc_lim(struct sk_buff *skb, __u8 *raw)
 				break;
 		}
 		if (nexthdr == NEXTHDR_DEST) {
+			int tlv_max = READ_ONCE(init_net.ipv6.sysctl.max_dst_opts_cnt);
+			int tlv_cnt = 0;
 			u16 i = 2;
 
 			while (1) {
 				struct ipv6_tlv_tnl_enc_lim *tel;
 
+				if (unlikely(tlv_cnt++ >= tlv_max))
+					break;
+
 				/* No more room for encapsulation limit */
 				if (i + sizeof(*tel) > optlen)
 					break;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] ipv6: Apply max_dst_opts_cnt to ip6_tnl_parse_tlv_enc_lim
  2026-04-17 22:03 [PATCH net] ipv6: Apply max_dst_opts_cnt to ip6_tnl_parse_tlv_enc_lim Daniel Borkmann
@ 2026-04-18 10:59 ` Justin Iurman
  2026-04-18 12:17   ` Daniel Borkmann
  0 siblings, 1 reply; 3+ messages in thread
From: Justin Iurman @ 2026-04-18 10:59 UTC (permalink / raw)
  To: Daniel Borkmann, kuba
  Cc: edumazet, dsahern, tom, willemdebruijn.kernel, idosch, pabeni,
	netdev

On 4/18/26 00:03, Daniel Borkmann wrote:
> Commit 47d3d7ac656a ("ipv6: Implement limits on Hop-by-Hop and
> Destination options") added net.ipv6.max_{hbh,dst}_opts_{cnt,len}
> and applied them in ip6_parse_tlv(), the generic TLV walker
> invoked from ipv6_destopt_rcv() and ipv6_parse_hopopts().
> 
> ip6_tnl_parse_tlv_enc_lim() does not go through ip6_parse_tlv();
> it has its own hand-rolled TLV scanner inside its NEXTHDR_DEST
> branch which looks for IPV6_TLV_TNL_ENCAP_LIMIT. That inner
> loop is bounded only by optlen, which can be up to 2048 bytes.
> Stuffing the Destination Options header with 2046 Pad1 (type=0)
> entries advances the scanner a single byte at a time, yielding
> ~2000 TLV iterations per extension header.
> 
> Reuse max_dst_opts_cnt to bound the TLV iterations, matching
> the semantics from 47d3d7ac656a.
> 
> Fixes: 47d3d7ac656a ("ipv6: Implement limits on Hop-by-Hop and Destination options")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
>   net/ipv6/ip6_tunnel.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
> index 907c6a2af331..0ab76f93c136 100644
> --- a/net/ipv6/ip6_tunnel.c
> +++ b/net/ipv6/ip6_tunnel.c
> @@ -430,11 +430,16 @@ __u16 ip6_tnl_parse_tlv_enc_lim(struct sk_buff *skb, __u8 *raw)
>   				break;
>   		}
>   		if (nexthdr == NEXTHDR_DEST) {
> +			int tlv_max = READ_ONCE(init_net.ipv6.sysctl.max_dst_opts_cnt);
> +			int tlv_cnt = 0;
>   			u16 i = 2;
>   
>   			while (1) {
>   				struct ipv6_tlv_tnl_enc_lim *tel;
>   
> +				if (unlikely(tlv_cnt++ >= tlv_max))
> +					break;
> +
>   				/* No more room for encapsulation limit */
>   				if (i + sizeof(*tel) > optlen)
>   					break;

Good point on reusing max_dst_opts_cnt in ip6_tnl_parse_tlv_enc_lim(), 
but this patch is not ready yet.

We need to be careful: max_dst_opts_cnt can be negative. If this is the 
case, ip6_tnl_parse_tlv_enc_lim() would probably return 0, which is not 
what we want here. From the doc:

max_dst_opts_number - INTEGER
         Maximum number of non-padding TLVs allowed in a Destination
         options extension header. If this value is less than zero
         then unknown options are disallowed and the number of known
         TLVs allowed is the absolute value of this number.

         Default: 8

Since ip6_tnl_parse_tlv_enc_lim() does not check for specific option 
types (e.g., Pad1, PadN, you-name-it) and does not differentiate known 
from unknown options during parsing, I would simply use the absolute 
value of max_dst_opts_cnt by default.

Also, I wouldn't use unlikely() because it could harm us more than it 
helps in this specific context (consistent with ip6_parse_tlv()).

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] ipv6: Apply max_dst_opts_cnt to ip6_tnl_parse_tlv_enc_lim
  2026-04-18 10:59 ` Justin Iurman
@ 2026-04-18 12:17   ` Daniel Borkmann
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Borkmann @ 2026-04-18 12:17 UTC (permalink / raw)
  To: Justin Iurman, kuba
  Cc: edumazet, dsahern, tom, willemdebruijn.kernel, idosch, pabeni,
	netdev

Hi Justin,

On 4/18/26 12:59 PM, Justin Iurman wrote:
[...]
> Good point on reusing max_dst_opts_cnt in ip6_tnl_parse_tlv_enc_lim(), but this patch is not ready yet.
> 
> We need to be careful: max_dst_opts_cnt can be negative. If this is the case, ip6_tnl_parse_tlv_enc_lim() would probably return 0, which is not what we want here. From the doc:
> 
> max_dst_opts_number - INTEGER
>          Maximum number of non-padding TLVs allowed in a Destination
>          options extension header. If this value is less than zero
>          then unknown options are disallowed and the number of known
>          TLVs allowed is the absolute value of this number.
> 
>          Default: 8
> 
> Since ip6_tnl_parse_tlv_enc_lim() does not check for specific option types (e.g., Pad1, PadN, you-name-it) and does not differentiate known from unknown options during parsing, I would simply use the absolute value of max_dst_opts_cnt by default.
> 
> Also, I wouldn't use unlikely() because it could harm us more than it helps in this specific context (consistent with ip6_parse_tlv()).

Thanks for the review, the suggestions make sense, and I've updated them in a v2.

Cheers,
Daniel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-04-18 12:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-17 22:03 [PATCH net] ipv6: Apply max_dst_opts_cnt to ip6_tnl_parse_tlv_enc_lim Daniel Borkmann
2026-04-18 10:59 ` Justin Iurman
2026-04-18 12:17   ` Daniel Borkmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox