* [net] xfrm_user: use xfrm_state_put to free xfrm_state_alloc return value @ 2018-11-21 6:57 Pan Bian 2018-11-21 8:00 ` Herbert Xu 0 siblings, 1 reply; 5+ messages in thread From: Pan Bian @ 2018-11-21 6:57 UTC (permalink / raw) To: Steffen Klassert, Herbert Xu, David S. Miller Cc: netdev, linux-kernel, Pan Bian From: Pan Bian <bianpan2013@163.com> The memory chunk allocated by xfrm_state_alloc() should be released with xfrm_state_put(), not kfree. Signed-off-by: Pan Bian <bianpan2013@163.com> --- 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [net] xfrm_user: use xfrm_state_put to free xfrm_state_alloc return value 2018-11-21 6:57 [net] xfrm_user: use xfrm_state_put to free xfrm_state_alloc return value Pan Bian @ 2018-11-21 8:00 ` Herbert Xu 2018-11-21 20:09 ` Mathias Krause 0 siblings, 1 reply; 5+ messages in thread From: Herbert Xu @ 2018-11-21 8:00 UTC (permalink / raw) To: Pan Bian Cc: Steffen Klassert, David S. Miller, netdev, linux-kernel, Pan Bian, Mathias Krause On Wed, Nov 21, 2018 at 02:57:48PM +0800, Pan Bian wrote: > From: Pan Bian <bianpan2013@163.com> > > The memory chunk allocated by xfrm_state_alloc() should be released with > xfrm_state_put(), not kfree. > > Signed-off-by: Pan Bian <bianpan2013@163.com> This bug was introduced by commit 565f0fa902b64020d5d147ff1708567e9e0b6e49 Author: Mathias Krause <minipli@googlemail.com> 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 <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [net] xfrm_user: use xfrm_state_put to free xfrm_state_alloc return value 2018-11-21 8:00 ` Herbert Xu @ 2018-11-21 20:09 ` Mathias Krause 2018-11-22 14:43 ` Herbert Xu 0 siblings, 1 reply; 5+ messages in thread From: Mathias Krause @ 2018-11-21 20:09 UTC (permalink / raw) To: Herbert Xu, Steffen Klassert Cc: Pan Bian, David S. Miller, netdev, linux-kernel, Pan Bian, minipli 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 <bianpan2013@163.com> > > > > The memory chunk allocated by xfrm_state_alloc() should be released with > > xfrm_state_put(), not kfree. > > > > Signed-off-by: Pan Bian <bianpan2013@163.com> > > This bug was introduced by > > commit 565f0fa902b64020d5d147ff1708567e9e0b6e49 > Author: Mathias Krause <minipli@googlemail.com> > 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 <bianpan2016@163.com> Cc: <stable@vger.kernel.org> # v4.18+ Signed-off-by: Mathias Krause <minipli@googlemail.com> --- 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; } ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [net] xfrm_user: use xfrm_state_put to free xfrm_state_alloc return value 2018-11-21 20:09 ` Mathias Krause @ 2018-11-22 14:43 ` Herbert Xu 2018-11-27 7:55 ` Steffen Klassert 0 siblings, 1 reply; 5+ messages in thread From: Herbert Xu @ 2018-11-22 14:43 UTC (permalink / raw) To: Mathias Krause Cc: Steffen Klassert, Pan Bian, David S. Miller, netdev, linux-kernel, Pan Bian 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 <bianpan2016@163.com> > Cc: <stable@vger.kernel.org> # v4.18+ > Signed-off-by: Mathias Krause <minipli@googlemail.com> Acked-by: Herbert Xu <herbert@gondor.apana.org.au> Looks good to me. Thanks! -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [net] xfrm_user: use xfrm_state_put to free xfrm_state_alloc return value 2018-11-22 14:43 ` Herbert Xu @ 2018-11-27 7:55 ` Steffen Klassert 0 siblings, 0 replies; 5+ messages in thread From: Steffen Klassert @ 2018-11-27 7:55 UTC (permalink / raw) To: Herbert Xu Cc: Mathias Krause, Pan Bian, David S. Miller, netdev, linux-kernel, Pan Bian 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 <bianpan2016@163.com> > > Cc: <stable@vger.kernel.org> # v4.18+ > > Signed-off-by: Mathias Krause <minipli@googlemail.com> > > Acked-by: Herbert Xu <herbert@gondor.apana.org.au> Applied, thanks everyone! ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-27 7:55 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-21 6:57 [net] xfrm_user: use xfrm_state_put to free xfrm_state_alloc return value Pan Bian 2018-11-21 8:00 ` Herbert Xu 2018-11-21 20:09 ` Mathias Krause 2018-11-22 14:43 ` Herbert Xu 2018-11-27 7:55 ` Steffen Klassert
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.