All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <simon.horman@corigine.com>
To: Leon Romanovsky <leon@kernel.org>
Cc: "David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Andrew Lunn <andrew@lunn.ch>, Ariel Elior <aelior@marvell.com>,
	Bin Luo <luobin9@huawei.com>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Coiby Xu <coiby.xu@gmail.com>,
	Derek Chickles <dchickles@marvell.com>,
	drivers@pensando.io, Felix Manlunas <fmanlunas@marvell.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Geetha sowjanya <gakula@marvell.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	GR-everest-linux-l2@marvell.com, GR-Linux-NIC-Dev@marvell.com,
	hariprasad <hkelam@marvell.com>, Ido Schimmel <idosch@nvidia.com>,
	Intel Corporation <linuxwwan@intel.com>,
	intel-wired-lan@lists.osuosl.org,
	Ioana Ciornei <ioana.ciornei@nxp.com>,
	Jerin Jacob <jerinj@marvell.com>,
	Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Jiri Pirko <jiri@nvidia.com>,
	Jonathan Lemon <jonathan.lemon@gmail.com>,
	Linu Cherian <lcherian@marvell.com>,
	linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-rdma@vger.kernel.org, linux-staging@lists.linux.dev,
	Loic Poulain <loic.poulain@linaro.org>,
	Manish Chopra <manishc@marvell.com>,
	M Chetan Kumar <m.chetan.kumar@intel.com>,
	Michael Chan <michael.chan@broadcom.com>,
	Michael Guralnik <michaelgur@mellanox.com>,
	netdev@vger.kernel.org, oss-drivers@corigine.com,
	Richard Cochran <richardcochran@gmail.com>,
	Saeed Mahameed <saeedm@nvidia.com>,
	Satanand Burla <sburla@marvell.com>,
	Sergey Ryazanov <ryazanov.s.a@gmail.com>,
	Shannon Nelson <snelson@pensando.io>,
	Subbaraya Sundeep <sbhatta@marvell.com>,
	Sunil Goutham <sgoutham@marvell.com>,
	Taras Chornyi <tchornyi@marvell.com>,
	Tariq Toukan <tariqt@nvidia.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	UNGLinuxDriver@microchip.com, Vadym Kochan <vkochan@marvell.com>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>
Subject: Re: [PATCH net-next v1 13/21] nfp: Move delink_register to be last command
Date: Mon, 27 Sep 2021 14:20:42 +0200	[thread overview]
Message-ID: <20210927122041.GA12315@corigine.com> (raw)
In-Reply-To: <YVGwtNEcWSgYvyyV@unreal>

On Mon, Sep 27, 2021 at 02:53:24PM +0300, Leon Romanovsky wrote:
> On Mon, Sep 27, 2021 at 10:39:24AM +0200, Simon Horman wrote:
> > On Sat, Sep 25, 2021 at 02:22:53PM +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@nvidia.com>
> > > 
> > > Open user space access to the devlink after driver is probed.
> > 
> > Hi Leon,
> > 
> > I think a description of why is warranted here.
> 
> After devlink_register(), users can send GET and SET netlink commands to
> the uninitialized driver. In some cases, nothing will happen, but not in
> all and hard to prove that ALL drivers are safe with such early access.
> 
> It means that local users can (in theory for some and in practice for
> others) crash the system (or leverage permissions) with early devlink_register()
> by accessing internal to driver pointers that are not set yet.
> 
> Like I said in the commit message, I'm not fixing all drivers.
> https://lore.kernel.org/netdev/cover.1632565508.git.leonro@nvidia.com/T/#m063eb4e67389bafcc3b3ddc07197bf43181b7209
> 
> Because some of the driver authors made a wonderful job to obfuscate their
> driver and write completely unmanageable code.
> 
> I do move devlink_register() to be last devlink command for all drivers,
> to allow me to clean devlink core locking and API in next series.
> 
> This series should raise your eyebrow and trigger a question: "is my
> driver vulnerable too?". And the answer will depend on devlink_register()
> position in the .probe() call.
> 
> Thanks

Thanks for the explanation.
And thanks for taking time to update the NFP driver.

> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

Acked-by: Simon Horman <simon.horman@corigine.com>


WARNING: multiple messages have this Message-ID (diff)
From: Simon Horman <simon.horman@corigine.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH net-next v1 13/21] nfp: Move delink_register to be last command
Date: Mon, 27 Sep 2021 14:20:42 +0200	[thread overview]
Message-ID: <20210927122041.GA12315@corigine.com> (raw)
In-Reply-To: <YVGwtNEcWSgYvyyV@unreal>

On Mon, Sep 27, 2021 at 02:53:24PM +0300, Leon Romanovsky wrote:
> On Mon, Sep 27, 2021 at 10:39:24AM +0200, Simon Horman wrote:
> > On Sat, Sep 25, 2021 at 02:22:53PM +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@nvidia.com>
> > > 
> > > Open user space access to the devlink after driver is probed.
> > 
> > Hi Leon,
> > 
> > I think a description of why is warranted here.
> 
> After devlink_register(), users can send GET and SET netlink commands to
> the uninitialized driver. In some cases, nothing will happen, but not in
> all and hard to prove that ALL drivers are safe with such early access.
> 
> It means that local users can (in theory for some and in practice for
> others) crash the system (or leverage permissions) with early devlink_register()
> by accessing internal to driver pointers that are not set yet.
> 
> Like I said in the commit message, I'm not fixing all drivers.
> https://lore.kernel.org/netdev/cover.1632565508.git.leonro at nvidia.com/T/#m063eb4e67389bafcc3b3ddc07197bf43181b7209
> 
> Because some of the driver authors made a wonderful job to obfuscate their
> driver and write completely unmanageable code.
> 
> I do move devlink_register() to be last devlink command for all drivers,
> to allow me to clean devlink core locking and API in next series.
> 
> This series should raise your eyebrow and trigger a question: "is my
> driver vulnerable too?". And the answer will depend on devlink_register()
> position in the .probe() call.
> 
> Thanks

