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, Guillaume Nault <gnault@redhat.com>,
	syzbot+3f960c64a104eaa2c813@syzkaller.appspotmail.com,
	Taehee Yoo <ap420073@gmail.com>, Jakub Kicinski <kuba@kernel.org>
Subject: [PATCH 5.8 53/56] Revert "netns: dont disable BHs when locking "nsid_lock""
Date: Fri, 25 Sep 2020 14:48:43 +0200	[thread overview]
Message-ID: <20200925124735.734169325@linuxfoundation.org> (raw)
In-Reply-To: <20200925124727.878494124@linuxfoundation.org>

From: Taehee Yoo <ap420073@gmail.com>

[ Upstream commit e1f469cd5866499ac40bfdca87411e1c525a10c7 ]

This reverts commit 8d7e5dee972f1cde2ba96c621f1541fa36e7d4f4.

To protect netns id, the nsid_lock is used when netns id is being
allocated and removed by peernet2id_alloc() and unhash_nsid().
The nsid_lock can be used in BH context but only spin_lock() is used
in this code.
Using spin_lock() instead of spin_lock_bh() can result in a deadlock in
the following scenario reported by the lockdep.
In order to avoid a deadlock, the spin_lock_bh() should be used instead
of spin_lock() to acquire nsid_lock.

Test commands:
    ip netns del nst
    ip netns add nst
    ip link add veth1 type veth peer name veth2
    ip link set veth1 netns nst
    ip netns exec nst ip link add name br1 type bridge vlan_filtering 1
    ip netns exec nst ip link set dev br1 up
    ip netns exec nst ip link set dev veth1 master br1
    ip netns exec nst ip link set dev veth1 up
    ip netns exec nst ip link add macvlan0 link br1 up type macvlan

Splat looks like:
[   33.615860][  T607] WARNING: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected
[   33.617194][  T607] 5.9.0-rc1+ #665 Not tainted
[ ... ]
[   33.670615][  T607] Chain exists of:
[   33.670615][  T607]   &mc->mca_lock --> &bridge_netdev_addr_lock_key --> &net->nsid_lock
[   33.670615][  T607]
[   33.673118][  T607]  Possible interrupt unsafe locking scenario:
[   33.673118][  T607]
[   33.674599][  T607]        CPU0                    CPU1
[   33.675557][  T607]        ----                    ----
[   33.676516][  T607]   lock(&net->nsid_lock);
[   33.677306][  T607]                                local_irq_disable();
[   33.678517][  T607]                                lock(&mc->mca_lock);
[   33.679725][  T607]                                lock(&bridge_netdev_addr_lock_key);
[   33.681166][  T607]   <Interrupt>
[   33.681791][  T607]     lock(&mc->mca_lock);
[   33.682579][  T607]
[   33.682579][  T607]  *** DEADLOCK ***
[ ... ]
[   33.922046][  T607] stack backtrace:
[   33.922999][  T607] CPU: 3 PID: 607 Comm: ip Not tainted 5.9.0-rc1+ #665
[   33.924099][  T607] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
[   33.925714][  T607] Call Trace:
[   33.926238][  T607]  dump_stack+0x78/0xab
[   33.926905][  T607]  check_irq_usage+0x70b/0x720
[   33.927708][  T607]  ? iterate_chain_key+0x60/0x60
[   33.928507][  T607]  ? check_path+0x22/0x40
[   33.929201][  T607]  ? check_noncircular+0xcf/0x180
[   33.930024][  T607]  ? __lock_acquire+0x1952/0x1f20
[   33.930860][  T607]  __lock_acquire+0x1952/0x1f20
[   33.931667][  T607]  lock_acquire+0xaf/0x3a0
[   33.932366][  T607]  ? peernet2id_alloc+0x3a/0x170
[   33.933147][  T607]  ? br_port_fill_attrs+0x54c/0x6b0 [bridge]
[   33.934140][  T607]  ? br_port_fill_attrs+0x5de/0x6b0 [bridge]
[   33.935113][  T607]  ? kvm_sched_clock_read+0x14/0x30
[   33.935974][  T607]  _raw_spin_lock+0x30/0x70
[   33.936728][  T607]  ? peernet2id_alloc+0x3a/0x170
[   33.937523][  T607]  peernet2id_alloc+0x3a/0x170
[   33.938313][  T607]  rtnl_fill_ifinfo+0xb5e/0x1400
[   33.939091][  T607]  rtmsg_ifinfo_build_skb+0x8a/0xf0
[   33.939953][  T607]  rtmsg_ifinfo_event.part.39+0x17/0x50
[   33.940863][  T607]  rtmsg_ifinfo+0x1f/0x30
[   33.941571][  T607]  __dev_notify_flags+0xa5/0xf0
[   33.942376][  T607]  ? __irq_work_queue_local+0x49/0x50
[   33.943249][  T607]  ? irq_work_queue+0x1d/0x30
[   33.943993][  T607]  ? __dev_set_promiscuity+0x7b/0x1a0
[   33.944878][  T607]  __dev_set_promiscuity+0x7b/0x1a0
[   33.945758][  T607]  dev_set_promiscuity+0x1e/0x50
[   33.946582][  T607]  br_port_set_promisc+0x1f/0x40 [bridge]
[   33.947487][  T607]  br_manage_promisc+0x8b/0xe0 [bridge]
[   33.948388][  T607]  __dev_set_promiscuity+0x123/0x1a0
[   33.949244][  T607]  __dev_set_rx_mode+0x68/0x90
[   33.950021][  T607]  dev_uc_add+0x50/0x60
[   33.950720][  T607]  macvlan_open+0x18e/0x1f0 [macvlan]
[   33.951601][  T607]  __dev_open+0xd6/0x170
[   33.952269][  T607]  __dev_change_flags+0x181/0x1d0
[   33.953056][  T607]  rtnl_configure_link+0x2f/0xa0
[   33.953884][  T607]  __rtnl_newlink+0x6b9/0x8e0
[   33.954665][  T607]  ? __lock_acquire+0x95d/0x1f20
[   33.955450][  T607]  ? lock_acquire+0xaf/0x3a0
[   33.956193][  T607]  ? is_bpf_text_address+0x5/0xe0
[   33.956999][  T607]  rtnl_newlink+0x47/0x70

