Message ID | CAEfhGiy=-AgMad_tquyXws7Vf3j9HWwhCnaZqWeq58_XSzcgoA@mail.gmail.com |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
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
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,