netdev - Re: [PATCH net] net: fix use-after-free in __netif_receive_skb

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: <20190710224724.GA28254@bistromath.localdomain>
Date:   Thu, 11 Jul 2019 00:47:24 +0200
From:   Sabrina Dubroca <sd@...asysnail.net>
To:     Edward Cree <ecree@...arflare.com>
Cc:     netdev@...r.kernel.org, Andreas Steinmetz <ast@...dv.de>
Subject: Re: [PATCH net] net: fix use-after-free in __netif_receive_skb_core

2019-07-10, 16:07:43 +0100, Edward Cree wrote:
> On 10/07/2019 14:52, Sabrina Dubroca wrote:
> > -static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc,
> > +static int __netif_receive_skb_core(struct sk_buff **pskb, bool pfmemalloc,
> >  				    struct packet_type **ppt_prev)
> >  {
> >  	struct packet_type *ptype, *pt_prev;
> >  	rx_handler_func_t *rx_handler;
> > +	struct sk_buff *skb = *pskb;
> Would it not be simpler just to change all users of skb to *pskb?
> Then you avoid having to keep doing "*pskb = skb;" whenever skb changes
>  (with concomitant risk of bugs if one gets missed).

Yes, that would be less risky. I wrote a version of the patch that did
exactly that, but found it really too ugly (both the patch and the
resulting code). We have more than 50 occurences of skb, including
things like:

    atomic_long_inc(&skb->dev->rx_dropped);

-- 
Sabrina