netdev - Re: [PATCH net] net, neigh: Do not trigger immediate probes on NUD_FAILED from neigh_managed

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:   Wed, 2 Feb 2022 09:32:37 -0700
From:   David Ahern <dsahern@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>, davem@...emloft.net
Cc:     kuba@...nel.org, roopa@...dia.com, edumazet@...gle.com,
        dsahern@...nel.org, john.fastabend@...il.com,
        netdev@...r.kernel.org,
        syzbot+5239d0e1778a500d477a@...kaller.appspotmail.com
Subject: Re: [PATCH net] net, neigh: Do not trigger immediate probes on
 NUD_FAILED from neigh_managed_work

On 2/1/22 12:39 PM, Daniel Borkmann wrote:
> syzkaller was able to trigger a deadlock for NTF_MANAGED entries [0]:
> 
>   kworker/0:16/14617 is trying to acquire lock:
>   ffffffff8d4dd370 (&tbl->lock){++-.}-{2:2}, at: ___neigh_create+0x9e1/0x2990 net/core/neighbour.c:652
>   [...]
>   but task is already holding lock:
>   ffffffff8d4dd370 (&tbl->lock){++-.}-{2:2}, at: neigh_managed_work+0x35/0x250 net/core/neighbour.c:1572
> 
> The neighbor entry turned to NUD_FAILED state, where __neigh_event_send()
> triggered an immediate probe as per commit cd28ca0a3dd1 ("neigh: reduce
> arp latency") via neigh_probe() given table lock was held.
> 
> One option to fix this situation is to defer the neigh_probe() back to
> the neigh_timer_handler() similarly as pre cd28ca0a3dd1. For the case
> of NTF_MANAGED, this deferral is acceptable given this only happens on
> actual failure state and regular / expected state is NUD_VALID with the
> entry already present.
> 
> The fix adds a parameter to __neigh_event_send() in order to communicate
> whether immediate probe is allowed or disallowed. Existing call-sites
> of neigh_event_send() default as-is to immediate probe. However, the
> neigh_managed_work() disables it via use of neigh_event_send_probe().
> 

...

> 
> Fixes: 7482e3841d52 ("net, neigh: Add NTF_MANAGED flag for managed neighbor entries")
> Reported-by: syzbot+5239d0e1778a500d477a@...kaller.appspotmail.com
> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: David Ahern <dsahern@...nel.org>
> Cc: Roopa Prabhu <roopa@...dia.com>
> ---
>  include/net/neighbour.h | 18 +++++++++++++-----
>  net/core/neighbour.c    | 18 ++++++++++++------
>  2 files changed, 25 insertions(+), 11 deletions(-)
> 


Reviewed-by: David Ahern <dsahern@...nel.org>