All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andi Kleen <andi@firstfloor.org>
To: David Miller <davem@davemloft.net>
Cc: andi@firstfloor.org, johannes@sipsolutions.net,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linville@tuxdriver.com
Subject: Re: [PATCH] Fix up truesize after pskb_expand_head() in wireless stack
Date: Mon, 5 Jan 2009 14:32:28 +0100	[thread overview]
Message-ID: <20090105133228.GP496@one.firstfloor.org> (raw)
In-Reply-To: <20090104.224916.253586382.davem@davemloft.net>

> At a minimum you need to add a skb->sk == NULL warn-on and abort path
> here, otherwise we will corrupt socket accounting and just explode
> somewhere else.  Adding this patch as-is will just introduce a new

I ran the patch for a few days now and nothing exploded, no messages.
I can add a skb->sk check and see if it triggers.

> There are cases where the Tx path of the wireless loops back packets
> back to the Rx path, and in such cases we certainly could see sockets
> attached to the SKB.
> 
> And Johannes is right,

He keeps talking about cases like monitoring sockets that don't
apply, which makes me somewhat suspicious of his analysis.

> absolutely cannot modify ->truesize blindly.  You can't change the
> truesize value if a socket is attached.

But pskb_expand_head modifies the size so obviously 
truesize needs to change too. So you're saying pskb_expand_head() 
is illegal when there might be a socket attached? Somehow
I suspect a lot of the pskb_expand_head() callers are failing
that requirement.

Ok I guess we could call skb_orphan() unconditionally.

Or if you guys have a better patch I can test.

-Andi

-- 
ak@linux.intel.com

  reply	other threads:[~2009-01-05 13:18 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-04 15:18 [PATCH] Fix up truesize after pskb_expand_head() in wireless stack Andi Kleen
2009-01-04 15:18 ` Andi Kleen
2009-01-04 16:05 ` Johannes Berg
2009-01-04 16:05   ` Johannes Berg
2009-01-04 16:28   ` Andi Kleen
2009-01-04 16:28     ` Andi Kleen
2009-01-04 16:41     ` Johannes Berg
2009-01-04 16:41       ` Johannes Berg
2009-01-04 17:43       ` Andi Kleen
2009-01-04 17:43         ` Andi Kleen
2009-01-04 17:33         ` Johannes Berg
2009-01-04 17:33           ` Johannes Berg
2009-01-04 18:41           ` Andi Kleen
2009-01-05  6:49             ` David Miller
2009-01-05 13:32               ` Andi Kleen [this message]
2009-01-05  8:36             ` Johannes Berg
2009-01-05 13:21               ` Andi Kleen
2009-01-05 13:16                 ` Johannes Berg
2009-01-05 13:16                   ` Johannes Berg
2009-01-05 13:36                   ` Andi Kleen
2009-01-05 13:31                     ` Johannes Berg
2009-01-05 14:05                       ` Andi Kleen

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=20090105133228.GP496@one.firstfloor.org \
    --to=andi@firstfloor.org \
    --cc=davem@davemloft.net \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.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.