Acked-by: Guillaume Nault <gnault@redhat.com>
Fixes: 8d7e5dee972f ("netns: don't disable BHs when locking "nsid_lock"")
Reported-by: syzbot+3f960c64a104eaa2c813@syzkaller.appspotmail.com
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/core/net_namespace.c |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -251,10 +251,10 @@ int peernet2id_alloc(struct net *net, st
 	if (refcount_read(&net->count) == 0)
 		return NETNSA_NSID_NOT_ASSIGNED;
 
-	spin_lock(&net->nsid_lock);
+	spin_lock_bh(&net->nsid_lock);
 	id = __peernet2id(net, peer);
 	if (id >= 0) {
-		spin_unlock(&net->nsid_lock);
+		spin_unlock_bh(&net->nsid_lock);
 		return id;
 	}
 
@@ -264,12 +264,12 @@ int peernet2id_alloc(struct net *net, st
 	 * just been idr_remove()'d from there in cleanup_net().
 	 */
 	if (!maybe_get_net(peer)) {
-		spin_unlock(&net->nsid_lock);
+		spin_unlock_bh(&net->nsid_lock);
 		return NETNSA_NSID_NOT_ASSIGNED;
 	}
 
 	id = alloc_netid(net, peer, -1);
-	spin_unlock(&net->nsid_lock);
+	spin_unlock_bh(&net->nsid_lock);
 
 	put_net(peer);
 	if (id < 0)
@@ -534,20 +534,20 @@ static void unhash_nsid(struct net *net,
 	for_each_net(tmp) {
 		int id;
 
-		spin_lock(&tmp->nsid_lock);
+		spin_lock_bh(&tmp->nsid_lock);
 		id = __peernet2id(tmp, net);
 		if (id >= 0)
 			idr_remove(&tmp->netns_ids, id);
-		spin_unlock(&tmp->nsid_lock);
+		spin_unlock_bh(&tmp->nsid_lock);
 		if (id >= 0)
 			rtnl_net_notifyid(tmp, RTM_DELNSID, id, 0, NULL,
 					  GFP_KERNEL);
 		if (tmp == last)
 			break;
 	}
-	spin_lock(&net->nsid_lock);
+	spin_lock_bh(&net->nsid_lock);
 	idr_destroy(&net->netns_ids);
-	spin_unlock(&net->nsid_lock);
+	spin_unlock_bh(&net->nsid_lock);
 }
 
 static LLIST_HEAD(cleanup_list);
@@ -760,9 +760,9 @@ static int rtnl_net_newid(struct sk_buff
 		return PTR_ERR(peer);
 	}
 
-	spin_lock(&net->nsid_lock);
+	spin_lock_bh(&net->nsid_lock);
 	if (__peernet2id(net, peer) >= 0) {
-		spin_unlock(&net->nsid_lock);
+		spin_unlock_bh(&net->nsid_lock);
 		err = -EEXIST;
 		NL_SET_BAD_ATTR(extack, nla);
 		NL_SET_ERR_MSG(extack,
@@ -771,7 +771,7 @@ static int rtnl_net_newid(struct sk_buff
 	}
 
 	err = alloc_netid(net, peer, nsid);
-	spin_unlock(&net->nsid_lock);
+	spin_unlock_bh(&net->nsid_lock);
 	if (err >= 0) {
 		rtnl_net_notifyid(net, RTM_NEWNSID, err, NETLINK_CB(skb).portid,
 				  nlh, GFP_KERNEL);



  parent reply	other threads:[~2020-09-25 12:57 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-25 12:47 [PATCH 5.8 00/56] 5.8.12-rc1 review Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 01/56] ibmvnic fix NULL tx_pools and rx_tools issue at do_reset Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 02/56] ibmvnic: add missing parenthesis in do_reset() Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 03/56] act_ife: load meta modules before tcf_idr_check_alloc() Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 04/56] bnxt_en: Avoid sending firmware messages when AER error is detected Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 05/56] bnxt_en: Fix NULL ptr dereference crash in bnxt_fw_reset_task() Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 06/56] cxgb4: fix memory leak during module unload Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 07/56] cxgb4: Fix offset when clearing filter byte counters Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 08/56] geneve: add transport ports in route lookup for geneve Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 09/56] hdlc_ppp: add range checks in ppp_cp_parse_cr() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 10/56] hinic: bump up the timeout of SET_FUNC_STATE cmd Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 11/56] ip: fix tos reflection in ack and reset packets Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 12/56] ipv4: Initialize flowi4_multipath_hash in data path Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 13/56] ipv4: Update exception handling for multipath routes via same device Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 14/56] ipv6: avoid lockdep issue in fib6_del() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 15/56] net: bridge: br_vlan_get_pvid_rcu() should dereference the VLAN group under RCU Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 16/56] net: DCB: Validate DCB_ATTR_DCB_BUFFER argument Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 17/56] net: dsa: rtl8366: Properly clear member config Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 18/56] net: Fix bridge enslavement failure Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 19/56] net: ipv6: fix kconfig dependency warning for IPV6_SEG6_HMAC Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 20/56] net/mlx5: Fix FTE cleanup Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 21/56] net: phy: call phy_disable_interrupts() in phy_attach_direct() instead Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 22/56] net: sched: initialize with 0 before setting erspan md->u Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 23/56] net: sch_generic: aviod concurrent reset and enqueue op for lockless qdisc Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 24/56] net: sctp: Fix IPv6 ancestor_size calc in sctp_copy_descendant Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 25/56] nfp: use correct define to return NONE fec Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 26/56] taprio: Fix allowing too small intervals Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 27/56] tipc: Fix memory leak in tipc_group_create_member() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 28/56] tipc: fix shutdown() of connection oriented socket Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 29/56] tipc: use skb_unshare() instead in tipc_buf_append() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 30/56] net/mlx5e: Enable adding peer miss rules only if merged eswitch is supported Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 31/56] net/mlx5e: TLS, Do not expose FPGA TLS counter if not supported Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 32/56] bnxt_en: Use memcpy to copy VPD field info Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 33/56] bnxt_en: return proper error codes in bnxt_show_temp Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 34/56] bnxt_en: Protect bnxt_set_eee() and bnxt_set_pauseparam() with mutex Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 35/56] net: lantiq: Wake TX queue again Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 36/56] net: lantiq: use netif_tx_napi_add() for TX NAPI Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 37/56] net: lantiq: Use napi_complete_done() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 38/56] net: lantiq: Disable IRQs only if NAPI gets scheduled Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 39/56] net: phy: Avoid NPD upon phy_detach() when driver is unbound Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 40/56] net: phy: Do not warn in phy_stop() on PHY_DOWN Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 41/56] net: qrtr: check skb_put_padto() return value Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 42/56] net: add __must_check to skb_put_padto() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 43/56] net: ethernet: ti: cpsw_new: fix suspend/resume Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 44/56] wireguard: noise: take lock when removing handshake entry from table Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 45/56] wireguard: peerlookup: take lock before checking hash in replace operation Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 46/56] net: ipa: fix u32_replace_bits by u32p_xxx version Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 47/56] net/mlx5e: Fix memory leak of tunnel info when rule under multipath not ready Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 48/56] hinic: fix rewaking txq after netif_tx_disable Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 49/56] hv_netvsc: Fix hibernation for mlx5 VF driver Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 50/56] net: dsa: link interfaces with the DSA master to get rid of lockdep warnings Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 51/56] net: dsa: microchip: ksz8795: really set the correct number of ports Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 52/56] net: macb: fix for pause frame receive enable bit Greg Kroah-Hartman
2020-09-25 12:48 ` Greg Kroah-Hartman [this message]
2020-09-25 12:48 ` [PATCH 5.8 54/56] net/mlx5e: Use RCU to protect rq->xdp_prog Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 55/56] net/mlx5e: Use synchronize_rcu to sync with NAPI Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 56/56] net/mlx5e: Fix endianness when calculating pedit mask first bit Greg Kroah-Hartman
2020-09-25 18:01 ` [PATCH 5.8 00/56] 5.8.12-rc1 review Jon Hunter
2020-09-26 16:09   ` Greg Kroah-Hartman
2020-09-25 18:32 ` Naresh Kamboju
2020-09-26 16:10   ` Greg Kroah-Hartman
2020-09-25 20:01 ` Shuah Khan
2020-09-26 16:10   ` Greg Kroah-Hartman
2020-09-25 21:22 ` Jeffrin Jose T
2020-09-26 16:10   ` Greg Kroah-Hartman
2020-09-26 15:44 ` Guenter Roeck
2020-09-26 16:10   ` 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=20200925124735.734169325@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=ap420073@gmail.com \
    --cc=gnault@redhat.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=syzbot+3f960c64a104eaa2c813@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.