From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org,
syzbot+a4c2dc980ac1af699b36@syzkaller.appspotmail.com,
Florian Westphal <fw@strlen.de>, Paolo Abeni <pabeni@redhat.com>,
Pablo Neira Ayuso <pablo@netfilter.org>
Subject: [PATCH 4.15 16/45] netfilter: on sockopt() acquire sock lock only in the required scope
Date: Fri, 23 Feb 2018 19:28:55 +0100 [thread overview]
Message-ID: <20180223170717.905934336@linuxfoundation.org> (raw)
In-Reply-To: <20180223170715.197760019@linuxfoundation.org>
4.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Paolo Abeni <pabeni@redhat.com>
commit 3f34cfae1238848fd53f25e5c8fd59da57901f4b upstream.
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:
======================================================
WARNING: possible circular locking dependency detected
4.15.0-rc9+ #212 Not tainted
------------------------------------------------------
syzkaller041579/3682 is trying to acquire lock:
(sk_lock-AF_INET6){+.+.}, at: [<000000008775e4dd>] lock_sock
include/net/sock.h:1463 [inline]
(sk_lock-AF_INET6){+.+.}, at: [<000000008775e4dd>]
do_ipv6_setsockopt.isra.8+0x3c5/0x39d0 net/ipv6/ipv6_sockglue.c:167
but task is already holding lock:
(rtnl_mutex){+.+.}, at: [<000000004342eaa9>] rtnl_lock+0x17/0x20
net/core/rtnetlink.c:74
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (rtnl_mutex){+.+.}:
__mutex_lock_common kernel/locking/mutex.c:756 [inline]
__mutex_lock+0x16f/0x1a80 kernel/locking/mutex.c:893
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:908
rtnl_lock+0x17/0x20 net/core/rtnetlink.c:74
register_netdevice_notifier+0xad/0x860 net/core/dev.c:1607
tee_tg_check+0x1a0/0x280 net/netfilter/xt_TEE.c:106
xt_check_target+0x22c/0x7d0 net/netfilter/x_tables.c:845
check_target net/ipv6/netfilter/ip6_tables.c:538 [inline]
find_check_entry.isra.7+0x935/0xcf0
net/ipv6/netfilter/ip6_tables.c:580
translate_table+0xf52/0x1690 net/ipv6/netfilter/ip6_tables.c:749
do_replace net/ipv6/netfilter/ip6_tables.c:1165 [inline]
do_ip6t_set_ctl+0x370/0x5f0 net/ipv6/netfilter/ip6_tables.c:1691
nf_sockopt net/netfilter/nf_sockopt.c:106 [inline]
nf_setsockopt+0x67/0xc0 net/netfilter/nf_sockopt.c:115
ipv6_setsockopt+0x115/0x150 net/ipv6/ipv6_sockglue.c:928
udpv6_setsockopt+0x45/0x80 net/ipv6/udp.c:1422
sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2978
SYSC_setsockopt net/socket.c:1849 [inline]
SyS_setsockopt+0x189/0x360 net/socket.c:1828
entry_SYSCALL_64_fastpath+0x29/0xa0
-> #0 (sk_lock-AF_INET6){+.+.}:
lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:3914
lock_sock_nested+0xc2/0x110 net/core/sock.c:2780
lock_sock include/net/sock.h:1463 [inline]
do_ipv6_setsockopt.isra.8+0x3c5/0x39d0 net/ipv6/ipv6_sockglue.c:167
ipv6_setsockopt+0xd7/0x150 net/ipv6/ipv6_sockglue.c:922
udpv6_setsockopt+0x45/0x80 net/ipv6/udp.c:1422
sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2978
SYSC_setsockopt net/socket.c:1849 [inline]
SyS_setsockopt+0x189/0x360 net/socket.c:1828
entry_SYSCALL_64_fastpath+0x29/0xa0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(rtnl_mutex);
lock(sk_lock-AF_INET6);
lock(rtnl_mutex);
lock(sk_lock-AF_INET6);
*** DEADLOCK ***
1 lock held by syzkaller041579/3682:
#0: (rtnl_mutex){+.+.}, at: [<000000004342eaa9>] rtnl_lock+0x17/0x20
net/core/rtnetlink.c:74
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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/ipv4/ip_sockglue.c | 14 ++++----------
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 6 +++++-
net/ipv6/ipv6_sockglue.c | 17 +++++------------
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 18 ++++++++++++------
4 files changed, 26 insertions(+), 29 deletions(-)
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1251,11 +1251,8 @@ int ip_setsockopt(struct sock *sk, int l
if (err == -ENOPROTOOPT && optname != IP_HDRINCL &&
optname != IP_IPSEC_POLICY &&
optname != IP_XFRM_POLICY &&
- !ip_mroute_opt(optname)) {
- lock_sock(sk);
+ !ip_mroute_opt(optname))
err = nf_setsockopt(sk, PF_INET, optname, optval, optlen);
- release_sock(sk);
- }
#endif
return err;
}
@@ -1280,12 +1277,9 @@ int compat_ip_setsockopt(struct sock *sk
if (err == -ENOPROTOOPT && optname != IP_HDRINCL &&
optname != IP_IPSEC_POLICY &&
optname != IP_XFRM_POLICY &&
- !ip_mroute_opt(optname)) {
- lock_sock(sk);
- err = compat_nf_setsockopt(sk, PF_INET, optname,
- optval, optlen);
- release_sock(sk);
- }
+ !ip_mroute_opt(optname))
+ err = compat_nf_setsockopt(sk, PF_INET, optname, optval,
+ optlen);
#endif
return err;
}
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
@@ -218,15 +218,19 @@ getorigdst(struct sock *sk, int optval,
struct nf_conntrack_tuple tuple;
memset(&tuple, 0, sizeof(tuple));
+
+ lock_sock(sk);
tuple.src.u3.ip = inet->inet_rcv_saddr;
tuple.src.u.tcp.port = inet->inet_sport;
tuple.dst.u3.ip = inet->inet_daddr;
tuple.dst.u.tcp.port = inet->inet_dport;
tuple.src.l3num = PF_INET;
tuple.dst.protonum = sk->sk_protocol;
+ release_sock(sk);
/* We only do TCP and SCTP at the moment: is there a better way? */
- if (sk->sk_protocol != IPPROTO_TCP && sk->sk_protocol != IPPROTO_SCTP) {
+ if (tuple.dst.protonum != IPPROTO_TCP &&
+ tuple.dst.protonum != IPPROTO_SCTP) {
pr_debug("SO_ORIGINAL_DST: Not a TCP/SCTP socket\n");
return -ENOPROTOOPT;
}
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -923,12 +923,8 @@ int ipv6_setsockopt(struct sock *sk, int
#ifdef CONFIG_NETFILTER
/* we need to exclude all possible ENOPROTOOPTs except default case */
if (err == -ENOPROTOOPT && optname != IPV6_IPSEC_POLICY &&
- optname != IPV6_XFRM_POLICY) {
- lock_sock(sk);
- err = nf_setsockopt(sk, PF_INET6, optname, optval,
- optlen);
- release_sock(sk);
- }
+ optname != IPV6_XFRM_POLICY)
+ err = nf_setsockopt(sk, PF_INET6, optname, optval, optlen);
#endif
return err;
}
@@ -958,12 +954,9 @@ int compat_ipv6_setsockopt(struct sock *
#ifdef CONFIG_NETFILTER
/* we need to exclude all possible ENOPROTOOPTs except default case */
if (err == -ENOPROTOOPT && optname != IPV6_IPSEC_POLICY &&
- optname != IPV6_XFRM_POLICY) {
- lock_sock(sk);
- err = compat_nf_setsockopt(sk, PF_INET6, optname,
- optval, optlen);
- release_sock(sk);
- }
+ optname != IPV6_XFRM_POLICY)
+ err = compat_nf_setsockopt(sk, PF_INET6, optname, optval,
+ optlen);
#endif
return err;
}
--- a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
@@ -226,20 +226,27 @@ static const struct nf_hook_ops ipv6_con
static int
ipv6_getorigdst(struct sock *sk, int optval, void __user *user, int *len)
{
- const struct inet_sock *inet = inet_sk(sk);
+ struct nf_conntrack_tuple tuple = { .src.l3num = NFPROTO_IPV6 };
const struct ipv6_pinfo *inet6 = inet6_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
const struct nf_conntrack_tuple_hash *h;
struct sockaddr_in6 sin6;
- struct nf_conntrack_tuple tuple = { .src.l3num = NFPROTO_IPV6 };
struct nf_conn *ct;
+ __be32 flow_label;
+ int bound_dev_if;
+ lock_sock(sk);
tuple.src.u3.in6 = sk->sk_v6_rcv_saddr;
tuple.src.u.tcp.port = inet->inet_sport;
tuple.dst.u3.in6 = sk->sk_v6_daddr;
tuple.dst.u.tcp.port = inet->inet_dport;
tuple.dst.protonum = sk->sk_protocol;
+ bound_dev_if = sk->sk_bound_dev_if;
+ flow_label = inet6->flow_label;
+ release_sock(sk);
- if (sk->sk_protocol != IPPROTO_TCP && sk->sk_protocol != IPPROTO_SCTP)
+ if (tuple.dst.protonum != IPPROTO_TCP &&
+ tuple.dst.protonum != IPPROTO_SCTP)
return -ENOPROTOOPT;
if (*len < 0 || (unsigned int) *len < sizeof(sin6))
@@ -257,14 +264,13 @@ ipv6_getorigdst(struct sock *sk, int opt
sin6.sin6_family = AF_INET6;
sin6.sin6_port = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.u.tcp.port;
- sin6.sin6_flowinfo = inet6->flow_label & IPV6_FLOWINFO_MASK;
+ sin6.sin6_flowinfo = flow_label & IPV6_FLOWINFO_MASK;
memcpy(&sin6.sin6_addr,
&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.u3.in6,
sizeof(sin6.sin6_addr));
nf_ct_put(ct);
- sin6.sin6_scope_id = ipv6_iface_scope_id(&sin6.sin6_addr,
- sk->sk_bound_dev_if);
+ sin6.sin6_scope_id = ipv6_iface_scope_id(&sin6.sin6_addr, bound_dev_if);
return copy_to_user(user, &sin6, sizeof(sin6)) ? -EFAULT : 0;
}
next prev parent reply other threads:[~2018-02-23 18:28 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-23 18:28 [PATCH 4.15 00/45] 4.15.6-stable review Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 01/45] tun: fix tun_napi_alloc_frags() frag allocator Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 02/45] ptr_ring: fail early if queue occupies more than KMALLOC_MAX_SIZE Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 03/45] ptr_ring: try vmalloc() when kmalloc() fails Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 04/45] selinux: ensure the context is NUL terminated in security_context_to_sid_core() Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 05/45] selinux: skip bounded transition processing if the policy isnt loaded Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 06/45] media: pvrusb2: properly check endpoint types Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 07/45] crypto: x86/twofish-3way - Fix %rbp usage Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 08/45] staging: android: ion: Add __GFP_NOWARN for system contig heap Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 09/45] staging: android: ion: Switch from WARN to pr_warn Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 10/45] blk_rq_map_user_iov: fix error override Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 11/45] KVM: x86: fix escape of guest dr6 to the host Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 12/45] kcov: detect double association with a single task Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 13/45] netfilter: x_tables: fix int overflow in xt_alloc_table_info() Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 14/45] netfilter: x_tables: avoid out-of-bounds reads in xt_request_find_{match|target} Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 15/45] netfilter: ipt_CLUSTERIP: fix out-of-bounds accesses in clusterip_tg_check() Greg Kroah-Hartman
2018-02-23 18:28 ` Greg Kroah-Hartman [this message]
2018-02-23 18:28 ` [PATCH 4.15 17/45] netfilter: xt_cgroup: initialize info->priv in cgroup_mt_check_v1() Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 18/45] netfilter: xt_RATEEST: acquire xt_rateest_mutex for hash insert Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 19/45] rds: tcp: correctly sequence cleanup on netns deletion Greg Kroah-Hartman
2018-02-23 18:28 ` [PATCH 4.15 20/45] rds: tcp: atomically purge entries from rds_tcp_conn_list during netns delete Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 21/45] net: avoid skb_warn_bad_offload on IS_ERR Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 22/45] net_sched: gen_estimator: fix lockdep splat Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 23/45] soc: qcom: rmtfs_mem: add missing MODULE_DESCRIPTION/AUTHOR/LICENSE Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 24/45] ASoC: ux500: add MODULE_LICENSE tag Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 25/45] video: fbdev/mmp: add MODULE_LICENSE Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 26/45] ARM: 8743/1: bL_switcher: add MODULE_LICENSE tag Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 27/45] arm64: dts: add #cooling-cells to CPU nodes Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 28/45] dn_getsockoptdecnet: move nf_{get/set}sockopt outside sock lock Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 29/45] ANDROID: binder: remove WARN() for redundant txn error Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 30/45] ANDROID: binder: synchronize_rcu() when using POLLFREE Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 31/45] staging: android: ashmem: Fix a race condition in pin ioctls Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 32/45] binder: check for binder_thread allocation failure in binder_poll() Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 33/45] binder: replace "%p" with "%pK" Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 34/45] staging: fsl-mc: fix build testing on x86 Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 35/45] staging: iio: adc: ad7192: fix external frequency setting Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 36/45] staging: iio: ad5933: switch buffer mode to software Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 37/45] xhci: Fix NULL pointer in xhci debugfs Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 38/45] xhci: Fix xhci debugfs devices node disappearance after hibernation Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 39/45] xhci: xhci debugfs device nodes werent removed after device plugged out Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 40/45] xhci: fix xhci debugfs errors in xhci_stop Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 41/45] usbip: keep usbip_device sockfd state in sync with tcp_socket Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 42/45] crypto: s5p-sss - Fix kernel Oops in AES-ECB mode Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 43/45] mei: me: add cannon point device ids Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 44/45] mei: me: add cannon point device ids for 4th device Greg Kroah-Hartman
2018-02-23 18:29 ` [PATCH 4.15 45/45] vmalloc: fix __GFP_HIGHMEM usage for vmalloc_32 on 32b systems Greg Kroah-Hartman
2018-02-23 23:57 ` [PATCH 4.15 00/45] 4.15.6-stable review kernelci.org bot
2018-02-24 0:38 ` Shuah Khan
2018-02-24 8:26 ` Greg Kroah-Hartman
2018-02-24 17:58 ` Guenter Roeck
2018-02-25 9:59 ` Greg Kroah-Hartman
2018-02-25 3:37 ` Dan Rue
2018-02-25 9:58 ` Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180223170717.905934336@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=fw@strlen.de \
--cc=linux-kernel@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=pablo@netfilter.org \
--cc=stable@vger.kernel.org \
--cc=syzbot+a4c2dc980ac1af699b36@syzkaller.appspotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.