Thanks for the explanation.
And thanks for taking time to update the NFP driver.

> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

Acked-by: Simon Horman <simon.horman@corigine.com>


  reply	other threads:[~2021-09-27 12:20 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-25 11:22 [PATCH net-next v1 00/21] Move devlink_register to be last devlink command Leon Romanovsky
2021-09-25 11:22 ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 01/21] devlink: Notify users when objects are accessible Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-28  2:49   ` Eric Dumazet
2021-09-28  2:49     ` [Intel-wired-lan] " Eric Dumazet
2021-09-28  7:34     ` Leon Romanovsky
2021-09-28  7:34       ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 02/21] bnxt_en: Register devlink instance at the end devlink configuration Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 03/21] liquidio: Overcome missing device lock protection in init/remove flows Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 04/21] dpaa2-eth: Register devlink instance at the end of probe Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 05/21] net: hinic: Open device for the user access when it is ready Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 06/21] ice: Open devlink when device " Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-27 19:47   ` Jesse Brandeburg
2021-09-27 19:47     ` [Intel-wired-lan] " Jesse Brandeburg
2021-09-25 11:22 ` [PATCH net-next v1 07/21] octeontx2: Move devlink registration to be last devlink command Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 08/21] net/prestera: Split devlink and traps registrations to separate routines Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 09/21] net/mlx4: Move devlink_register to be the last initialization command Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 10/21] net/mlx5: Accept devlink user input after driver initialization complete Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 11/21] mlxsw: core: Register devlink instance last Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-26 14:56   ` Ido Schimmel
2021-09-26 14:56     ` [Intel-wired-lan] " Ido Schimmel
2021-09-26 14:56     ` Ido Schimmel
2021-09-25 11:22 ` [PATCH net-next v1 12/21] net: mscc: ocelot: delay devlink registration to the end Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 13/21] nfp: Move delink_register to be last command Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-27  8:39   ` Simon Horman
2021-09-27  8:39     ` [Intel-wired-lan] " Simon Horman
2021-09-27 11:53     ` Leon Romanovsky
2021-09-27 11:53       ` [Intel-wired-lan] " Leon Romanovsky
2021-09-27 12:20       ` Simon Horman [this message]
2021-09-27 12:20         ` Simon Horman
2021-09-25 11:22 ` [PATCH net-next v1 14/21] ionic: Move devlink registration to be last devlink command Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-27 16:07   ` Shannon Nelson
2021-09-27 16:07     ` [Intel-wired-lan] " Shannon Nelson
2021-09-25 11:22 ` [PATCH net-next v1 15/21] qed: " Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 16/21] net: ethernet: ti: " Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 17/21] netdevsim: " Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 18/21] net: wwan: iosm: Move devlink_register " Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:22 ` [PATCH net-next v1 19/21] ptp: ocp: Move devlink registration " Leon Romanovsky
2021-09-25 11:22   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:23 ` [PATCH net-next v1 20/21] staging: qlge: " Leon Romanovsky
2021-09-25 11:23   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-25 11:23 ` [PATCH net-next v1 21/21] net: dsa: " Leon Romanovsky
2021-09-25 11:23   ` [Intel-wired-lan] " Leon Romanovsky
2021-09-29 13:02   ` Vladimir Oltean
2021-09-29 13:02     ` [Intel-wired-lan] " Vladimir Oltean
2021-09-29 13:07     ` Leon Romanovsky
2021-09-29 13:07       ` [Intel-wired-lan] " Leon Romanovsky

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=20210927122041.GA12315@corigine.com \
    --to=simon.horman@corigine.com \
    --cc=GR-Linux-NIC-Dev@marvell.com \
    --cc=GR-everest-linux-l2@marvell.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=aelior@marvell.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andrew@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=claudiu.manoil@nxp.com \
    --cc=coiby.xu@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dchickles@marvell.com \
    --cc=drivers@pensando.io \
    --cc=f.fainelli@gmail.com \
    --cc=fmanlunas@marvell.com \
    --cc=gakula@marvell.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hkelam@marvell.com \
    --cc=idosch@nvidia.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=ioana.ciornei@nxp.com \
    --cc=jerinj@marvell.com \
    --cc=jesse.brandeburg@intel.com \
    --cc=jiri@nvidia.com \
    --cc=jonathan.lemon@gmail.com \
    --cc=kuba@kernel.org \
    --cc=lcherian@marvell.com \
    --cc=leon@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=linuxwwan@intel.com \
    --cc=loic.poulain@linaro.org \
    --cc=luobin9@huawei.com \
    --cc=m.chetan.kumar@intel.com \
    --cc=manishc@marvell.com \
    --cc=michael.chan@broadcom.com \
    --cc=michaelgur@mellanox.com \
    --cc=netdev@vger.kernel.org \
    --cc=oss-drivers@corigine.com \
    --cc=richardcochran@gmail.com \
    --cc=ryazanov.s.a@gmail.com \
    --cc=saeedm@nvidia.com \
    --cc=sbhatta@marvell.com \
    --cc=sburla@marvell.com \
    --cc=sgoutham@marvell.com \
    --cc=snelson@pensando.io \
    --cc=tariqt@nvidia.com \
    --cc=tchornyi@marvell.com \
    --cc=vivien.didelot@gmail.com \
    --cc=vkochan@marvell.com \
    --cc=vladimir.oltean@nxp.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.