Re: [PATCH net] ipv6: Implement limits on extension header parsing

[Daniel Borkmann <daniel@iogearbox.net>]

On 4/18/26 4:15 PM, Justin Iurman wrote:
> On 4/18/26 15:46, Justin Iurman wrote:
>> On 4/18/26 15:15, Eric Dumazet wrote:
>>> On Sat, Apr 18, 2026 at 5:50 AM Justin Iurman <justin.iurman@gmail.com> wrote:
>>>> On 4/18/26 14:26, Daniel Borkmann wrote:
>>>>> On 4/18/26 1:45 PM, Justin Iurman wrote:
>>>>>> On 4/17/26 19:18, Daniel Borkmann wrote:
>>>>> [...]
>>>>>>> diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
>>>>>>> index d2cd33e2698d..93f865545a7c 100644
>>>>>>> --- a/net/ipv6/sysctl_net_ipv6.c
>>>>>>> +++ b/net/ipv6/sysctl_net_ipv6.c
>>>>>>> @@ -135,6 +135,14 @@ static struct ctl_table ipv6_table_template[] = {
>>>>>>>            .extra1        = SYSCTL_ZERO,
>>>>>>>            .extra2        = &flowlabel_reflect_max,
>>>>>>>        },
>>>>>>> +    {
>>>>>>> +        .procname    = "max_ext_hdrs_number",
>>>>>>> +        .data        = &init_net.ipv6.sysctl.max_ext_hdrs_cnt,
>>>>>>> +        .maxlen        = sizeof(int),
>>>>>>> +        .mode        = 0644,
>>>>>>> +        .proc_handler    = proc_dointvec_minmax,
>>>>>>> +        .extra1        = SYSCTL_ONE,
>>>>>>> +    },
>>>>>>>        {
>>>>>>>            .procname    = "max_dst_opts_number",
>>>>>>>            .data        = &init_net.ipv6.sysctl.max_dst_opts_cnt,
>>>>>>
>>>>>> NACKed-by: Justin Iurman <justin.iurman@gmail.com>
>>>>>>
>>>>>> +1000 on the need, but NAK on the way it is done. IMO, we don't want
>>>>>> yet-another-sysctl for that. Instead, we have (well, not yet, but it's
>>>>>> about time) this series [1] to enforce ordering and occurrences of
>>>>>> Extension Headers, which is based on an IETF draft [2] (FYI, draft-
>>>>>> ietf-6man-eh-limits is dead). I think we should enforce ordering and
>>>>>> occurrences in this code path too, instead of relying on a sysctl.
>>>>>> Let's keep both code paths consistent.
>>>>
>>>>> Hm, that series [1] should probably go to net instead of net-next, but atm
>>>>
>>>> +1, would make sense.
>>>>
>>>>> hasn't moved since a month. I'd still think max_ext_hdrs_number would be
>>>>> useful given it has less complexity also for stable, but I guess ultimately
>>>>> up to maintainers..
>>>>
>>>> In the short term, I agree. What worries me is that we end up with a
>>>> redundant, or even useless, sysctl once the other series is applied,
>>>> which will only increase user confusion.
>>>
>>> Given the amount of bugs in this code, a sysctl is safe and quire reasonable.
>>>
>>> No one will object when it is eventually removed (or has no action)
>>>
>>> For the record,  I approve Daniel patch.
>>
>> Fair enough. If there is consensus on this patch, then let me just suggest two changes:
>>
>> - make it clear in the sysctl description that it mainly applies to TX (as opposed to the other series [1] discussed earlier that applies to RX)
> 
> Sorry, I meant it does not apply to core RX (ip6_rcv()), which is what series [1] does.
> 
>> - set the default to 8 (which should be the max value) instead of 32, as per RFC8200, Sec. 4.
Ok, I'll switch to use 8 as a default limit and I'm looking to also cover ip6_rcv()
path as well in the next revision given its also affected but less severe as the
icmp6 path.

Thanks,
Daniel