Bug 198791
| Summary: | Deadlock/hang in lock_sock_nested from getorigdst | ||
|---|---|---|---|
| Product: | Networking | Reporter: | Petr Vandrovec (petr) |
| Component: | Netfilter/Iptables | Assignee: | networking_netfilter-iptables (networking_netfilter-iptables) |
| Status: | NEW --- | ||
| Severity: | normal | CC: | pabeni |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 4.16.0-rc1-64-00034 | Subsystem: | |
| Regression: | Yes | Bisected commit-id: | |
|
Description
Petr Vandrovec
2018-02-15 00:08:17 UTC
bisect says that there is a problem with move of lock_sock from ip_setsockopt into getorigdst(): is getsockopt operation, not setsockopt, and that change did not modify ip_getsockopt() to not acquire socket lock, it modified ip_setsockopt() only. So sk lock gets acquired twice, once in ip_getsockopt, and once in getorigdst :-( Perhaps intention was to remove lock from ip_getsockopt rather than ip_setsockopt? I would guess that most of nf_setsockopt stuff will need exclusion, no? commit 3f34cfae1238848fd53f25e5c8fd59da57901f4b Author: Paolo Abeni <pabeni@redhat.com> Date: Tue Jan 30 19:01:40 2018 +0100 netfilter: on sockopt() acquire sock lock only in the required scope Syzbot reported several deadlocks in the netfilter area caused by rtnl lock and socket lock being acquired with a different order on different code paths, leading to backtraces like the following one: ... The problem, as Florian noted, is that nf_setsockopt() is always called with the socket held, even if the lock itself is required only for very tight scopes and only for some operation. This patch addresses the issues moving the lock_sock() call only where really needed, namely in ipv*_getorigdst(), so that nf_setsockopt() does not need anymore to acquire both locks. Fixes: 22265a5c3c10 ("netfilter: xt_TEE: resolve oif using netdevice notifiers") Reported-by: syzbot+a4c2dc980ac1af699b36@syzkaller.appspotmail.com Suggested-by: Florian Westphal <fw@strlen.de> Signed-off-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> BTW, net/decnet/af_decnet.c seems to be broken too - both its setsockopt & getsockopt call nf_[gs]etsockopt with sk lock held, which does not match new invocation rules for IPv4/IPv6. (In reply to Petr Vandrovec from comment #1) > bisect says that there is a problem with move of lock_sock from > ip_setsockopt into getorigdst(): is getsockopt operation, not setsockopt, > and that change did not modify ip_getsockopt() to not acquire socket lock, > it modified ip_setsockopt() only. So sk lock gets acquired twice, once in > ip_getsockopt, and once in getorigdst :-( This should be fixed by the following patch: http://patchwork.ozlabs.org/patch/870827/ The patch should already be in the nf tree and should land mainline soon. Thanks, Paolo |