netdev - Re: [PATCH bpf-next] selftests/bpf: Restore the netns after flow dissector reattach test

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] [day] [month] [year] [list]

Message-ID: <87pniucthc.fsf@cloudflare.com>
Date:   Fri, 18 Oct 2019 11:07:11 +0200
From:   Jakub Sitnicki <jakub@...udflare.com>
To:     Martin Lau <kafai@...com>
Cc:     "bpf\@vger.kernel.org" <bpf@...r.kernel.org>,
        "netdev\@vger.kernel.org" <netdev@...r.kernel.org>,
        "kernel-team\@cloudflare.com" <kernel-team@...udflare.com>,
        Alexei Starovoitov <ast@...nel.org>,
        "Andrii Nakryiko" <andriin@...com>
Subject: Re: [PATCH bpf-next] selftests/bpf: Restore the netns after flow dissector reattach test

On Thu, Oct 17, 2019 at 08:18 PM CEST, Martin Lau wrote:
> On Thu, Oct 17, 2019 at 10:37:52AM +0200, Jakub Sitnicki wrote:

[...]

>> ---
>>  .../bpf/prog_tests/flow_dissector_reattach.c  | 21 +++++++++++++++----
>>  1 file changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/flow_dissector_reattach.c b/tools/testing/selftests/bpf/prog_tests/flow_dissector_reattach.c
>> index 777faffc4639..1f51ba66b98b 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/flow_dissector_reattach.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/flow_dissector_reattach.c
>> @@ -91,12 +91,18 @@ static void do_flow_dissector_reattach(void)
>>
>>  void test_flow_dissector_reattach(void)
>>  {
>> -	int init_net, err;
>> +	int init_net, self_net, err;
>> +
>> +	self_net = open("/proc/self/ns/net", O_RDONLY);
>> +	if (CHECK_FAIL(self_net < 0)) {
>> +		perror("open(/proc/self/ns/net");
>> +		return;
>> +	}
>>
>>  	init_net = open("/proc/1/ns/net", O_RDONLY);
>>  	if (CHECK_FAIL(init_net < 0)) {
>>  		perror("open(/proc/1/ns/net)");
>> -		return;
>> +		goto out_close;
> Mostly nit.  close(-1) is ok-ish...  The same goes for the "out_close" in
> do_flow_dissector_reattach().

Happy to fix it up. Is your concern that calls to close invalid FDs
clutter strace output or something else?

>
> Acked-by: Martin KaFai Lau <kafai@...com>

Thanks for the review.

-Jakub