stable.vger.kernel.org archive mirror
 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, Yaju Cao <yacao@redhat.com>,
	"K. Y. Srinivasan" <kys@microsoft.com>,
	Haiyang Zhang <haiyangz@microsoft.com>,
	Jason Wang <jasowang@redhat.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 3.4 11/27] netvsc: dont flush peers notifying work during setting mtu
Date: Mon, 13 Jan 2014 16:26:25 -0800	[thread overview]
Message-ID: <20140114002623.686439245@linuxfoundation.org> (raw)
In-Reply-To: <20140114002623.356220317@linuxfoundation.org>

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

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

From: Jason Wang <jasowang@redhat.com>

[ Upstream commit 50dc875f2e6e2e04aed3b3033eb0ac99192d6d02 ]

There's a possible deadlock if we flush the peers notifying work during setting
mtu:

[   22.991149] ======================================================
[   22.991173] [ INFO: possible circular locking dependency detected ]
[   22.991198] 3.10.0-54.0.1.el7.x86_64.debug #1 Not tainted
[   22.991219] -------------------------------------------------------
[   22.991243] ip/974 is trying to acquire lock:
[   22.991261]  ((&(&net_device_ctx->dwork)->work)){+.+.+.}, at: [<ffffffff8108af95>] flush_work+0x5/0x2e0
[   22.991307]
but task is already holding lock:
[   22.991330]  (rtnl_mutex){+.+.+.}, at: [<ffffffff81539deb>] rtnetlink_rcv+0x1b/0x40
[   22.991367]
which lock already depends on the new lock.

[   22.991398]
the existing dependency chain (in reverse order) is:
[   22.991426]
-> #1 (rtnl_mutex){+.+.+.}:
[   22.991449]        [<ffffffff810dfdd9>] __lock_acquire+0xb19/0x1260
[   22.991477]        [<ffffffff810e0d12>] lock_acquire+0xa2/0x1f0
[   22.991501]        [<ffffffff81673659>] mutex_lock_nested+0x89/0x4f0
[   22.991529]        [<ffffffff815392b7>] rtnl_lock+0x17/0x20
[   22.991552]        [<ffffffff815230b2>] netdev_notify_peers+0x12/0x30
[   22.991579]        [<ffffffffa0340212>] netvsc_send_garp+0x22/0x30 [hv_netvsc]
[   22.991610]        [<ffffffff8108d251>] process_one_work+0x211/0x6e0
[   22.991637]        [<ffffffff8108d83b>] worker_thread+0x11b/0x3a0
[   22.991663]        [<ffffffff81095e5d>] kthread+0xed/0x100
[   22.991686]        [<ffffffff81681c6c>] ret_from_fork+0x7c/0xb0
[   22.991715]
-> #0 ((&(&net_device_ctx->dwork)->work)){+.+.+.}:
[   22.991715]        [<ffffffff810de817>] check_prevs_add+0x967/0x970
[   22.991715]        [<ffffffff810dfdd9>] __lock_acquire+0xb19/0x1260
[   22.991715]        [<ffffffff810e0d12>] lock_acquire+0xa2/0x1f0
[   22.991715]        [<ffffffff8108afde>] flush_work+0x4e/0x2e0
[   22.991715]        [<ffffffff8108e1b5>] __cancel_work_timer+0x95/0x130
[   22.991715]        [<ffffffff8108e303>] cancel_delayed_work_sync+0x13/0x20
[   22.991715]        [<ffffffffa03404e4>] netvsc_change_mtu+0x84/0x200 [hv_netvsc]
[   22.991715]        [<ffffffff815233d4>] dev_set_mtu+0x34/0x80
[   22.991715]        [<ffffffff8153bc2a>] do_setlink+0x23a/0xa00
[   22.991715]        [<ffffffff8153d054>] rtnl_newlink+0x394/0x5e0
[   22.991715]        [<ffffffff81539eac>] rtnetlink_rcv_msg+0x9c/0x260
[   22.991715]        [<ffffffff8155cdd9>] netlink_rcv_skb+0xa9/0xc0
[   22.991715]        [<ffffffff81539dfa>] rtnetlink_rcv+0x2a/0x40
[   22.991715]        [<ffffffff8155c41d>] netlink_unicast+0xdd/0x190
[   22.991715]        [<ffffffff8155c807>] netlink_sendmsg+0x337/0x750
[   22.991715]        [<ffffffff8150d219>] sock_sendmsg+0x99/0xd0
[   22.991715]        [<ffffffff8150d63e>] ___sys_sendmsg+0x39e/0x3b0
[   22.991715]        [<ffffffff8150eba2>] __sys_sendmsg+0x42/0x80
[   22.991715]        [<ffffffff8150ebf2>] SyS_sendmsg+0x12/0x20
[   22.991715]        [<ffffffff81681d19>] system_call_fastpath+0x16/0x1b

