From: Pan Bian <[email protected]>
The memory chunk allocated by xfrm_state_alloc() should be released with
xfrm_state_put(), not kfree.
Signed-off-by: Pan Bian <[email protected]>
---
net/xfrm/xfrm_user.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index c9a84e2..267922c 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2288,13 +2288,13 @@ static int xfrm_add_acquire(struct sk_buff *skb, struct nlmsghdr *nlh,
}
- kfree(x);
+ xfrm_state_put(x);
kfree(xp);
return 0;
free_state:
- kfree(x);
+ xfrm_state_put(x);
nomem:
return err;
}
--
2.7.4
On Wed, Nov 21, 2018 at 02:57:48PM +0800, Pan Bian wrote:
> From: Pan Bian <[email protected]>
>
> The memory chunk allocated by xfrm_state_alloc() should be released with
> xfrm_state_put(), not kfree.
>
> Signed-off-by: Pan Bian <[email protected]>
This bug was introduced by
commit 565f0fa902b64020d5d147ff1708567e9e0b6e49
Author: Mathias Krause <[email protected]>
Date: Thu May 3 10:55:07 2018 +0200
While using xfrm_state_put may work it's certainly not the designed
to do this. We should instead export a function that calls
kmem_cache_free on xfrm_state directly and use that here.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Wed, Nov 21, 2018 at 04:00:45PM +0800, Herbert Xu wrote:
> On Wed, Nov 21, 2018 at 02:57:48PM +0800, Pan Bian wrote:
> > From: Pan Bian <[email protected]>
> >
> > The memory chunk allocated by xfrm_state_alloc() should be released with
> > xfrm_state_put(), not kfree.
> >
> > Signed-off-by: Pan Bian <[email protected]>
>
> This bug was introduced by
>
> commit 565f0fa902b64020d5d147ff1708567e9e0b6e49
> Author: Mathias Krause <[email protected]>
> Date: Thu May 3 10:55:07 2018 +0200
>
Oh, snap. You're totally right. I missed the kfree() in xfrm_user.c.
Sorry for that!
> While using xfrm_state_put may work it's certainly not the designed
> to do this. We should instead export a function that calls
> kmem_cache_free on xfrm_state directly and use that here.
Maybe something like the below patch? Steffen?
-- >8 --
Subject: [PATCH] xfrm_user: fix freeing of xfrm states on acquire
Commit 565f0fa902b6 ("xfrm: use a dedicated slab cache for struct
xfrm_state") moved xfrm state objects to use their own slab cache.
However, it missed to adapt xfrm_user to use this new cache when
freeing xfrm states.
Fix this by introducing and make use of a new helper for freeing
xfrm_state objects.
Fixes: 565f0fa902b6 ("xfrm: use a dedicated slab cache for struct xfrm_state")
Reported-by: Pan Bian <[email protected]>
Cc: <[email protected]> # v4.18+
Signed-off-by: Mathias Krause <[email protected]>
---
include/net/xfrm.h | 1 +
net/xfrm/xfrm_state.c | 8 +++++++-
net/xfrm/xfrm_user.c | 4 ++--
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 0eb390c205af..da588def3c61 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1552,6 +1552,7 @@ int xfrm_state_walk(struct net *net, struct xfrm_state_walk *walk,
int (*func)(struct xfrm_state *, int, void*), void *);
void xfrm_state_walk_done(struct xfrm_state_walk *walk, struct net *net);
struct xfrm_state *xfrm_state_alloc(struct net *net);
+void xfrm_state_free(struct xfrm_state *x);
struct xfrm_state *xfrm_state_find(const xfrm_address_t *daddr,
const xfrm_address_t *saddr,
const struct flowi *fl,
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index dc4a9f1fb941..0a0b01b688d7 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -426,6 +426,12 @@ static void xfrm_put_mode(struct xfrm_mode *mode)
module_put(mode->owner);
}
+void xfrm_state_free(struct xfrm_state *x)
+{
+ kmem_cache_free(xfrm_state_cache, x);
+}
+EXPORT_SYMBOL(xfrm_state_free);
+
static void xfrm_state_gc_destroy(struct xfrm_state *x)
{
tasklet_hrtimer_cancel(&x->mtimer);
@@ -452,7 +458,7 @@ static void xfrm_state_gc_destroy(struct xfrm_state *x)
}
xfrm_dev_state_free(x);
security_xfrm_state_free(x);
- kmem_cache_free(xfrm_state_cache, x);
+ xfrm_state_free(x);
}
static void xfrm_state_gc_task(struct work_struct *work)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index c9a84e22f5d5..277c1c46fe94 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2288,13 +2288,13 @@ static int xfrm_add_acquire(struct sk_buff *skb, struct nlmsghdr *nlh,
}
- kfree(x);
+ xfrm_state_free(x);
kfree(xp);
return 0;
free_state:
- kfree(x);
+ xfrm_state_free(x);
nomem:
return err;
}
On Wed, Nov 21, 2018 at 09:09:23PM +0100, Mathias Krause wrote:
>
> -- >8 --
>
> Subject: [PATCH] xfrm_user: fix freeing of xfrm states on acquire
>
> Commit 565f0fa902b6 ("xfrm: use a dedicated slab cache for struct
> xfrm_state") moved xfrm state objects to use their own slab cache.
> However, it missed to adapt xfrm_user to use this new cache when
> freeing xfrm states.
>
> Fix this by introducing and make use of a new helper for freeing
> xfrm_state objects.
>
> Fixes: 565f0fa902b6 ("xfrm: use a dedicated slab cache for struct xfrm_state")
> Reported-by: Pan Bian <[email protected]>
> Cc: <[email protected]> # v4.18+
> Signed-off-by: Mathias Krause <[email protected]>
Acked-by: Herbert Xu <[email protected]>
Looks good to me. Thanks!
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, Nov 22, 2018 at 10:43:21PM +0800, Herbert Xu wrote:
> On Wed, Nov 21, 2018 at 09:09:23PM +0100, Mathias Krause wrote:
> >
> > -- >8 --
> >
> > Subject: [PATCH] xfrm_user: fix freeing of xfrm states on acquire
> >
> > Commit 565f0fa902b6 ("xfrm: use a dedicated slab cache for struct
> > xfrm_state") moved xfrm state objects to use their own slab cache.
> > However, it missed to adapt xfrm_user to use this new cache when
> > freeing xfrm states.
> >
> > Fix this by introducing and make use of a new helper for freeing
> > xfrm_state objects.
> >
> > Fixes: 565f0fa902b6 ("xfrm: use a dedicated slab cache for struct xfrm_state")
> > Reported-by: Pan Bian <[email protected]>
> > Cc: <[email protected]> # v4.18+
> > Signed-off-by: Mathias Krause <[email protected]>
>
> Acked-by: Herbert Xu <[email protected]>
Applied, thanks everyone!