netdev - Re: [PATCH] net: dpaa2-eth: fix use-after-free in dpaa2_eth

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:   Tue, 16 Nov 2021 07:16:49 +0300
From:   Pavel Skripkin <paskripkin@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>,
        Dan Carpenter <dan.carpenter@...cle.com>
Cc:     ioana.ciornei@....com, davem@...emloft.net, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] net: dpaa2-eth: fix use-after-free in dpaa2_eth_remove

On 11/16/21 04:27, Jakub Kicinski wrote:
> I'd ignore that path, it's just special casing that's supposed to keep
> the driver-visible API sane. Nobody should be touching netdev past
> free_netdev(). Actually if you can it'd be interesting to add checks
> for using whatever netdev_priv(ndev) returned past free_netdev(ndev).
> 
> Most UAFs that come to mind from the past were people doing something
> like:
> 
> 	struct my_priv *mine = netdev_priv(ndev);
> 
> 	netdev_unregister(ndev);
> 	free_netdev(ndev);
> 
> 	free(mine->bla); /* UAF, free_netdev() frees the priv */
> 
I've implemented this checker couple of months ago. The latest smatch 
(v1.72) should warn about this type of bugs. All reported bugs are fixed 
already :)

My checker warns about using priv pointer after free_netdev() and 
free_candev() calls. There are a few more wrappers like 
free_sja1000dev(), so it worth to add them to check list too. Will add 
them today later


Important thing, that there are complex situations like

	struct priv *priv = get_priv_from_smth(smth);

	free_netdev(priv->netdev);
	clean_up_priv(priv);

and for now I have no idea how to handle it (ex: ems_usb_disconnect).




With regards,
Pavel Skripkin