* [PATCH net 1/1] xfrm: ipcomp: Free destination pages on acomp errors [not found] <cover.1777923646.git.zylzyl2333@gmail.com> @ 2026-05-05 8:52 ` Ren Wei 2026-05-05 9:18 ` [v2 PATCH] " Herbert Xu 0 siblings, 1 reply; 3+ messages in thread From: Ren Wei @ 2026-05-05 8:52 UTC (permalink / raw) To: netdev Cc: steffen.klassert, herbert, davem, edumazet, kuba, pabeni, horms, yuantan098, yifanwucs, tomapufckgml, bird, ronbogo, zylzyl2333, n05ec From: Yilin Zhu <zylzyl2333@gmail.com> ipcomp_setup_req() allocates destination pages for the acomp output scatterlist. On successful completion, ipcomp_post_acomp() attaches the used pages to the skb and frees any unused pages. On an acomp error, ipcomp_post_acomp() skips directly to freeing the request. acomp_request_free() only releases the request itself, so the caller-allocated destination pages are left allocated. Track the number of destination pages allocated for the request and free them on the error path before releasing the request. Fixes: eb2953d26971 ("xfrm: ipcomp: Use crypto_acomp interface") Cc: stable@kernel.org Reported-by: Yuan Tan <yuantan098@gmail.com> Reported-by: Yifan Wu <yifanwucs@gmail.com> Reported-by: Juefei Pu <tomapufckgml@gmail.com> Reported-by: Xin Liu <bird@lzu.edu.cn> Co-developed-by: Peihan Liu <ronbogo@outlook.com> Signed-off-by: Peihan Liu <ronbogo@outlook.com> Signed-off-by: Yilin Zhu <zylzyl2333@gmail.com> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn> --- net/xfrm/xfrm_ipcomp.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c index 5f38dff16177..1e0080300ec7 100644 --- a/net/xfrm/xfrm_ipcomp.c +++ b/net/xfrm/xfrm_ipcomp.c @@ -33,6 +33,7 @@ struct ipcomp_data { struct ipcomp_req_extra { struct xfrm_state *x; + int dpages; struct scatterlist sg[]; }; @@ -44,6 +45,12 @@ static inline struct ipcomp_skb_cb *ipcomp_cb(struct sk_buff *skb) return cb; } +static void ipcomp_free_dpages(struct scatterlist *dsg, int dpages) +{ + while (dpages--) + __free_page(sg_page(dsg++)); +} + static int ipcomp_post_acomp(struct sk_buff *skb, int err, int hlen) { struct acomp_req *req = ipcomp_cb(skb)->req; @@ -51,8 +58,13 @@ static int ipcomp_post_acomp(struct sk_buff *skb, int err, int hlen) struct scatterlist *dsg; int len, dlen; - if (unlikely(err)) + if (unlikely(err)) { + if (req) { + extra = acomp_request_extra(req); + ipcomp_free_dpages(extra->sg, extra->dpages); + } goto out_free_req; + } extra = acomp_request_extra(req); dsg = extra->sg; @@ -167,6 +179,7 @@ static struct acomp_req *ipcomp_setup_req(struct xfrm_state *x, extra = acomp_request_extra(req); extra->x = x; + extra->dpages = 0; dsg = extra->sg; sg = dsg + dnfrags; @@ -184,6 +197,7 @@ static struct acomp_req *ipcomp_setup_req(struct xfrm_state *x, if (!page) break; sg_set_page(dsg + i, page, PAGE_SIZE, 0); + extra->dpages++; total += PAGE_SIZE; } if (!i) -- 2.47.3 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* [v2 PATCH] xfrm: ipcomp: Free destination pages on acomp errors 2026-05-05 8:52 ` [PATCH net 1/1] xfrm: ipcomp: Free destination pages on acomp errors Ren Wei @ 2026-05-05 9:18 ` Herbert Xu 2026-05-06 6:33 ` Orion Zhu 0 siblings, 1 reply; 3+ messages in thread From: Herbert Xu @ 2026-05-05 9:18 UTC (permalink / raw) To: Ren Wei Cc: netdev, steffen.klassert, davem, edumazet, kuba, pabeni, horms, yuantan098, yifanwucs, tomapufckgml, bird, ronbogo, zylzyl2333 On Tue, May 05, 2026 at 04:52:59PM +0800, Ren Wei wrote: > From: Yilin Zhu <zylzyl2333@gmail.com> > > ipcomp_setup_req() allocates destination pages for the acomp output > scatterlist. On successful completion, ipcomp_post_acomp() attaches the > used pages to the skb and frees any unused pages. > > On an acomp error, ipcomp_post_acomp() skips directly to freeing the > request. acomp_request_free() only releases the request itself, so the > caller-allocated destination pages are left allocated. > > Track the number of destination pages allocated for the request and free > them on the error path before releasing the request. > > Fixes: eb2953d26971 ("xfrm: ipcomp: Use crypto_acomp interface") > Cc: stable@kernel.org > Reported-by: Yuan Tan <yuantan098@gmail.com> > Reported-by: Yifan Wu <yifanwucs@gmail.com> > Reported-by: Juefei Pu <tomapufckgml@gmail.com> > Reported-by: Xin Liu <bird@lzu.edu.cn> > Co-developed-by: Peihan Liu <ronbogo@outlook.com> > Signed-off-by: Peihan Liu <ronbogo@outlook.com> > Signed-off-by: Yilin Zhu <zylzyl2333@gmail.com> > Signed-off-by: Ren Wei <n05ec@lzu.edu.cn> > --- > net/xfrm/xfrm_ipcomp.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) Thanks for the patch! How about just moving the out_free_req label: ---8<--- Move the out_free_req label up by a couple of lines so that the allocated dst SG list gets freed on error as well as success. Fixes: eb2953d26971 ("xfrm: ipcomp: Use crypto_acomp interface") Cc: stable@kernel.org Reported-by: Yuan Tan <yuantan098@gmail.com> Reported-by: Yifan Wu <yifanwucs@gmail.com> Reported-by: Juefei Pu <tomapufckgml@gmail.com> Reported-by: Xin Liu <bird@lzu.edu.cn> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c index 5f38dff16177..2947321b043d 100644 --- a/net/xfrm/xfrm_ipcomp.c +++ b/net/xfrm/xfrm_ipcomp.c @@ -51,11 +51,12 @@ static int ipcomp_post_acomp(struct sk_buff *skb, int err, int hlen) struct scatterlist *dsg; int len, dlen; + extra = acomp_request_extra(req); + dsg = extra->sg; + if (unlikely(err)) goto out_free_req; - extra = acomp_request_extra(req); - dsg = extra->sg; dlen = req->dlen; pskb_trim_unique(skb, 0); @@ -84,10 +85,10 @@ static int ipcomp_post_acomp(struct sk_buff *skb, int err, int hlen) skb_shinfo(skb)->nr_frags++; } while ((dlen -= len)); +out_free_req: for (; dsg; dsg = sg_next(dsg)) __free_page(sg_page(dsg)); -out_free_req: acomp_request_free(req); return err; } -- 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 related [flat|nested] 3+ messages in thread
* Re: [v2 PATCH] xfrm: ipcomp: Free destination pages on acomp errors 2026-05-05 9:18 ` [v2 PATCH] " Herbert Xu @ 2026-05-06 6:33 ` Orion Zhu 0 siblings, 0 replies; 3+ messages in thread From: Orion Zhu @ 2026-05-06 6:33 UTC (permalink / raw) To: Herbert Xu Cc: Ren Wei, netdev, steffen.klassert, davem, edumazet, kuba, pabeni, horms, yuantan098, yifanwucs, tomapufckgml, bird, ronbogo On Tue, 5 May 2026 at 02:19, Herbert Xu <herbert@gondor.apana.org.au> wrote: > > On Tue, May 05, 2026 at 04:52:59PM +0800, Ren Wei wrote: > > From: Yilin Zhu <zylzyl2333@gmail.com> > > > > ipcomp_setup_req() allocates destination pages for the acomp output > > scatterlist. On successful completion, ipcomp_post_acomp() attaches the > > used pages to the skb and frees any unused pages. > > > > On an acomp error, ipcomp_post_acomp() skips directly to freeing the > > request. acomp_request_free() only releases the request itself, so the > > caller-allocated destination pages are left allocated. > > > > Track the number of destination pages allocated for the request and free > > them on the error path before releasing the request. > > > > Fixes: eb2953d26971 ("xfrm: ipcomp: Use crypto_acomp interface") > > Cc: stable@kernel.org > > Reported-by: Yuan Tan <yuantan098@gmail.com> > > Reported-by: Yifan Wu <yifanwucs@gmail.com> > > Reported-by: Juefei Pu <tomapufckgml@gmail.com> > > Reported-by: Xin Liu <bird@lzu.edu.cn> > > Co-developed-by: Peihan Liu <ronbogo@outlook.com> > > Signed-off-by: Peihan Liu <ronbogo@outlook.com> > > Signed-off-by: Yilin Zhu <zylzyl2333@gmail.com> > > Signed-off-by: Ren Wei <n05ec@lzu.edu.cn> > > --- > > net/xfrm/xfrm_ipcomp.c | 16 +++++++++++++++- > > 1 file changed, 15 insertions(+), 1 deletion(-) > > Thanks for the patch! How about just moving the out_free_req label: > > ---8<--- > Move the out_free_req label up by a couple of lines so that the > allocated dst SG list gets freed on error as well as success. > > Fixes: eb2953d26971 ("xfrm: ipcomp: Use crypto_acomp interface") > Cc: stable@kernel.org > Reported-by: Yuan Tan <yuantan098@gmail.com> > Reported-by: Yifan Wu <yifanwucs@gmail.com> > Reported-by: Juefei Pu <tomapufckgml@gmail.com> > Reported-by: Xin Liu <bird@lzu.edu.cn> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> > > diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c > index 5f38dff16177..2947321b043d 100644 > --- a/net/xfrm/xfrm_ipcomp.c > +++ b/net/xfrm/xfrm_ipcomp.c > @@ -51,11 +51,12 @@ static int ipcomp_post_acomp(struct sk_buff *skb, int err, int hlen) > struct scatterlist *dsg; > int len, dlen; > > + extra = acomp_request_extra(req); > + dsg = extra->sg; > + > if (unlikely(err)) > goto out_free_req; > > - extra = acomp_request_extra(req); > - dsg = extra->sg; > dlen = req->dlen; > > pskb_trim_unique(skb, 0); > @@ -84,10 +85,10 @@ static int ipcomp_post_acomp(struct sk_buff *skb, int err, int hlen) > skb_shinfo(skb)->nr_frags++; > } while ((dlen -= len)); > > +out_free_req: > for (; dsg; dsg = sg_next(dsg)) > __free_page(sg_page(dsg)); > > -out_free_req: > acomp_request_free(req); > return err; > } > -- > 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 Thanks, but I think moving the label is not safe for all paths reaching ipcomp_post_acomp(). ipcomp_post_acomp() is also called when ipcomp_setup_req() returns an ERR_PTR. In particular, acomp_request_alloc_extra() can fail after ipcomp_cb(skb)->req is set to NULL, so moving acomp_request_extra(req) before the error check would dereference NULL. There are also setup failures after the request is allocated but before the destination SG list contains any allocated pages. skb_to_sgvec() can fail before sg_init_table(dsg, dnfrags), and alloc_page() can fail before the first destination page is installed. In those cases walking extra->sg with sg_next() and freeing sg_page(dsg) would operate on an uninitialized or empty destination SG list, potentially freeing NULL or non-owned pages. That is why the patch tracks the number of destination pages actually allocated and frees exactly that count on the acomp error path. Best regards, Yilin Zhu ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-06 6:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1777923646.git.zylzyl2333@gmail.com>
2026-05-05 8:52 ` [PATCH net 1/1] xfrm: ipcomp: Free destination pages on acomp errors Ren Wei
2026-05-05 9:18 ` [v2 PATCH] " Herbert Xu
2026-05-06 6:33 ` Orion Zhu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox