netdev - Re: [PATCH bpf] xsk: Initialise xskb free_list

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: <20211221122149.72160edc@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 21 Dec 2021 12:21:49 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Magnus Karlsson <magnus.karlsson@...il.com>
Cc:     "Loftus, Ciara" <ciara.loftus@...el.com>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        Björn Töpel <bjorn@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Network Development <netdev@...r.kernel.org>,
        "Fijalkowski, Maciej" <maciej.fijalkowski@...el.com>,
        bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf] xsk: Initialise xskb free_list_node

On Tue, 21 Dec 2021 10:00:13 +0100 Magnus Karlsson wrote:
> On Tue, Dec 21, 2021 at 9:32 AM Loftus, Ciara <ciara.loftus@...el.com> wrote:
> > > Thank you for this fix Ciara! Though I do think the Fixes tag should
> > > be the one above: 199d983bc015 ("xsk: Fix crash on double free in
> > > buffer pool"). Before that commit, there was no test for an empty list
> > > in the xp_free path. The entry was unconditionally put on the list and
> > > "initialized" in that way, so that code will work without this patch.
> > > What do you think?  
> >
> > Agree - that makes sense.
> > Can the fixes tag be updated when pulled into the tree with:
> > Fixes: 199d983bc015 ("xsk: Fix crash on double free in buffer pool")  
> 
> On the other hand, this was a fix for 2b43470add8c ("xsk: Introduce
> AF_XDP buffer allocation API"), the original tag you have in your
> patch. What should the Fixes tag point to in this case? Need some
> advice please.

My $0.02 would be that if all relevant commits form a chain of fixes
it doesn't matter much which one you put in the tag. To me your
suggestion of going with 199d983bc015 makes most sense since from a
cursory look the direct issue doesn't really exist without that commit.

Plus we probably don't want 199d983bc015 to be backported until we
apply this fix, so it'd be good if "Fixes: 199d983bc015" appeared in
linux-next.

You can always put multiple Fixes tags on the commit, if you're unsure.