[PATCH net v2] ipv6: Apply max_dst_opts_cnt to ip6_tnl_parse_tlv_enc_lim

[PATCH net v2] ipv6: Apply max_dst_opts_cnt to ip6_tnl_parse_tlv_enc_lim

[Daniel Borkmann]

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>
---
 v1->v2:
   - Remove unlikely (Justin)
   - Use abs() given max_dst_opts_cnt's negative meaning (Justin)

 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..0f50b7fcb24e 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 = abs(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 (tlv_cnt++ >= tlv_max)
+					break;
+
 				/* No more room for encapsulation limit */
 				if (i + sizeof(*tel) > optlen)
 					break;
-- 
2.43.0

Re: [PATCH net v2] ipv6: Apply max_dst_opts_cnt to ip6_tnl_parse_tlv_enc_lim

[Justin Iurman]

On 4/18/26 14:15, 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>
> ---
>   v1->v2:
>     - Remove unlikely (Justin)
>     - Use abs() given max_dst_opts_cnt's negative meaning (Justin)
> 
>   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..0f50b7fcb24e 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 = abs(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 (tlv_cnt++ >= tlv_max)
> +					break;
> +
>   				/* No more room for encapsulation limit */
>   				if (i + sizeof(*tel) > optlen)
>   					break;

Thanks for v2, Daniel.

I'm still wondering: should we align the above parsing behavior with the 
one in ip6_parse_tlv() to keep things consistent? That is: don't 
increment tlv_cnt for Pad1/PadN, make sure we don't exceed 8 bytes per 
padding (consecutive Pad1's, or a PadN), and we could also check that a 
PadN payload is only made of zeroes. Open question...

Otherwise, LGTM:
Reviewed-by: Justin Iurman <justin.iurman@gmail.com>

Re: [PATCH net v2] ipv6: Apply max_dst_opts_cnt to ip6_tnl_parse_tlv_enc_lim

[Daniel Borkmann]

On 4/18/26 2:40 PM, Justin Iurman wrote:
> On 4/18/26 14:15, 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>
>> ---
>>   v1->v2:
>>     - Remove unlikely (Justin)
>>     - Use abs() given max_dst_opts_cnt's negative meaning (Justin)
>>
>>   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..0f50b7fcb24e 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 = abs(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 (tlv_cnt++ >= tlv_max)
>> +                    break;
>> +
>>                   /* No more room for encapsulation limit */
>>                   if (i + sizeof(*tel) > optlen)
>>                       break;
> 
> Thanks for v2, Daniel.
> 
> I'm still wondering: should we align the above parsing behavior with the one in ip6_parse_tlv() to keep things consistent? That is: don't increment tlv_cnt for Pad1/PadN, make sure we don't exceed 8 bytes per padding (consecutive Pad1's, or a PadN), and we could also check that a PadN payload is only made of zeroes. Open question...

Hm, so it would be sth like the below on top of this one, I'd wish we wouldn't
have to open code another ip6_parse_tlv().

Have you seen such cases with the encap limit in the wild (vs encap limit as
first tlv) where a stack places leading Pad1/PadN before encap limit which the
v2 patch wouldn't catch?

  net/ipv6/ip6_tunnel.c | 42 +++++++++++++++++++++++++++++++-----------
  1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 0f50b7fcb24e..1fa42a1cd97c 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -432,28 +432,48 @@ __u16 ip6_tnl_parse_tlv_enc_lim(struct sk_buff *skb, __u8 *raw)
  		if (nexthdr == NEXTHDR_DEST) {
  			int tlv_max = abs(READ_ONCE(init_net.ipv6.sysctl.max_dst_opts_cnt));
  			int tlv_cnt = 0;
+			int padlen = 0;
  			u16 i = 2;
  
-			while (1) {
-				struct ipv6_tlv_tnl_enc_lim *tel;
+			while (i < optlen) {
+				struct ipv6_tlv_tnl_enc_lim *tel =
+					(struct ipv6_tlv_tnl_enc_lim *)(skb->data + off + i);
+				int tel_len;
  
-				if (tlv_cnt++ >= tlv_max)
+				if (tel->type == IPV6_TLV_PAD1) {
+					if (++padlen > 7)
+						break;
+					i++;
+					continue;
+				}
+
+				if (i + 2 > optlen)
+					break;
+				tel_len = tel->length + 2;
+				if (i + tel_len > optlen)
  					break;
  
-				/* No more room for encapsulation limit */
-				if (i + sizeof(*tel) > optlen)
+				if (tel->type == IPV6_TLV_PADN) {
+					padlen += tel_len;
+					if (padlen > 7)
+						break;
+					if (memchr_inv((u8 *)tel + 2, 0,
+						       tel->length))
+						break;
+					i += tel_len;
+					continue;
+				}
+				padlen = 0;
+
+				if (tlv_cnt++ >= tlv_max)
  					break;
  
-				tel = (struct ipv6_tlv_tnl_enc_lim *)(skb->data + off + i);
  				/* return index of option if found and valid */
  				if (tel->type == IPV6_TLV_TNL_ENCAP_LIMIT &&
  				    tel->length == 1)
  					return i + off - nhoff;
-				/* else jump to next option */
-				if (tel->type)
-					i += tel->length + 2;
-				else
-					i++;
+
+				i += tel_len;
  			}
  		}
  		nexthdr = hdr->nexthdr;
-- 
2.43.0

> Otherwise, LGTM:
> Reviewed-by: Justin Iurman <justin.iurman@gmail.com>

Thanks,
Daniel

Re: [PATCH net v2] ipv6: Apply max_dst_opts_cnt to ip6_tnl_parse_tlv_enc_lim

[Justin Iurman]

On 4/19/26 00:19, Daniel Borkmann wrote:
> On 4/18/26 2:40 PM, Justin Iurman wrote:
>> On 4/18/26 14:15, 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>
>>> ---
>>>   v1->v2:
>>>     - Remove unlikely (Justin)
>>>     - Use abs() given max_dst_opts_cnt's negative meaning (Justin)
>>>
>>>   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..0f50b7fcb24e 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 = 
>>> abs(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 (tlv_cnt++ >= tlv_max)
>>> +                    break;
>>> +
>>>                   /* No more room for encapsulation limit */
>>>                   if (i + sizeof(*tel) > optlen)
>>>                       break;
>>
>> Thanks for v2, Daniel.
>>
>> I'm still wondering: should we align the above parsing behavior with 
>> the one in ip6_parse_tlv() to keep things consistent? That is: don't 
>> increment tlv_cnt for Pad1/PadN, make sure we don't exceed 8 bytes per 
>> padding (consecutive Pad1's, or a PadN), and we could also check that 
>> a PadN payload is only made of zeroes. Open question...
> 
> Hm, so it would be sth like the below on top of this one, I'd wish we 
> wouldn't
> have to open code another ip6_parse_tlv().
> 
> Have you seen such cases with the encap limit in the wild (vs encap 
> limit as
> first tlv) where a stack places leading Pad1/PadN before encap limit 
> which the
> v2 patch wouldn't catch?

Nope. But if it happens, users would be confused as max_dst_opts_cnt 
would not have the same meaning in two different code paths. OTOH, I 
agree that such situation would look suspicious. I guess it's fine to 
keep your patch as is and to not over-complicate things unnecessarily.

>   net/ipv6/ip6_tunnel.c | 42 +++++++++++++++++++++++++++++++-----------
>   1 file changed, 31 insertions(+), 11 deletions(-)
> 
> diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
> index 0f50b7fcb24e..1fa42a1cd97c 100644
> --- a/net/ipv6/ip6_tunnel.c
> +++ b/net/ipv6/ip6_tunnel.c
> @@ -432,28 +432,48 @@ __u16 ip6_tnl_parse_tlv_enc_lim(struct sk_buff 
> *skb, __u8 *raw)
>           if (nexthdr == NEXTHDR_DEST) {
>               int tlv_max = 
> abs(READ_ONCE(init_net.ipv6.sysctl.max_dst_opts_cnt));
>               int tlv_cnt = 0;
> +            int padlen = 0;
>               u16 i = 2;
> 
> -            while (1) {
> -                struct ipv6_tlv_tnl_enc_lim *tel;
> +            while (i < optlen) {
> +                struct ipv6_tlv_tnl_enc_lim *tel =
> +                    (struct ipv6_tlv_tnl_enc_lim *)(skb->data + off + i);
> +                int tel_len;
> 
> -                if (tlv_cnt++ >= tlv_max)
> +                if (tel->type == IPV6_TLV_PAD1) {
> +                    if (++padlen > 7)
> +                        break;
> +                    i++;
> +                    continue;
> +                }
> +
> +                if (i + 2 > optlen)
> +                    break;
> +                tel_len = tel->length + 2;
> +                if (i + tel_len > optlen)
>                       break;
> 
> -                /* No more room for encapsulation limit */
> -                if (i + sizeof(*tel) > optlen)
> +                if (tel->type == IPV6_TLV_PADN) {
> +                    padlen += tel_len;
> +                    if (padlen > 7)
> +                        break;
> +                    if (memchr_inv((u8 *)tel + 2, 0,
> +                               tel->length))
> +                        break;
> +                    i += tel_len;
> +                    continue;
> +                }
> +                padlen = 0;
> +
> +                if (tlv_cnt++ >= tlv_max)
>                       break;
> 
> -                tel = (struct ipv6_tlv_tnl_enc_lim *)(skb->data + off + 
> i);
>                   /* return index of option if found and valid */
>                   if (tel->type == IPV6_TLV_TNL_ENCAP_LIMIT &&
>                       tel->length == 1)
>                       return i + off - nhoff;
> -                /* else jump to next option */
> -                if (tel->type)
> -                    i += tel->length + 2;
> -                else
> -                    i++;
> +
> +                i += tel_len;
>               }
>           }
>           nexthdr = hdr->nexthdr;

Re: [PATCH net v2] ipv6: Apply max_dst_opts_cnt to ip6_tnl_parse_tlv_enc_lim

[Ido Schimmel]

On Sun, Apr 19, 2026 at 12:37:35AM +0200, Justin Iurman wrote:
> Nope. But if it happens, users would be confused as max_dst_opts_cnt would
> not have the same meaning in two different code paths. OTOH, I agree that
> such situation would look suspicious. I guess it's fine to keep your patch
> as is and to not over-complicate things unnecessarily.

I agree that it's weird to reuse max_dst_opts_cnt here:

1. The meaning is different from the Rx path.

2. We only enforce max_dst_opts_cnt, but not max_dst_opts_len.

3. The default is derived from the initial netns, unlike in the Rx path.

Given the above and that:

1. We believe that 8 options until the tunnel encapsulation limit option
is liberal enough.

2. We don't want to over-complicate things.

Can we go with an hard coded 8 and see if anyone complains? In the
unlikely case that someone complains we can at least gain some insight
into how this option is actually used with tunnels.

Re: [PATCH net v2] ipv6: Apply max_dst_opts_cnt to ip6_tnl_parse_tlv_enc_lim

[Justin Iurman]

On 4/19/26 16:31, Ido Schimmel wrote:
> On Sun, Apr 19, 2026 at 12:37:35AM +0200, Justin Iurman wrote:
>> Nope. But if it happens, users would be confused as max_dst_opts_cnt would
>> not have the same meaning in two different code paths. OTOH, I agree that
>> such situation would look suspicious. I guess it's fine to keep your patch
>> as is and to not over-complicate things unnecessarily.
> 
> I agree that it's weird to reuse max_dst_opts_cnt here:
> 
> 1. The meaning is different from the Rx path.
> 
> 2. We only enforce max_dst_opts_cnt, but not max_dst_opts_len.
> 
> 3. The default is derived from the initial netns, unlike in the Rx path.
> 
> Given the above and that:
> 
> 1. We believe that 8 options until the tunnel encapsulation limit option
> is liberal enough.
> 
> 2. We don't want to over-complicate things.
> 
> Can we go with an hard coded 8 and see if anyone complains? In the
> unlikely case that someone complains we can at least gain some insight
> into how this option is actually used with tunnels.

In general, I'm not a big fan of hard-coded values, but I also think 
that in this context it would make sense to do so. This is not a strong 
+1, let's say it's more a "not against it".

Re: [PATCH net v2] ipv6: Apply max_dst_opts_cnt to ip6_tnl_parse_tlv_enc_lim

[Daniel Borkmann]

On 4/20/26 8:55 PM, Justin Iurman wrote:
> On 4/19/26 16:31, Ido Schimmel wrote:
>> On Sun, Apr 19, 2026 at 12:37:35AM +0200, Justin Iurman wrote:
>>> Nope. But if it happens, users would be confused as max_dst_opts_cnt would
>>> not have the same meaning in two different code paths. OTOH, I agree that
>>> such situation would look suspicious. I guess it's fine to keep your patch
>>> as is and to not over-complicate things unnecessarily.
>>
>> I agree that it's weird to reuse max_dst_opts_cnt here:
>>
>> 1. The meaning is different from the Rx path.
>>
>> 2. We only enforce max_dst_opts_cnt, but not max_dst_opts_len.
>>
>> 3. The default is derived from the initial netns, unlike in the Rx path.
>>
>> Given the above and that:
>>
>> 1. We believe that 8 options until the tunnel encapsulation limit option
>> is liberal enough.
>>
>> 2. We don't want to over-complicate things.
>>
>> Can we go with an hard coded 8 and see if anyone complains? In the
>> unlikely case that someone complains we can at least gain some insight
>> into how this option is actually used with tunnels.
> 
> In general, I'm not a big fan of hard-coded values, but I also think that in this context it would make sense to do so. This is not a strong +1, let's say it's more a "not against it".

Makes sense, I'll update it in a v3.