diff mbox

tcp: skb_shift cannot cache frag ptrs past pskb_expand_head

Message ID Pine.LNX.4.64.0811252202550.14851@wrl-59.cs.helsinki.fi
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Ilpo Järvinen Nov. 25, 2008, 8:08 p.m. UTC
Since pskb_expand_head creates copy of the shared area we
cannot keep any frag ptr past de-cloning. This fixes the
tcpdump recvfrom -EFAULT problem.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/core/skbuff.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

Comments

David Miller Nov. 25, 2008, 9:57 p.m. UTC | #1
From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Tue, 25 Nov 2008 22:08:06 +0200 (EET)

> Since pskb_expand_head creates copy of the shared area we
> cannot keep any frag ptr past de-cloning. This fixes the
> tcpdump recvfrom -EFAULT problem.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>

Good catch.

I think everyone mucking around with these sorts of things
has made this error at least once or twice :)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 57555a4..133ecd6 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2018,7 +2018,10 @@  void skb_split(struct sk_buff *skb, struct sk_buff *skb1, const u32 len)
 		skb_split_no_header(skb, skb1, len, pos);
 }
 
-/* Shifting from/to a cloned skb is a no-go. */
+/* Shifting from/to a cloned skb is a no-go.
+ *
+ * Caller cannot keep skb_shinfo related pointers past calling here!
+ */
 static int skb_prepare_for_shift(struct sk_buff *skb)
 {
 	return skb_cloned(skb) && pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
@@ -2070,6 +2073,8 @@  int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
 			    skb_prepare_for_shift(tgt))
 				return 0;
 
+			/* All previous frag pointers might be stale! */
+			fragfrom = &skb_shinfo(skb)->frags[from];
 			fragto = &skb_shinfo(tgt)->frags[merge];
 
 			fragto->size += shiftlen;