netdev - Re: [PATCH net-next] netdevsim: Register and unregister devlink traps on probe/remove device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YXU5+XLhQ9zkBGNY@shredder>
Date:   Sun, 24 Oct 2021 13:48:25 +0300
From:   Ido Schimmel <idosch@...sch.org>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Ido Schimmel <idosch@...lanox.com>,
        Jiri Pirko <jiri@...lanox.com>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org,
        syzbot+93d5accfaefceedf43c1@...kaller.appspotmail.com
Subject: Re: [PATCH net-next] netdevsim: Register and unregister devlink
 traps on probe/remove device

On Sun, Oct 24, 2021 at 12:54:52PM +0300, Leon Romanovsky wrote:
> On Sun, Oct 24, 2021 at 12:05:12PM +0300, Ido Schimmel wrote:
> > On Sun, Oct 24, 2021 at 11:42:11AM +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@...dia.com>
> > > 
> > > Align netdevsim to be like all other physical devices that register and
> > > unregister devlink traps during their probe and removal respectively.
> > 
> > No, this is incorrect. Out of the three drivers that support both reload
> > and traps, both netdevsim and mlxsw unregister the traps during reload.
> > Here is another report from syzkaller about mlxsw [1].
> 
> Sorry, I overlooked it.
> 
> > 
> > Please revert both 22849b5ea595 ("devlink: Remove not-executed trap
> > policer notifications") and 8bbeed485823 ("devlink: Remove not-executed
> > trap group notifications").
> 
> However, before we rush and revert commit, can you please explain why
> current behavior to reregister traps on reload is correct?
> 
> I think that you are not changing traps during reload, so traps before
> reload will be the same as after reload, am I right?

During reload we tear down the entire driver and load it again. As part
of the reload_down() operation we tear down the various objects from
both devlink and the device (e.g., shared buffer, ports, traps, etc.).
As part of the reload_up() operation we issue a device reset and
register everything back.

While the list of objects doesn't change, their properties (e.g., shared
buffer size, trap action, policer rate) do change back to the default
after reload and we cannot go back on that as it's a user-visible
change.