netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Leon Romanovsky <leon@kernel.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Jiri Pirko <jiri@resnulli.us>, Ido Schimmel <idosch@idosch.org>,
	Yang Yingliang <yangyingliang@huawei.com>,
	netdev@vger.kernel.org, jiri@nvidia.com, davem@davemloft.net,
	edumazet@google.com, pabeni@redhat.com
Subject: Re: [PATCH net] net: devlink: fix UAF in devlink_compat_running_version()
Date: Wed, 30 Nov 2022 21:20:34 +0200	[thread overview]
Message-ID: <Y4etAg+vcnRCMWx9@unreal> (raw)
In-Reply-To: <20221130092042.0c223a8c@kernel.org>

On Wed, Nov 30, 2022 at 09:20:42AM -0800, Jakub Kicinski wrote:
> On Wed, 30 Nov 2022 18:00:05 +0100 Jiri Pirko wrote:
> > Wed, Nov 30, 2022 at 05:46:59PM CET, kuba@kernel.org wrote:
> > >On Wed, 30 Nov 2022 12:42:39 +0100 Jiri Pirko wrote:  
> > >> **)
> > >> I see. With the change I suggest, meaning doing
> > >> devlink_port_register/unregister() and netdev_register/unregister only
> > >> for registered devlink instance, you don't need this at all. When you
> > >> hit this compat callback, the netdevice is there and therefore devlink
> > >> instance is registered for sure.  
> > >
> > >If you move devlink registration up it has to be under the instance
> > >lock, otherwise we're back to reload problems. That implies unregister
> > >should be under the lock too. But then we can't wait for refs in
> > >unregister. Perhaps I don't understand the suggestion.  
> > 
> > I unlock for register and for the rest of the init I lock again.
> 
> The moment you register that instance callbacks can start coming.
> Leon move the register call last for a good reason - all drivers
> we looked at had bugs in handling init.

Plus we had very cozy lock->unlock->lock sequences during devlink
command execution, which caused to races between devlink calls
and driver initialization.

So I'm also interested to see what Jiri meant by saying "I unlock for
register and for the rest of the init I lock again".

Thanks

  reply	other threads:[~2022-11-30 19:21 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-22 12:10 [PATCH net] net: devlink: fix UAF in devlink_compat_running_version() Yang Yingliang
2022-11-22 14:32 ` Leon Romanovsky
2022-11-22 15:25   ` Yang Yingliang
2022-11-22 19:04     ` Leon Romanovsky
2022-11-22 20:27       ` Jakub Kicinski
2022-11-23  1:50         ` Yang Yingliang
2022-11-23  6:40         ` Yang Yingliang
2022-11-23  7:41           ` Leon Romanovsky
2022-11-23  8:34             ` Yang Yingliang
2022-11-23  9:33               ` Leon Romanovsky
2022-11-23 19:18               ` Ido Schimmel
2022-11-24  2:18                 ` Jakub Kicinski
2022-11-24  5:56                   ` Leon Romanovsky
2022-11-28  9:20                   ` Ido Schimmel
2022-11-28  9:58                     ` Jiri Pirko
2022-11-28 11:50                       ` Leon Romanovsky
2022-11-28 13:52                         ` Jiri Pirko
2022-11-29  8:44                           ` Leon Romanovsky
2022-11-29  9:05                             ` Jiri Pirko
2022-11-29 11:20                               ` Leon Romanovsky
2022-11-29 11:44                                 ` Jiri Pirko
2022-11-28 18:20                       ` Jakub Kicinski
2022-11-29  8:31                         ` Jiri Pirko
2022-11-30  2:18                           ` Jakub Kicinski
2022-11-30  8:54                             ` Leon Romanovsky
2022-11-30 11:32                               ` Jiri Pirko
2022-11-30 16:36                               ` Jakub Kicinski
2022-11-30 11:42                             ` Jiri Pirko
2022-11-30 16:46                               ` Jakub Kicinski
2022-11-30 17:00                                 ` Jiri Pirko
2022-11-30 17:20                                   ` Jakub Kicinski
2022-11-30 19:20                                     ` Leon Romanovsky [this message]
2022-12-01  8:40                                       ` Jiri Pirko
2022-12-01 10:05                                         ` Leon Romanovsky
2022-12-01 12:20                                           ` Jiri Pirko
2022-12-01  8:39                                     ` Jiri Pirko
2022-11-30 22:25                 ` Jacob Keller
2022-11-24  2:20           ` Jakub Kicinski
2022-11-24  2:47           ` Jakub Kicinski
2022-11-24  7:28             ` Yang Yingliang
2022-11-28 10:01 ` Jiri Pirko

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=Y4etAg+vcnRCMWx9@unreal \
    --to=leon@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=idosch@idosch.org \
    --cc=jiri@nvidia.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=yangyingliang@huawei.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).