* [PATCH] gre: fix a possible skb leak
@ 2013-06-24 13:26 Eric Dumazet
2013-06-24 14:03 ` Veaceslav Falico
2013-06-25 23:08 ` [PATCH] " David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Eric Dumazet @ 2013-06-24 13:26 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Pravin B Shelar, Daniel Borkmann
From: Eric Dumazet <edumazet@google.com>
commit 68c331631143 ("v4 GRE: Add TCP segmentation offload for GRE")
added a possible skb leak, because it frees only the head of segment
list, in case a skb_linearize() call fails.
This patch adds a kfree_skb_list() helper to fix the bug.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Pravin B Shelar <pshelar@nicira.com>
Cc: Daniel Borkmann <dborkman@redhat.com>
---
include/linux/skbuff.h | 1 +
net/core/skbuff.c | 20 ++++++++++++--------
net/ipv4/gre.c | 2 +-
3 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 9c676eae..dec1748 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -627,6 +627,7 @@ static inline struct rtable *skb_rtable(const struct sk_buff *skb)
}
extern void kfree_skb(struct sk_buff *skb);
+extern void kfree_skb_list(struct sk_buff *segs);
extern void skb_tx_error(struct sk_buff *skb);
extern void consume_skb(struct sk_buff *skb);
extern void __kfree_skb(struct sk_buff *skb);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index cfd777b..1c1738c 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -483,15 +483,8 @@ EXPORT_SYMBOL(skb_add_rx_frag);
static void skb_drop_list(struct sk_buff **listp)
{
- struct sk_buff *list = *listp;
-
+ kfree_skb_list(*listp);
*listp = NULL;
-
- do {
- struct sk_buff *this = list;
- list = list->next;
- kfree_skb(this);
- } while (list);
}
static inline void skb_drop_fraglist(struct sk_buff *skb)
@@ -651,6 +644,17 @@ void kfree_skb(struct sk_buff *skb)
}
EXPORT_SYMBOL(kfree_skb);
+void kfree_skb_list(struct sk_buff *segs)
+{
+ while (segs) {
+ struct sk_buff *next = segs->next;
+
+ kfree_skb(segs);
+ segs = next;
+ }
+}
+EXPORT_SYMBOL(kfree_skb_list);
+
/**
* skb_tx_error - report an sk_buff xmit error
* @skb: buffer that triggered an error
diff --git a/net/ipv4/gre.c b/net/ipv4/gre.c
index b2e805a..7856d16 100644
--- a/net/ipv4/gre.c
+++ b/net/ipv4/gre.c
@@ -178,7 +178,7 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
err = __skb_linearize(skb);
if (err) {
- kfree_skb(segs);
+ kfree_skb_list(segs);
segs = ERR_PTR(err);
goto out;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: gre: fix a possible skb leak
2013-06-24 13:26 [PATCH] gre: fix a possible skb leak Eric Dumazet
@ 2013-06-24 14:03 ` Veaceslav Falico
2013-06-24 14:36 ` Eric Dumazet
2013-06-24 14:38 ` Daniel Borkmann
2013-06-25 23:08 ` [PATCH] " David Miller
1 sibling, 2 replies; 6+ messages in thread
From: Veaceslav Falico @ 2013-06-24 14:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Pravin B Shelar, Daniel Borkmann
On Mon, Jun 24, 2013 at 06:26:00AM -0700, Eric Dumazet wrote:
>From: Eric Dumazet <edumazet@google.com>
>
>commit 68c331631143 ("v4 GRE: Add TCP segmentation offload for GRE")
>added a possible skb leak, because it frees only the head of segment
>list, in case a skb_linearize() call fails.
>
>This patch adds a kfree_skb_list() helper to fix the bug.
>
>Signed-off-by: Eric Dumazet <edumazet@google.com>
>Cc: Pravin B Shelar <pshelar@nicira.com>
>Cc: Daniel Borkmann <dborkman@redhat.com>
>
>---
>include/linux/skbuff.h | 1 +
> net/core/skbuff.c | 20 ++++++++++++--------
> net/ipv4/gre.c | 2 +-
> 3 files changed, 14 insertions(+), 9 deletions(-)
>
>
>
>--
>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 --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>index 9c676eae..dec1748 100644
>--- a/include/linux/skbuff.h
>+++ b/include/linux/skbuff.h
>@@ -627,6 +627,7 @@ static inline struct rtable *skb_rtable(const struct sk_buff *skb)
> }
>
> extern void kfree_skb(struct sk_buff *skb);
>+extern void kfree_skb_list(struct sk_buff *segs);
> extern void skb_tx_error(struct sk_buff *skb);
> extern void consume_skb(struct sk_buff *skb);
> extern void __kfree_skb(struct sk_buff *skb);
>diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>index cfd777b..1c1738c 100644
>--- a/net/core/skbuff.c
>+++ b/net/core/skbuff.c
>@@ -483,15 +483,8 @@ EXPORT_SYMBOL(skb_add_rx_frag);
>
> static void skb_drop_list(struct sk_buff **listp)
> {
>- struct sk_buff *list = *listp;
>-
>+ kfree_skb_list(*listp);
> *listp = NULL;
>-
>- do {
>- struct sk_buff *this = list;
>- list = list->next;
>- kfree_skb(this);
>- } while (list);
> }
>
> static inline void skb_drop_fraglist(struct sk_buff *skb)
>@@ -651,6 +644,17 @@ void kfree_skb(struct sk_buff *skb)
> }
> EXPORT_SYMBOL(kfree_skb);
>
>+void kfree_skb_list(struct sk_buff *segs)
>+{
>+ while (segs) {
>+ struct sk_buff *next = segs->next;
>+
>+ kfree_skb(segs);
>+ segs = next;
>+ }
>+}
>+EXPORT_SYMBOL(kfree_skb_list);
>+
> /**
> * skb_tx_error - report an sk_buff xmit error
> * @skb: buffer that triggered an error
>diff --git a/net/ipv4/gre.c b/net/ipv4/gre.c
>index b2e805a..7856d16 100644
>--- a/net/ipv4/gre.c
>+++ b/net/ipv4/gre.c
>@@ -178,7 +178,7 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
>
> err = __skb_linearize(skb);
> if (err) {
>- kfree_skb(segs);
>+ kfree_skb_list(segs);
> segs = ERR_PTR(err);
> goto out;
> }
>
Maybe we can also use it here (build-tested only):
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 4850dc1..5776819 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2924,10 +2924,7 @@ perform_csum_check:
return segs;
err:
- while ((skb = segs)) {
- segs = skb->next;
- kfree_skb(skb);
- }
+ kfree_skb_list(segs);
return ERR_PTR(err);
}
EXPORT_SYMBOL_GPL(skb_segment);
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 4bcabf3..27ba045 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -575,11 +575,7 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
return 0;
}
- while (frag) {
- skb = frag->next;
- kfree_skb(frag);
- frag = skb;
- }
+ kfree_skb_list(frag);
IP_INC_STATS(dev_net(dev), IPSTATS_MIB_FRAGFAILS);
return err;
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: gre: fix a possible skb leak
2013-06-24 14:03 ` Veaceslav Falico
@ 2013-06-24 14:36 ` Eric Dumazet
2013-06-24 14:39 ` Veaceslav Falico
2013-06-24 14:38 ` Daniel Borkmann
1 sibling, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2013-06-24 14:36 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: David Miller, netdev, Pravin B Shelar, Daniel Borkmann
On Mon, 2013-06-24 at 16:03 +0200, Veaceslav Falico wrote:
> Maybe we can also use it here (build-tested only):
Sure, but it will be done later on net-next
This patch is a stable candidate, so I made a somewhat minimal change
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: gre: fix a possible skb leak
2013-06-24 14:36 ` Eric Dumazet
@ 2013-06-24 14:39 ` Veaceslav Falico
0 siblings, 0 replies; 6+ messages in thread
From: Veaceslav Falico @ 2013-06-24 14:39 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Pravin B Shelar, Daniel Borkmann
On Mon, Jun 24, 2013 at 07:36:00AM -0700, Eric Dumazet wrote:
>On Mon, 2013-06-24 at 16:03 +0200, Veaceslav Falico wrote:
>
>> Maybe we can also use it here (build-tested only):
>
>Sure, but it will be done later on net-next
>
>This patch is a stable candidate, so I made a somewhat minimal change
Makes sense, didn't realize that it's for net, not net-next.
Thank you!
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: gre: fix a possible skb leak
2013-06-24 14:03 ` Veaceslav Falico
2013-06-24 14:36 ` Eric Dumazet
@ 2013-06-24 14:38 ` Daniel Borkmann
1 sibling, 0 replies; 6+ messages in thread
From: Daniel Borkmann @ 2013-06-24 14:38 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: Eric Dumazet, David Miller, netdev, Pravin B Shelar
On 06/24/2013 04:03 PM, Veaceslav Falico wrote:
> On Mon, Jun 24, 2013 at 06:26:00AM -0700, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet@google.com>
...
> Maybe we can also use it here (build-tested only):
Yep, this clean-up looks good I think. But then -net needs to be merged into
-net-next first (also for the gre_offload patch).
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 4850dc1..5776819 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2924,10 +2924,7 @@ perform_csum_check:
> return segs;
>
> err:
> - while ((skb = segs)) {
> - segs = skb->next;
> - kfree_skb(skb);
> - }
> + kfree_skb_list(segs);
> return ERR_PTR(err);
> }
> EXPORT_SYMBOL_GPL(skb_segment);
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index 4bcabf3..27ba045 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -575,11 +575,7 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
> return 0;
> }
>
> - while (frag) {
> - skb = frag->next;
> - kfree_skb(frag);
> - frag = skb;
> - }
> + kfree_skb_list(frag);
> IP_INC_STATS(dev_net(dev), IPSTATS_MIB_FRAGFAILS);
> return err;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gre: fix a possible skb leak
2013-06-24 13:26 [PATCH] gre: fix a possible skb leak Eric Dumazet
2013-06-24 14:03 ` Veaceslav Falico
@ 2013-06-25 23:08 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2013-06-25 23:08 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, pshelar, dborkman
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 24 Jun 2013 06:26:00 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> commit 68c331631143 ("v4 GRE: Add TCP segmentation offload for GRE")
> added a possible skb leak, because it frees only the head of segment
> list, in case a skb_linearize() call fails.
>
> This patch adds a kfree_skb_list() helper to fix the bug.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied and queued up for -stable, thanks Eric.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-25 23:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-24 13:26 [PATCH] gre: fix a possible skb leak Eric Dumazet
2013-06-24 14:03 ` Veaceslav Falico
2013-06-24 14:36 ` Eric Dumazet
2013-06-24 14:39 ` Veaceslav Falico
2013-06-24 14:38 ` Daniel Borkmann
2013-06-25 23:08 ` [PATCH] " David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).