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]
Date:   Mon, 28 Nov 2022 10:20:43 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     Ido Schimmel <idosch@...sch.org>,
        Yang Yingliang <yangyingliang@...wei.com>,
        Leon Romanovsky <leon@...nel.org>, netdev@...r.kernel.org,
        jiri@...dia.com, davem@...emloft.net, edumazet@...gle.com,
        pabeni@...hat.com
Subject: Re: [PATCH net] net: devlink: fix UAF in
 devlink_compat_running_version()

On Mon, 28 Nov 2022 10:58:58 +0100 Jiri Pirko wrote:
> >Long term, we either need to find a way to make the ethtool compat stuff
> >work correctly or just get rid of it and have affected drivers implement
> >the relevant ethtool operations instead of relying on devlink.
> >
> >[1] https://lore.kernel.org/netdev/20221122121048.776643-1-yangyingliang@huawei.com/  
> 
> I just had a call with Ido. We both think that this might be a good
> solution for -net to avoid the use after free.
> 
> For net-next, we eventually should change driver init flows to register
> devlink instance first and only after that register devlink_port and
> related netdevice. The ordering is important for the userspace app. For
> example the init flow:
> <- RTnetlink new netdev event
> app sees devlink_port handle in IFLA_DEVLINK_PORT
> -> query devlink instance using this handle  
> <- ENODEV
> 
> The instance is not registered yet.
> 
> So we need to make sure all devlink_port_register() calls are happening
> after devlink_register(). This is aligned with the original flow before
> devlink_register() was moved by Leon. Also it is aligned with devlink
> reload and devlink port split flows.

Cool. Do you also agree with doing proper refcounting for the devlink
instance struct and the liveness check after locking the instance?

Powered by blists - more mailing lists