netdev - Re: [RFC PATCH 3/5] bpf: add BPF_PROG_ATTACH and BPF_PROG

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: <90bdc6be-bd8e-4c2c-68e9-ca8809ea38a4@zonque.org>
Date:   Thu, 18 Aug 2016 17:17:47 +0200
From:   Daniel Mack <daniel@...que.org>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Eric Dumazet <eric.dumazet@...il.com>
Cc:     htejun@...com, daniel@...earbox.net, ast@...com,
        davem@...emloft.net, kafai@...com, fw@...len.de,
        pablo@...filter.org, harald@...hat.com, netdev@...r.kernel.org
Subject: Re: [RFC PATCH 3/5] bpf: add BPF_PROG_ATTACH and BPF_PROG_DETACH
 commands

On 08/17/2016 08:10 PM, Alexei Starovoitov wrote:
> On Wed, Aug 17, 2016 at 09:16:02AM -0700, Eric Dumazet wrote:
>> On Wed, 2016-08-17 at 16:00 +0200, Daniel Mack wrote:
>>
>>> +		progp = is_ingress ? &cgrp->bpf_ingress : &cgrp->bpf_egress;
>>> +
>>> +		rcu_read_lock();
>>> +		old_prog = rcu_dereference(*progp);
>>> +		rcu_assign_pointer(*progp, prog);
>>> +
>>> +		if (old_prog)
>>> +			bpf_prog_put(old_prog);
>>> +
>>> +		rcu_read_unlock();
>>
>>
>> This is a bogus locking strategy.
> 
> yep. this rcu_lock/unlock won't solve the race between parallel
> bpf_prog_attach calls.
> please use xchg() similar to bpf_fd_array_map_update_elem()
> Then prog pointer will be swapped automically and bpf_prog_put()
> will free it via call_rcu.
> The reader side in sk_filter_cgroup_bpf() looks correct.

Thanks! I reworked all the bits I got comments on, and fixed some other
details as well. I'll wait some days to see what else shakes out of this
thread, and then post again.

FWIW, the current code can be found here:

  https://github.com/zonque/linux/commits/cg-bpf-syscall


Thanks,
Daniel