All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Svenning Sørensen" <sss@secomea.com>
To: "David S. Miller" <davem@davemloft.net>
Cc: <linux-kernel@vger.kernel.org>, <netdev@vger.kernel.org>
Subject: [PATCH] net: guard against coalescing packets from buggy network drivers
Date: Fri, 25 Apr 2014 17:02:05 +0200	[thread overview]
Message-ID: <535A78ED.3020309@secomea.com> (raw)

This patch adds a guard against coalescing packets received by buggy network
drivers that don't initialize skb->tail properly.

Coalescing such packets will corrupt data in a way that makes it hard to
track down the real cause of the problem. Observed symptoms are a flood of
WARNs by tcp_recvmsg at random times, leaving no clear indication as to why
the packet buffers were corrupted. See for example this thread:
https://groups.google.com/forum/#!topic/linux.kernel/A8ZI9aXooSU

Obviously the correct thing to do is fixing the drivers in question (eg. by
using skb_put() instead of just setting skb->len) - this patch will 
hopefully
help to track them down with less effort than I had to put into it :)

NB: in my case it was an out-of-tree Bluegiga WiFi driver; a quick look 
indicates
that there are in-tree drivers (for hardware that I don't own) with the 
same bug.


Signed-off-by: Svenning Soerensen <sss@secomea.com>

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 1b62343..188a60a 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3833,6 +3833,19 @@ bool skb_try_coalesce(struct sk_buff *to, struct 
sk_buff *from,
          return false;

      if (len <= skb_tailroom(to)) {
+        /* Guard against buggy network drivers that don't initialize
+         * skb properly: if they manipulate skb->len directly without
+         * setting skb->tail, the skb_put below may return a pointer to
+         * live data (such as TCP headers) that (without this guard)
+         * would get overwritten by coalescing.
+         * The correct thing to do is fixing the network drivers - but
+         * this guard should give us a chance to spot them before they
+         * cause real hard-to-debug problems :)
+         */
+        if (WARN_ONCE(skb_tail_pointer(to) < to->data,
+                  "skb inconsistency: tail below data\n"))
+            return false;
+
          BUG_ON(skb_copy_bits(from, 0, skb_put(to, len), len));
          *delta_truesize = 0;
          return true;


             reply	other threads:[~2014-04-25 15:07 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-25 15:02 Svenning Sørensen [this message]
2014-04-25 15:43 ` [PATCH] net: guard against coalescing packets from buggy network drivers Eric Dumazet
2014-04-25 16:01   ` Svenning Sørensen
2014-04-25 16:49     ` Eric Dumazet
2014-04-25 19:32       ` Svenning Sørensen

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=535A78ED.3020309@secomea.com \
    --to=sss@secomea.com \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --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.