lkml.org 
[lkml]   [2018]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 4.4 159/190] ipsec: check return value of skb_to_sgvec always
From
Date
On Thu, 2018-05-17 at 11:10 +0200, Greg Kroah-Hartman wrote:
> On Wed, May 16, 2018 at 02:58:02PM +0100, Ben Hutchings wrote:
> > On Wed, 2018-04-11 at 20:36 +0200, Greg Kroah-Hartman wrote:
> > > 4.4-stable review patch.  If anyone has any objections, please let me know.
> > >
> > > ------------------
> > >
> > > From: Jason A. Donenfeld <Jason@zx2c4.com>
> > >
> > > commit 3f29770723fe498a5c5f57c3a31a996ebdde03e1 upstream.
> >
> > [...]
> >
> > This leaves the error paths in esp{4,6}.c leaking memory; fixed
> > upstream by:
> >
> > commit e6194923237f3952b955c343b65b211f36bce01c
> > Author: Steffen Klassert <steffen.klassert@secunet.com>
> > Date:   Thu Jul 13 09:13:30 2017 +0200
> >
> >     esp: Fix memleaks on error paths.
>
> Really?  That patch doesn't apply at all.  It looks like this patch
> fixes things that happened after the above patch.
>
> Or am I confused?

It's partly fixing bugs introduced in 4.12, but it's also fixing
similar bugs introduced by the commit that has been backported to
stable branches.

I've attached a backport to 4.4 that will also work for 4.9 (it has
identical versions of esp4 and esp6). I would appreciate a review from
those actually familiar with the code. Also, it looks like
esp6_input() still has a potential leak both upstream and in stable
branches.

Ben.

--
Ben Hutchings
Software Developer, Codethink Ltd.
From e6194923237f3952b955c343b65b211f36bce01c Mon Sep 17 00:00:00 2001
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Thu, 13 Jul 2017 09:13:30 +0200
Subject: [PATCH] esp: Fix memleaks on error paths.

We leak the temporary allocated resources in error paths,
fix this by freeing them.

Fixes: fca11ebde3f ("esp4: Reorganize esp_output")
Fixes: 383d0350f2c ("esp6: Reorganize esp_output")
Fixes: 3f29770723f ("ipsec: check return value of skb_to_sgvec always")
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/esp4.c | 13 ++++++++-----
net/ipv6/esp6.c | 9 +++++----
2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index 0cbee0a666ff..dbb31a942dfa 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -381,7 +381,7 @@ int esp_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
(unsigned char *)esph - skb->data,
assoclen + ivlen + esp->clen + alen);
if (unlikely(err < 0))
- goto error;
+ goto error_free;

if (!esp->inplace) {
int allocsize;
@@ -392,7 +392,7 @@ int esp_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
spin_lock_bh(&x->lock);
if (unlikely(!skb_page_frag_refill(allocsize, pfrag, GFP_ATOMIC))) {
spin_unlock_bh(&x->lock);
- goto error;
+ goto error_free;
}

skb_shinfo(skb)->nr_frags = 1;
@@ -409,7 +409,7 @@ int esp_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
(unsigned char *)esph - skb->data,
assoclen + ivlen + esp->clen + alen);
if (unlikely(err < 0))
- goto error;
+ goto error_free;
}

if ((x->props.flags & XFRM_STATE_ESN))
@@ -442,8 +442,9 @@ int esp_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *

if (sg != dsg)
esp_ssg_unref(x, tmp);
- kfree(tmp);

+error_free:
+ kfree(tmp);
error:
return err;
}
@@ -695,8 +696,10 @@ skip_cow:

sg_init_table(sg, nfrags);
err = skb_to_sgvec(skb, sg, 0, skb->len);
- if (unlikely(err < 0))
+ if (unlikely(err < 0)) {
+ kfree(tmp);
goto out;
+ }

skb->ip_summed = CHECKSUM_NONE;

diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 9ed35473dcb5..392def1fcf21 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -345,7 +345,7 @@ int esp6_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info
(unsigned char *)esph - skb->data,
assoclen + ivlen + esp->clen + alen);
if (unlikely(err < 0))
- goto error;
+ goto error_free;

if (!esp->inplace) {
int allocsize;
@@ -356,7 +356,7 @@ int esp6_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info
spin_lock_bh(&x->lock);
if (unlikely(!skb_page_frag_refill(allocsize, pfrag, GFP_ATOMIC))) {
spin_unlock_bh(&x->lock);
- goto error;
+ goto error_free;
}

skb_shinfo(skb)->nr_frags = 1;
@@ -373,7 +373,7 @@ int esp6_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info
(unsigned char *)esph - skb->data,
assoclen + ivlen + esp->clen + alen);
if (unlikely(err < 0))
- goto error;
+ goto error_free;
}

if ((x->props.flags & XFRM_STATE_ESN))
@@ -406,8 +406,9 @@ int esp6_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info

if (sg != dsg)
esp_ssg_unref(x, tmp);
- kfree(tmp);

+error_free:
+ kfree(tmp);
error:
return err;
}
--
2.11.0
\
 
 \ /
  Last update: 2018-05-17 23:08    [W:0.185 / U:11.392 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site