All of lore.kernel.org
 help / color / mirror / Atom feed
From: Willy Tarreau <w@1wt.eu>
To: Jakub Kicinski <kuba@kernel.org>
Cc: syzbot <syzbot+6f8ddb9f2ff4adf065cb@syzkaller.appspotmail.com>,
	davem@davemloft.net, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, syzkaller-bugs@googlegroups.com
Subject: Re: [syzbot] WARNING: refcount bug in __linkwatch_run_queue
Date: Wed, 17 Nov 2021 15:20:12 +0100	[thread overview]
Message-ID: <20211117142012.GB6276@1wt.eu> (raw)
In-Reply-To: <20211117061548.63c25223@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

On Wed, Nov 17, 2021 at 06:15:48AM -0800, Jakub Kicinski wrote:
> On Wed, 17 Nov 2021 09:19:07 +0100 Willy Tarreau wrote:
> > Thanks for the report. I'm seeing that linkwatch_do_dev() is also
> > called in linkwatch_forget_dev(), and am wondering if we're not
> > seeing a sequence like this one:
> > 
> >   linkwatch_forget_dev()
> >     list_del_init()
> >     linkwatch_do_dev()
> >       netdev_state_change()
> >         ... one of the notifiers
> >            ... linkwatch_add_event() => adds to watch list
> >       dev_put()
> >   ...
> >   
> >   __linkwatch_run_queue()
> >     linkwatch_do_dev()
> >       dev_put()
> >         => bang!  
> > 
> > Well, in theory, no, since linkwatch_add_event() will call dev_hold()
> > when adding to the list, so we ought to leave the first call with a
> > refcount still covering the list's presence, and I don't see how it
> > can reach zero before reaching dev_put() in linkwatch_do_dev() as this
> > function is only called when the event was picked from the list.
> > 
> > The only difference I'm seeing is that before the patch, a call to
> > linkwatch_forget_dev() on a non-present device would call dev_put()
> > without going through dev_activate(), dev_deactivate(), nor
> > netdev_state_change(), but I'm not seeing how that could make a
> > difference. linkwatch_forget_dev() is called from netdev_wait_allrefs()
> > which will wait for the refcnt to be exactly 1, thus even if we queue
> > an extra event we cant leave that function until the event has been
> > processed.
> 
> The ref leak could come from anywhere, tho. Like:
> 
> https://lore.kernel.org/all/87a6i3t2zg.fsf@nvidia.com/

OK thanks for the link, so better wait for this part to clarify itself
and see if the issue magically disappears ?

Willy

  reply	other threads:[~2021-11-17 14:21 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-16  9:23 [syzbot] WARNING: refcount bug in __linkwatch_run_queue syzbot
2021-11-17  8:19 ` Willy Tarreau
2021-11-17 14:15   ` Jakub Kicinski
2021-11-17 14:20     ` Willy Tarreau [this message]
2021-11-17 14:22       ` Jakub Kicinski

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=20211117142012.GB6276@1wt.eu \
    --to=w@1wt.eu \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=syzbot+6f8ddb9f2ff4adf065cb@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.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.