Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1367175

Re: bpf: net/core/filter.c:2115 suspicious rcu_dereference_protected() usage!

Show all headers | View raw

On 03/30/2016 02:24 PM, Michal Kubecek wrote:
> On Wed, Mar 30, 2016 at 01:33:44PM +0200, Daniel Borkmann wrote:
>> On 03/30/2016 11:42 AM, Michal Kubecek wrote:
>>>
>>> I'm just not sure checking if we hold the right lock depending on caller
>>> is worth the extra complexity. After all, what is really needed is to
>>> hold _some_ lock guaranteeing sk_attach_prog() and sk_detach_filter()
>>> are safe so that just changing the condition in both to
>>>
>>>    sock_owned_by_user(sk) || lockdep_rtnl_is_held()
>>
>> It would certainly silence it, but would be less accurate in terms of lock
>> proving as opposed to the diff above. E.g. rntl could be held elsewhere,
>> while someone attaches a socket filter w/o having locked the socket (currently
>> not the case, but it would kind of defeat the purpose of rcu_dereference_protected()
>> here). Was thinking about using a extra socket flag to indicate it's
>> externally managed, but it's not really worth wasting sk's flags bit
>> space just for this corner case.
>
> Originally my reasoning was that to actually hide a locking issue from
> lockdep, this would have to happen every time we get down into the
> function which is unlikely. But thinking about it again, this code path
> is not so frequent and the fuzzers tend to do strange things so that it
> could really happen.

In this case actually nothing too fancy, just seems that filters on tap devices
might not be really used by anyone (issue is already couple of years old).

> Sasha/Jiri, could you test the patch with your testcases? I received it
> corrupted (strange leading whitespaces) so I better add cleaned up
> version below.

Tested this yesterday night on my machine with PROVE_RCU + PROVE_RCU_REPEATEDLY
enabled, and it can easily be triggered with a simple ioctl(tun_fd,
TUN{ATTACH,DETACH}FILTER, ...) on a tap device, and the patch now silences
it. Sorry for the white space damage (should have just attached it), I'd send
it later today.

Thanks,
Daniel

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: bpf: net/core/filter.c:2115 suspicious  rcu_dereference_protected() usage! Michal Kubecek <mkubecek@suse.cz> - 2016-03-29 15:00 +0200
  Re: bpf: net/core/filter.c:2115 suspicious rcu_dereference_protected()  usage! Daniel Borkmann <daniel@iogearbox.net> - 2016-03-29 16:00 +0200
    Re: bpf: net/core/filter.c:2115 suspicious rcu_dereference_protected()  usage! Daniel Borkmann <daniel@iogearbox.net> - 2016-03-29 16:40 +0200
      Re: bpf: net/core/filter.c:2115 suspicious  rcu_dereference_protected() usage! Michal Kubecek <mkubecek@suse.cz> - 2016-03-30 11:50 +0200
        Re: bpf: net/core/filter.c:2115 suspicious rcu_dereference_protected()  usage! Daniel Borkmann <daniel@iogearbox.net> - 2016-03-30 13:40 +0200
          Re: bpf: net/core/filter.c:2115 suspicious  rcu_dereference_protected() usage! Michal Kubecek <mkubecek@suse.cz> - 2016-03-30 14:30 +0200
            Re: bpf: net/core/filter.c:2115 suspicious rcu_dereference_protected()  usage! Daniel Borkmann <borkmann@iogearbox.net> - 2016-03-30 14:40 +0200

csiph-web