netdev - Re: [RFC PATCH bpf-next 2/6] bpf: add bpf_get

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 9 Apr 2018 09:52:47 -0700
From:   Yonghong Song <yhs@...com>
To:     Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...com>, <netdev@...r.kernel.org>
CC:     <kernel-team@...com>
Subject: Re: [RFC PATCH bpf-next 2/6] bpf: add bpf_get_stack helper



On 4/9/18 3:01 AM, Daniel Borkmann wrote:
> On 04/09/2018 07:02 AM, Alexei Starovoitov wrote:
>> On 4/8/18 9:53 PM, Yonghong Song wrote:
>>>>> @@ -1004,7 +1007,8 @@ static void __bpf_prog_put(struct bpf_prog
>>>>> *prog, bool do_idr_lock)
>>>>>               bpf_prog_kallsyms_del(prog->aux->func[i]);
>>>>>           bpf_prog_kallsyms_del(prog);
>>>>>
>>>>> -        call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu);
>>>>> +        synchronize_rcu();
>>>>> +        __bpf_prog_put_rcu(&prog->aux->rcu);
>>>>
>>>> there should have been lockdep splat.
>>>> We cannot call synchronize_rcu here, since we cannot sleep
>>>> in some cases.
>>>
>>> Let me double check this. The following is the reason
>>> why I am using synchronize_rcu().
>>>
>>> With call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu) and
>>> _bpf_prog_put_rcu calls put_callchain_buffers() which
>>> calls mutex_lock(), the runtime with CONFIG_DEBUG_ATOMIC_SLEEP=y
>>> will complains since potential sleep inside the call_rcu is not
>>> allowed.
>>
>> I see. Indeed. We cannot call put_callchain_buffers() from rcu callback,
>> but doing synchronize_rcu() here is also not possible.
>> How about moving put_callchain into bpf_prog_free_deferred() ?
> 
> +1, the assumption is that you can call bpf_prog_put() and also the
> bpf_map_put() from any context. Sleeping here for a long time might
> subtly break things badly.

Thanks for the suggestion! This should work.