netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Eric Dumazet <edumazet@google.com>,
	AceLan Kao <acelan.kao@canonical.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Andrii Nakryiko" <andriin@fb.com>,
	"Wei Wang" <weiwan@google.com>,
	"Cong Wang" <cong.wang@bytedance.com>,
	"Taehee Yoo" <ap420073@gmail.com>,
	"Björn Töpel" <bjorn@kernel.org>, netdev <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] net: called rtnl_unlock() before runpm resumes devices
Date: Tue, 20 Apr 2021 12:27:15 -0700	[thread overview]
Message-ID: <20210420122715.2066b537@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <CANn89iJLSmtBNoDo8QJ6a0MzsHjdLB0Pf=cs9e4g8Y6-KuFiMQ@mail.gmail.com>

On Tue, 20 Apr 2021 10:34:17 +0200 Eric Dumazet wrote:
> On Tue, Apr 20, 2021 at 9:54 AM AceLan Kao <acelan.kao@canonical.com> wrote:
> >
> > From: "Chia-Lin Kao (AceLan)" <acelan.kao@canonical.com>
> >
> > The rtnl_lock() has been called in rtnetlink_rcv_msg(), and then in
> > __dev_open() it calls pm_runtime_resume() to resume devices, and in
> > some devices' resume function(igb_resum,igc_resume) they calls rtnl_lock()
> > again. That leads to a recursive lock.
> >
> > It should leave the devices' resume function to decide if they need to
> > call rtnl_lock()/rtnl_unlock(), so call rtnl_unlock() before calling
> > pm_runtime_resume() and then call rtnl_lock() after it in __dev_open().
> >
> >  
> 
> Hi Acelan
> 
> When was the bugg added ?
> Please add a Fixes: tag

For immediate cause probably:

Fixes: 9474933caf21 ("igb: close/suspend race in netif_device_detach")

> By doing so, you give more chances for reviewers to understand why the
> fix is not risky,
> and help stable teams work.

IMO the driver lacks internal locking. Taking rtnl from resume is just
one example, git history shows many more places that lacked locking and
got papered over with rtnl here.

  reply	other threads:[~2021-04-20 19:27 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-20  7:54 [PATCH] net: called rtnl_unlock() before runpm resumes devices AceLan Kao
2021-04-20  8:34 ` Eric Dumazet
2021-04-20 19:27   ` Jakub Kicinski [this message]
2021-04-22  6:30     ` AceLan Kao
2021-04-22  7:09       ` Heiner Kallweit
2021-04-23  3:42         ` AceLan Kao
2021-04-24 20:07           ` Heiner Kallweit
2021-04-26  7:36             ` AceLan Kao
2021-04-26  8:42               ` Heiner Kallweit
2021-04-27  1:58                 ` AceLan Kao
2021-04-27  6:18                   ` Heiner Kallweit
2021-04-29 11:58 ` Krzysztof Kozlowski
2021-04-29 19:33   ` Heiner Kallweit
2021-06-30  5:19     ` AceLan Kao

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=20210420122715.2066b537@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=acelan.kao@canonical.com \
    --cc=andriin@fb.com \
    --cc=ap420073@gmail.com \
    --cc=ast@kernel.org \
    --cc=bjorn@kernel.org \
    --cc=cong.wang@bytedance.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=weiwan@google.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).