All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Cong Wang <cwang@twopensource.com>,
	netdev <netdev@vger.kernel.org>, Jiri Pirko <jiri@resnulli.us>
Subject: Re: [PATCH net-next] ipv6: protect skb->sk accesses from recursive dereference inside the stack
Date: Thu, 02 Apr 2015 13:34:08 +0200	[thread overview]
Message-ID: <1427974448.2093675.248500389.6D8AFBB6@webmail.messagingengine.com> (raw)
In-Reply-To: <1427939630.25985.186.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Apr 2, 2015, at 03:53, Eric Dumazet wrote:
> On Thu, 2015-04-02 at 02:56 +0200, Hannes Frederic Sowa wrote:
> > 
> > On Thu, Apr 2, 2015, at 02:48, Eric Dumazet wrote:
> > > On Thu, 2015-04-02 at 02:35 +0200, Hannes Frederic Sowa wrote:
> > > > On Thu, Apr 2, 2015, at 02:27, Eric Dumazet wrote: 
> > > > > orphaning skb just because they traverse a tunnel would be quite
> > > > > horrible.
> > > > 
> > > > Agreed, but we have some bits in the skb->sk pointer left for signaling
> > > > we are only keeping it around for destructor and upper layer
> > > > notifications. Destructors should be the only ones having to deal with
> > > > skb->sk and they can mask the bit. That would touch a lot of NULL
> > > > checks, though.
> > > 
> > > Have you checked net/sched/sch_fq.c per chance ?
> > > 
> > > skb->sk is not an opaque value.
> > 
> > Do you think that skb->sk access through skb->__sk & ~0x1ULL would slow
> > down the code too much?
> 
> I was only replying to your claim "Destructors should be the only ones
> having to deal with skb->sk", which looked wrong to me.

You are correct, there might be even more places I need to use this. 

> Adding bit masking would slow down the code, but we did this already for
> skb->dst

Exactly.

My plan is that I actually would propose this patch for -net and stable
as it fixes the problem that currently we oops when running dhclient on
a vxlan interface. As soon as net gets merged into net-next I I would
propose this change if it works out smoothly and looks maintainable.

Thanks,
Hannes

  reply	other threads:[~2015-04-02 11:34 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-01 15:07 [PATCH net-next] ipv6: protect skb->sk accesses from recursive dereference inside the stack Hannes Frederic Sowa
2015-04-01 18:40 ` David Miller
2015-04-01 18:57   ` Hannes Frederic Sowa
2015-04-01 19:26     ` David Miller
2015-04-02  0:06 ` Cong Wang
2015-04-02  0:27   ` Eric Dumazet
2015-04-02  0:35     ` Hannes Frederic Sowa
2015-04-02  0:48       ` Eric Dumazet
2015-04-02  0:56         ` Hannes Frederic Sowa
2015-04-02  1:53           ` Eric Dumazet
2015-04-02 11:34             ` Hannes Frederic Sowa [this message]
2015-04-02 16:24               ` David Miller
2015-04-02  2:55     ` Cong Wang
2015-04-02 12:05       ` Hannes Frederic Sowa
2015-04-06 20:14 ` David Miller

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=1427974448.2093675.248500389.6D8AFBB6@webmail.messagingengine.com \
    --to=hannes@stressinduktion.org \
    --cc=cwang@twopensource.com \
    --cc=eric.dumazet@gmail.com \
    --cc=jiri@resnulli.us \
    --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.