netdev - Re: [PATCH bpf-next v6 7/7] selftests/bpf: Add tests for XDP bonding

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: <0bb57862-c8aa-0279-84ae-77f9cbdc47c2@iogearbox.net>
Date:   Mon, 9 Aug 2021 23:41:31 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Jussi Maki <joamaki@...il.com>,
        Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        j.vosburgh@...il.com, Andy Gospodarek <andy@...yhouse.net>,
        vfalico@...il.com, Andrii Nakryiko <andrii@...nel.org>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        Magnus Karlsson <magnus.karlsson@...el.com>
Subject: Re: [PATCH bpf-next v6 7/7] selftests/bpf: Add tests for XDP bonding

On 8/9/21 4:24 PM, Jussi Maki wrote:
[...]
>>> +       if (!test__start_subtest("xdp_bonding_redirect_multi"))
>>> +               test_xdp_bonding_redirect_multi(&skeletons);
>>> +
>>> +out:
>>> +       xdp_dummy__destroy(skeletons.xdp_dummy);
>>> +       xdp_tx__destroy(skeletons.xdp_tx);
>>> +       xdp_redirect_multi_kern__destroy(skeletons.xdp_redirect_multi_kern);
>>> +
>>> +       libbpf_set_print(old_print_fn);
>>> +       if (root_netns_fd)
>>
>> technically, fd could be 0, so for fds we have if (fd >= 0)
>> everywhere. Also, if open() above fails, root_netns_fd will be -1 and
>> you'll still attempt to close it.
> 
> Good catch. Daniel, could you fix this when applying to be "if
> (root_netns_fd >= 0)"?

Yep, done now, I had to rebase due to 220ade77452c ("bonding: 3ad: fix the concurrency
between __bond_release_one() and bond_3ad_state_machine_handler()") which this series
here didn't take into account. Please double check.

Thanks everyone,
Daniel