lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 26 May 2019 10:50:09 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Daniel Borkmann <daniel@...earbox.net>,
        John Hurley <john.hurley@...ronome.com>,
        netdev@...r.kernel.org, jiri@...lanox.com, davem@...emloft.net,
        xiyou.wangcong@...il.com
Cc:     simon.horman@...ronome.com, jakub.kicinski@...ronome.com,
        oss-drivers@...ronome.com, alexei.starovoitov@...il.com
Subject: Re: [RFC net-next 1/1] net: sched: protect against loops in TC filter
 hooks

On 2019-05-24 2:32 p.m., Daniel Borkmann wrote:
> On 05/24/2019 06:05 PM, John Hurley wrote:
>> TC hooks allow the application of filters and actions to packets at both
>> ingress and egress of the network stack. It is possible, with poor
>> configuration, that this can produce loops whereby an ingress hook calls
>> a mirred egress action that has an egress hook that redirects back to
>> the first ingress etc. The TC core classifier protects against loops when
>> doing reclassifies but, as yet, there is no protection against a packet
>> looping between multiple hooks. This can lead to stack overflow panics.
>>
>> Add a per cpu counter that tracks recursion of packets through TC hooks.
>> The packet will be dropped if a recursive limit is passed and the counter
>> reset for the next packet.
> 
> NAK. This is quite a hack in the middle of the fast path. Such redirection
> usually has a rescheduling point, like in cls_bpf case. If this is not the
> case for mirred action as I read your commit message above, then fix mirred
> instead if it's such broken.
> 

Actually a per-cpu approach will *never* work. This has to be per-skb
(since the skb is looping).
Take a look at the (new) skb metadata encoding approach from Florian;
you should be able to encode the counters in the skb.

cheers,
jamal

Powered by blists - more mailing lists