All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Xin Long <lucien.xin@gmail.com>,
	syzbot+ddde1c7b7ff7442d7f2d@syzkaller.appspotmail.com,
	Florian Westphal <fw@strlen.de>, Paolo Abeni <pabeni@redhat.com>,
	Pablo Neira Ayuso <pablo@netfilter.org>,
	Krzysztof Piotr Oledzki <ole@ans.pl>
Subject: [PATCH 3.18 01/13] netfilter: drop outermost socket lock in getsockopt()
Date: Mon, 26 Feb 2018 21:15:29 +0100	[thread overview]
Message-ID: <20180226201527.311780415@linuxfoundation.org> (raw)
In-Reply-To: <20180226201527.242286068@linuxfoundation.org>

3.18-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Paolo Abeni <pabeni@redhat.com>

commit 01ea306f2ac2baff98d472da719193e738759d93 upstream.

The Syzbot reported a possible deadlock in the netfilter area caused by
rtnl lock, xt lock and socket lock being acquired with a different order
on different code paths, leading to the following backtrace:
Reviewed-by: Xin Long <lucien.xin@gmail.com>

======================================================
WARNING: possible circular locking dependency detected
4.15.0+ #301 Not tainted
------------------------------------------------------
syzkaller233489/4179 is trying to acquire lock:
  (rtnl_mutex){+.+.}, at: [<0000000048e996fd>] rtnl_lock+0x17/0x20
net/core/rtnetlink.c:74

but task is already holding lock:
  (&xt[i].mutex){+.+.}, at: [<00000000328553a2>]
xt_find_table_lock+0x3e/0x3e0 net/netfilter/x_tables.c:1041

which lock already depends on the new lock.
===

Since commit 3f34cfae1230 ("netfilter: on sockopt() acquire sock lock
only in the required scope"), we already acquire the socket lock in
the innermost scope, where needed. In such commit I forgot to remove
the outer-most socket lock from the getsockopt() path, this commit
addresses the issues dropping it now.

v1 -> v2: fix bad subj, added relavant 'fixes' tag

Fixes: 22265a5c3c10 ("netfilter: xt_TEE: resolve oif using netdevice notifiers")
Fixes: 202f59afd441 ("netfilter: ipt_CLUSTERIP: do not hold dev")
Fixes: 3f34cfae1230 ("netfilter: on sockopt() acquire sock lock only in the required scope")
Reported-by: syzbot+ddde1c7b7ff7442d7f2d@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>
Tested-by: Krzysztof Piotr Oledzki <ole@ans.pl>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 net/ipv4/ip_sockglue.c   |    7 +------
 net/ipv6/ipv6_sockglue.c |   10 ++--------
 2 files changed, 3 insertions(+), 14 deletions(-)

--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1375,10 +1375,7 @@ int ip_getsockopt(struct sock *sk, int l
 		if (get_user(len, optlen))
 			return -EFAULT;
 
-		lock_sock(sk);
-		err = nf_getsockopt(sk, PF_INET, optname, optval,
-				&len);
-		release_sock(sk);
+		err = nf_getsockopt(sk, PF_INET, optname, optval, &len);
 		if (err >= 0)
 			err = put_user(len, optlen);
 		return err;
@@ -1410,9 +1407,7 @@ int compat_ip_getsockopt(struct sock *sk
 		if (get_user(len, optlen))
 			return -EFAULT;
 
-		lock_sock(sk);
 		err = compat_nf_getsockopt(sk, PF_INET, optname, optval, &len);
-		release_sock(sk);
 		if (err >= 0)
 			err = put_user(len, optlen);
 		return err;
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -1314,10 +1314,7 @@ int ipv6_getsockopt(struct sock *sk, int
 		if (get_user(len, optlen))
 			return -EFAULT;
 
-		lock_sock(sk);
-		err = nf_getsockopt(sk, PF_INET6, optname, optval,
-				&len);
-		release_sock(sk);
+		err = nf_getsockopt(sk, PF_INET6, optname, optval, &len);
 		if (err >= 0)
 			err = put_user(len, optlen);
 	}
@@ -1356,10 +1353,7 @@ int compat_ipv6_getsockopt(struct sock *
 		if (get_user(len, optlen))
 			return -EFAULT;
 
-		lock_sock(sk);
-		err = compat_nf_getsockopt(sk, PF_INET6,
-					   optname, optval, &len);
-		release_sock(sk);
+		err = compat_nf_getsockopt(sk, PF_INET6, optname, optval, &len);
 		if (err >= 0)
 			err = put_user(len, optlen);
 	}

  reply	other threads:[~2018-02-26 20:15 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-26 20:15 [PATCH 3.18 00/13] 3.18.97-stable review Greg Kroah-Hartman
2018-02-26 20:15 ` Greg Kroah-Hartman [this message]
2018-02-26 20:15 ` [PATCH 3.18 02/13] PCI: keystone: Fix interrupt-controller-node lookup Greg Kroah-Hartman
2018-02-26 20:15 ` [PATCH 3.18 03/13] xtensa: fix high memory/reserved memory collision Greg Kroah-Hartman
2018-02-26 20:15 ` [PATCH 3.18 04/13] scsi: ibmvfc: fix misdefined reserved field in ibmvfc_fcp_rsp_info Greg Kroah-Hartman
2018-02-26 20:15 ` [PATCH 3.18 05/13] cfg80211: fix cfg80211_beacon_dup Greg Kroah-Hartman
2018-02-26 20:15   ` Greg Kroah-Hartman
2018-02-26 20:15 ` [PATCH 3.18 06/13] iio: buffer: check if a buffer has been set up when poll is called Greg Kroah-Hartman
2018-02-26 20:15 ` [PATCH 3.18 07/13] iio: adis_lib: Initialize trigger before requesting interrupt Greg Kroah-Hartman
2018-02-26 20:15 ` [PATCH 3.18 08/13] x86/oprofile: Fix bogus GCC-8 warning in nmi_setup() Greg Kroah-Hartman
2018-02-26 20:15 ` [PATCH 3.18 09/13] irqchip/gic-v3: Use wmb() instead of smb_wmb() in gic_raise_softirq() Greg Kroah-Hartman
2018-02-26 20:15 ` [PATCH 3.18 10/13] arm64: Disable unhandled signal log messages by default Greg Kroah-Hartman
2018-02-26 20:15 ` [PATCH 3.18 11/13] usb: dwc3: gadget: Set maxpacket size for ep0 IN Greg Kroah-Hartman
2018-02-26 20:15 ` [PATCH 3.18 12/13] usb: gadget: f_fs: Process all descriptors during bind Greg Kroah-Hartman
2018-02-26 20:15 ` [PATCH 3.18 13/13] ASN.1: fix out-of-bounds read when parsing indefinite length item Greg Kroah-Hartman
2018-02-27  0:53 ` [PATCH 3.18 00/13] 3.18.97-stable review Shuah Khan
2018-02-27  2:17 ` Harsh Shandilya
2018-02-27 13:10   ` Greg Kroah-Hartman
2018-02-27  4:58 ` kernelci.org bot
2018-02-27 14:53 ` Guenter Roeck
2018-02-27 18:32   ` 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=20180226201527.311780415@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=fw@strlen.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lucien.xin@gmail.com \
    --cc=ole@ans.pl \
    --cc=pabeni@redhat.com \
    --cc=pablo@netfilter.org \
    --cc=stable@vger.kernel.org \
    --cc=syzbot+ddde1c7b7ff7442d7f2d@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.