netdev - Re: [PATCH bpf] bpf: test_maps: cleanup sockmaps when test ends

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]

Date:   Tue, 23 Jan 2018 19:16:35 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     John Fastabend <john.fastabend@...il.com>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Prashant Bhole <bhole_prashant_q7@....ntt.co.jp>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Shuah Khan <shuahkh@....samsung.com>, netdev@...r.kernel.org
Subject: Re: [PATCH bpf] bpf: test_maps: cleanup sockmaps when test ends

On 01/23/2018 06:18 AM, John Fastabend wrote:
> On 01/22/2018 09:01 PM, Alexei Starovoitov wrote:
>> On Tue, Jan 23, 2018 at 01:55:30PM +0900, Prashant Bhole wrote:
>>> On 1/23/2018 1:38 PM, Alexei Starovoitov wrote:
>>>> On Tue, Jan 23, 2018 at 01:30:44PM +0900, Prashant Bhole wrote:
>>>>> Bug: BPF programs and maps related to sockmaps test exist in
>>>>> memory even after test_maps ends
>>>>>
>>>>> This patch fixes it by empyting sockmaps when test ends.
>>>>>
>>>>> Fixes: 6f6d33f3b3d0f ("bpf: selftests add sockmap tests")
>>>>> Signed-off-by: Prashant Bhole <bhole_prashant_q7@....ntt.co.jp>
>>>>
>>>> that's a fine workaround and I'm planning to apply this patch
>>>> to bpf-next, but it's not a fix. The sockmap should have cleaned
>>>> itself up.

Agree, this definitely needs kernel side fixing.

>>> Ok. Do I need to re-submit it targeted to -bpf-next and without fixes tag?
>>
>> No need. It's fine.
> 
> Also I'm looking over sockmap code now for the bug, should have
> something shortly.
> 
> Agree this is a nice cleanup of the test code though. On the other
> hand I should add some explicit tests for this case (deleting map
> with elements) as well though.

Ok, thanks for looking into fixing it! The latter could be added along
with test case for the fix.

I've tested and applied Prashant's patch to bpf-next with a note that
real fix is still TBD. Thanks for catching Prashant!