All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <eric.dumazet@gmail.com>
To: Marc Dionne <marc.c.dionne@gmail.com>
Cc: netdev@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Craig Gallek <kraig@google.com>,
	edumazet@google.com
Subject: Re: Crash with SO_REUSEPORT and ef456144da8ef507c8cf504284b6042e9201a05c
Date: Tue, 19 Jan 2016 08:14:52 -0800	[thread overview]
Message-ID: <1453220092.1223.257.camel@edumazet-glaptop2.roam.corp.google.com> (raw)
In-Reply-To: <CAB9dFdugvFvdQLGAU83CQHYbwaWCMHrAm5Xk746iwMG5g94PqQ@mail.gmail.com>

On Tue, 2016-01-19 at 11:57 -0400, Marc Dionne wrote:
> I shared this one with Craig but I thought I'd put it out to a wider audience.
> 
> Trying to run the current kernel mainline on a test system I found
> that any attempt to run many of our executables would crash the
> system.  The networking code in all of these opens and listens on
> multiple UDP sockets set with SO_REUSEPORT.  We also like to bind the
> first socket before setting SO_REUSEPORT so we can catch some cases
> where the port is actually in use by someone else (for instance a
> previous incarnation of the same service).
> 
> This is easily reproduced with this sequence:
> - create 2 sockets A and B
> - bind socket A to an address
> - set SO_REUSEPORT on socket A
> - set SO_REUSEPORT on socket B
> - bind socket B to the same address as A
> 
> The sk_reuseport_cb structure is only allocated at bind time if
> SO_REUSEPORT is already set, so A doesn't have one.  When we bind B, A
> is found as a match that has SO_REUSEPORT and reuseport_add_sock will
> try to use the NULL sk_reuseport_cb structure from A, causing a crash.
> 
> Not sure what the best fix is, but seems like the structure could be
> either allocated (if not already done) when setting SO_REUSEPORT, or
> when we find it to be NULL in reuseport_add_sock (but locking may be
> an issue there).  I was able to test that allocating sk_reuseport_cb
> when setting SO_REUSEPORT makes things behave normally again; see
> attached patch.  That's surely not a correct/complete fix as B (in the
> scenario above) will have an unnecessary sk_reuseport_cb which will
> trigger a warning and should be dealt with.

Hi Marc

Your patch looks fine to me, please add a "Fixes:" tag in it ?

Fixes: e32ea7e74727 ("soreuseport: fast reuseport UDP socket selection")

Thanks.

  parent reply	other threads:[~2016-01-19 16:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-19 15:57 Crash with SO_REUSEPORT and ef456144da8ef507c8cf504284b6042e9201a05c Marc Dionne
2016-01-19 16:04 ` Marc Dionne
2016-01-19 16:31   ` Craig Gallek
2016-01-19 17:08     ` Marc Dionne
2016-01-19 18:11       ` Craig Gallek
2016-01-19 18:51         ` Marc Dionne
2016-01-19 16:14 ` Eric Dumazet [this message]
2016-01-19 19:08 Craig Gallek
2016-01-19 19:32 ` Marc Dionne

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1453220092.1223.257.camel@edumazet-glaptop2.roam.corp.google.com \
    --to=eric.dumazet@gmail.com \
    --cc=edumazet@google.com \
    --cc=kraig@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.c.dionne@gmail.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.