Re: [PATCH net-next v1 13/21] nfp: Move delink_register to be last command

From: Simon Horman
Date: Mon Sep 27 2021 - 08:21:21 EST


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@xxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxx/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@xxxxxxxxxx>

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