Re: Re: [PATCH] net/packet: fix race condition between fanout_add and __unregister_prot_hook

On Tue, Sep 19, 2017 at 3:21 AM, Nixiaoming [off-list ref] wrote:

po->bind_lock is held when registering and unregistering the
protocol hook. fanout_release does access po->running or
prot_hook.

It is called from packet_release, which does hold the bind_lock
when unregistering the protocol hook.

On Tue, Sep 19, 2017 at 12:09 PM, Willem de Bruijn
[off-list ref] wrote:

On Tue, Sep 19, 2017 at 3:21 AM, Nixiaoming [off-list ref] wrote:

quoted

On Fri, Sep 15, 2017 at 10:46 AM, Willem de Bruijn

[off-list ref] wrote:

quoted

quoted

In case of failure we also need to unlink and free match. I

quoted

sent the following:

quoted

quoted

http://patchwork.ozlabs.org/patch/813945/

+       spin_lock(&po->bind_lock);

+       if (po->running &&

+           match->type == type &&

           match->prot_hook.type == po->prot_hook.type &&

           match->prot_hook.dev == po->prot_hook.dev) {

                err = -ENOSPC;

@@ -1761,6 +1760,13 @@  static int fanout_add(struct sock *sk, u16 id, u16

type_flags)

                          err = 0;

                }

       }

+       spin_unlock(&po->bind_lock);

+

+       if (err && !refcount_read(&match->sk_ref)) {

+                list_del(&match->list);

+                kfree(match);

+       }





In the function fanout_add add spin_lock to protect po-> running and po->
fanout,

then whether it should be in the function fanout_release also add spin_lock
protection ?

po->bind_lock is held when registering and unregistering the
protocol hook. fanout_release does access po->running or
prot_hook.

whoops. does *not* access.