This is because we hold the rtnl_lock() before ndo_change_mtu() and try to flush
the work in netvsc_change_mtu(), in the mean time, netdev_notify_peers() may be
called from worker and also trying to hold the rtnl_lock. This will lead the
flush won't succeed forever. Solve this by not canceling and flushing the work,
this is safe because the transmission done by NETDEV_NOTIFY_PEERS was
synchronized with the netif_tx_disable() called by netvsc_change_mtu().

Reported-by: Yaju Cao <yacao@redhat.com>
Tested-by: Yaju Cao <yacao@redhat.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Acked-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/net/hyperv/netvsc_drv.c |    1 -
 1 file changed, 1 deletion(-)

--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -321,7 +321,6 @@ static int netvsc_change_mtu(struct net_
 		return -EINVAL;
 
 	nvdev->start_remove = true;
-	cancel_delayed_work_sync(&ndevctx->dwork);
 	cancel_work_sync(&ndevctx->work);
 	netif_tx_disable(ndev);
 	rndis_filter_device_remove(hdev);



  parent reply	other threads:[~2014-01-14  0:26 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-14  0:26 [PATCH 3.4 00/27] 3.4.77-stable review Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 01/27] net: do not pretend FRAGLIST support Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 02/27] rds: prevent BUG_ON triggered on congestion update to loopback Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 03/27] macvtap: Do not double-count received packets Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 04/27] macvtap: update file current position Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 05/27] tun: " Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 06/27] macvtap: signal truncated packets Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 07/27] ipv6: dont count addrconf generated routes against gc limit Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 08/27] net: drop_monitor: fix the value of maxattr Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 09/27] net: unix: allow set_peek_off to fail Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 10/27] tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0 Greg Kroah-Hartman
2014-01-14  0:26 ` Greg Kroah-Hartman [this message]
2014-01-14  0:26 ` [PATCH 3.4 12/27] net: unix: allow bind to fail on mutex lock Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 13/27] net: inet_diag: zero out uninitialized idiag_{src,dst} fields Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 14/27] drivers/net/hamradio: Integer overflow in hdlcdrv_ioctl() Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 16/27] rds: prevent dereference of a NULL device Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 17/27] net: rose: restore old recvmsg behavior Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 18/27] vlan: Fix header ops passthru when doing TX VLAN offload Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 19/27] net: llc: fix use after free in llc_ui_recvmsg Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 20/27] bridge: use spin_lock_bh() in br_multicast_set_hash_max Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 21/27] ARM: fix "bad mode in ... handler" message for undefined instructions Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 22/27] ARM: shmobile: mackerel: Fix coherent DMA mask Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 23/27] x86, fpu, amd: Clear exceptions in AMD FXSAVE workaround Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 24/27] sched: Fix race on toggling cfs_bandwidth_used Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 25/27] sched: Fix cfs_bandwidth misuse of hrtimer_expires_remaining Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 26/27] sched: Fix hrtimer_cancel()/rq->lock deadlock Greg Kroah-Hartman
2014-01-14  0:26 ` [PATCH 3.4 27/27] sched: Guarantee new group-entities always have weight Greg Kroah-Hartman
2014-01-14  2:59 ` [PATCH 3.4 00/27] 3.4.77-stable review Guenter Roeck
2014-01-14  3:03   ` Greg Kroah-Hartman
2014-01-14 19:29 ` Shuah Khan

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=20140114002623.686439245@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=haiyangz@microsoft.com \
    --cc=jasowang@redhat.com \
    --cc=kys@microsoft.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=yacao@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).