netdev - Re: [PATCH net 1/4] net: fix double-free on fraglist GSO skbs

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: <6f4db46541880179766a30cf6d5e47f44190b98d.camel@redhat.com>
Date:   Fri, 07 May 2021 10:46:15 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>,
        Steffen Klassert <steffen.klassert@...unet.com>,
        Willem de Bruijn <willemb@...gle.com>,
        Miaohe Lin <linmiaohe@...wei.com>
Subject: Re: [PATCH net 1/4] net: fix double-free on fraglist GSO skbs

On Thu, 2021-05-06 at 14:17 -0700, Jakub Kicinski wrote:
> On Thu, 06 May 2021 17:55:36 +0200 Paolo Abeni wrote:
> > On Thu, 2021-05-06 at 10:32 -0400, Willem de Bruijn wrote:
> > > On Thu, May 6, 2021 at 7:07 AM Paolo Abeni <pabeni@...hat.com> wrote:  
> > > > If we want to be safe about future possible sock_wfree users, I think
> > > > the approach here should be different: in skb_segment(), tail-  
> > > > > destructor is expected to be NULL, while skb_segment_list(), all the  
> > > > list skbs can be owned by the same socket. Possibly we could open-
> > > > code skb_release_head_state(), omitting the skb orphaning part
> > > > for sock_wfree() destructor.
> > > > 
> > > > Note that the this is not currently needed - sock_wfree destructor
> > > > can't reach there.
> > > > 
> > > > Given all the above, I'm unsure if you are fine with (or at least do
> > > > not oppose to) the code proposed in this patch?  
> > > 
> > > Yes. Thanks for clarifying, Paolo.  
> > 
> > Thank you for reviewing!
> > 
> > @David, @Jakub: I see this series is already archived as "change
> > requested", should I repost?
> 
> Yes, please. Patch 2 adds two new sparse warnings. 
> 
> I think you need csum_unfold() to go from __sum16 to __wsum.

Yes, indeed. I'll send a v2 with such change, thanks!

Paolo
>