Messages in this thread | | | Date | Tue, 19 Jan 2016 14:51:53 -0400 | Subject | Re: Crash with SO_REUSEPORT and ef456144da8ef507c8cf504284b6042e9201a05c | From | Marc Dionne <> |
| |
On Tue, Jan 19, 2016 at 2:11 PM, Craig Gallek <kraig@google.com> wrote: > On Tue, Jan 19, 2016 at 12:08 PM, Marc Dionne <marc.c.dionne@gmail.com> wrote: >> On Tue, Jan 19, 2016 at 12:31 PM, Craig Gallek <kraig@google.com> wrote: >>> >>> I need to think about how to handle setsockopt-after-bind condition a >>> bit more, but the NULL pointer dereference is obviously wrong. Do you >>> have a way to easily reproduce this? I've only managed to get it to >>> happen once so far... >> >> The attached code reliably triggers the crash for me. > > I think the patch below will address this issue (sorry in advance if > gmail screws up the whitespace...). I'll send it for formal review > once I finish testing it. > > Craig > > diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c > index 1df98c557440..004cb2c974ac 100644 > --- a/net/core/sock_reuseport.c > +++ b/net/core/sock_reuseport.c > @@ -97,6 +97,11 @@ int reuseport_add_sock(struct sock *sk, const > struct sock *sk2) > { > struct sock_reuseport *reuse; > > + if (!rcu_access_pointer(sk2->sk_reuseport_cb)) { > + int err = reuseport_alloc(sk2); > + if (err) return err; > + } > + > spin_lock_bh(&reuseport_lock); > reuse = rcu_dereference_protected(sk2->sk_reuseport_cb, > lockdep_is_held(&reuseport_lock)),
That works fine, thanks..
Just wondering though, is there a bit of a race there? Seems like it might be safer to have a version of reuseport_alloc that doesn't take the lock and use it here, moving the block after the lock is taken.
Marc
| |