| Message ID | 20191129120539.11861-1-stefan.bader@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,E/D/B] xfrm: Fix memleak on xfrm state destroy | expand |
On 29.11.19 13:05, Stefan Bader wrote: > From: Steffen Klassert <steffen.klassert@secunet.com> > > We leak the page that we use to create skb page fragments > when destroying the xfrm_state. Fix this by dropping a > page reference if a page was assigned to the xfrm_state. > > Fixes: cac2661c53f3 ("esp4: Avoid skb_cow_data whenever possible") > Reported-by: JD <jdtxs00@gmail.com> > Reported-by: Paul Wouters <paul@nohats.ca> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> > > BugLink: https://bugs.launchpad.net/bugs/1853197 > > (cherry picked from commit 86c6739eda7d2a03f2db30cbee67a5fb81afa8ba) > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > > This fixes a memory leak which appears to loose 8 pages for each ipsec > connection that is done. Issue was introduced in v4.11 and fixed in v5.4 > so unstable should have it. > > Needs -C2 to apply to Bionic [build-tested in Bionic, too]. > > net/xfrm/xfrm_state.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c > index c6f3c4a1bd99..f3423562d933 100644 > --- a/net/xfrm/xfrm_state.c > +++ b/net/xfrm/xfrm_state.c > @@ -495,6 +495,8 @@ static void ___xfrm_state_destroy(struct xfrm_state *x) > x->type->destructor(x); > xfrm_put_type(x->type); > } > + if (x->xfrag.page) > + put_page(x->xfrag.page); > xfrm_dev_state_free(x); > security_xfrm_state_free(x); > xfrm_state_free(x); >
On Fri, Nov 29, 2019 at 01:05:39PM +0100, Stefan Bader wrote: > From: Steffen Klassert <steffen.klassert@secunet.com> > > We leak the page that we use to create skb page fragments > when destroying the xfrm_state. Fix this by dropping a > page reference if a page was assigned to the xfrm_state. > > Fixes: cac2661c53f3 ("esp4: Avoid skb_cow_data whenever possible") > Reported-by: JD <jdtxs00@gmail.com> > Reported-by: Paul Wouters <paul@nohats.ca> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> > > BugLink: https://bugs.launchpad.net/bugs/1853197 > > (cherry picked from commit 86c6739eda7d2a03f2db30cbee67a5fb81afa8ba) > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- > > This fixes a memory leak which appears to loose 8 pages for each ipsec > connection that is done. Issue was introduced in v4.11 and fixed in v5.4 > so unstable should have it. > > Needs -C2 to apply to Bionic [build-tested in Bionic, too]. > > net/xfrm/xfrm_state.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c > index c6f3c4a1bd99..f3423562d933 100644 > --- a/net/xfrm/xfrm_state.c > +++ b/net/xfrm/xfrm_state.c > @@ -495,6 +495,8 @@ static void ___xfrm_state_destroy(struct xfrm_state *x) > x->type->destructor(x); > xfrm_put_type(x->type); > } > + if (x->xfrag.page) > + put_page(x->xfrag.page); > xfrm_dev_state_free(x); > security_xfrm_state_free(x); > xfrm_state_free(x); > -- > 2.17.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team Acked-by: Sultan Alsawaf <sultan.alsawaf@canonical.com>
The patch didn't apply cleanly to Bionic. The affected function was called xfrm_state_gc_destroy() in Bionic (not ___xfrm_state_destroy()). I updated the patch for Bionic accordingly. On 2019-11-29 13:05:39 , Stefan Bader wrote: > From: Steffen Klassert <steffen.klassert@secunet.com> > > We leak the page that we use to create skb page fragments > when destroying the xfrm_state. Fix this by dropping a > page reference if a page was assigned to the xfrm_state. > > Fixes: cac2661c53f3 ("esp4: Avoid skb_cow_data whenever possible") > Reported-by: JD <jdtxs00@gmail.com> > Reported-by: Paul Wouters <paul@nohats.ca> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> > > BugLink: https://bugs.launchpad.net/bugs/1853197 > > (cherry picked from commit 86c6739eda7d2a03f2db30cbee67a5fb81afa8ba) > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- > > This fixes a memory leak which appears to loose 8 pages for each ipsec > connection that is done. Issue was introduced in v4.11 and fixed in v5.4 > so unstable should have it. > > Needs -C2 to apply to Bionic [build-tested in Bionic, too]. > > net/xfrm/xfrm_state.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c > index c6f3c4a1bd99..f3423562d933 100644 > --- a/net/xfrm/xfrm_state.c > +++ b/net/xfrm/xfrm_state.c > @@ -495,6 +495,8 @@ static void ___xfrm_state_destroy(struct xfrm_state *x) > x->type->destructor(x); > xfrm_put_type(x->type); > } > + if (x->xfrag.page) > + put_page(x->xfrag.page); > xfrm_dev_state_free(x); > security_xfrm_state_free(x); > xfrm_state_free(x); > -- > 2.17.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 02.12.19 08:52, Khaled Elmously wrote: > The patch didn't apply cleanly to Bionic. The affected function was called xfrm_state_gc_destroy() in Bionic (not ___xfrm_state_destroy()). > > I updated the patch for Bionic accordingly. > > On 2019-11-29 13:05:39 , Stefan Bader wrote: >> From: Steffen Klassert <steffen.klassert@secunet.com> >> >> We leak the page that we use to create skb page fragments >> when destroying the xfrm_state. Fix this by dropping a >> page reference if a page was assigned to the xfrm_state. >> >> Fixes: cac2661c53f3 ("esp4: Avoid skb_cow_data whenever possible") >> Reported-by: JD <jdtxs00@gmail.com> >> Reported-by: Paul Wouters <paul@nohats.ca> >> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> >> >> BugLink: https://bugs.launchpad.net/bugs/1853197 >> >> (cherry picked from commit 86c6739eda7d2a03f2db30cbee67a5fb81afa8ba) >> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> >> --- >> >> This fixes a memory leak which appears to loose 8 pages for each ipsec >> connection that is done. Issue was introduced in v4.11 and fixed in v5.4 >> so unstable should have it. >> >> Needs -C2 to apply to Bionic [build-tested in Bionic, too]. You might read the comments ^ >> >> net/xfrm/xfrm_state.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c >> index c6f3c4a1bd99..f3423562d933 100644 >> --- a/net/xfrm/xfrm_state.c >> +++ b/net/xfrm/xfrm_state.c >> @@ -495,6 +495,8 @@ static void ___xfrm_state_destroy(struct xfrm_state *x) >> x->type->destructor(x); >> xfrm_put_type(x->type); >> } >> + if (x->xfrag.page) >> + put_page(x->xfrag.page); >> xfrm_dev_state_free(x); >> security_xfrm_state_free(x); >> xfrm_state_free(x); >> -- >> 2.17.1 >> >> >> -- >> kernel-team mailing list >> kernel-team@lists.ubuntu.com >> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index c6f3c4a1bd99..f3423562d933 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -495,6 +495,8 @@ static void ___xfrm_state_destroy(struct xfrm_state *x) x->type->destructor(x); xfrm_put_type(x->type); } + if (x->xfrag.page) + put_page(x->xfrag.page); xfrm_dev_state_free(x); security_xfrm_state_free(x); xfrm_state_free(x);