Message ID | 20201208065036.9458-1-yanjun.zhu@intel.com |
---|---|
State | Superseded |
Headers | show |
Series | [1/1] xdp: avoid calling kfree twice | expand |
On 2020-12-08 07:50, Zhu Yanjun wrote: > From: Zhu Yanjun <zyjzyj2000@gmail.com> > > In the function xdp_umem_pin_pages, if npgs != umem->npgs and > npgs >= 0, the function xdp_umem_unpin_pages is called. In this > function, kfree is called to handle umem->pgs, and then in the > function xdp_umem_pin_pages, kfree is called again to handle > umem->pgs. Eventually, umem->pgs is freed twice. > Hi Zhu, Thanks for the cleanup! kfree(NULL) is valid, so this is not a double-free, but still a nice cleanup! > Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com> > --- > net/xdp/xdp_umem.c | 17 +++++------------ > 1 file changed, 5 insertions(+), 12 deletions(-) > > diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c > index 56a28a686988..ff5173f72920 100644 > --- a/net/xdp/xdp_umem.c > +++ b/net/xdp/xdp_umem.c > @@ -97,7 +97,6 @@ static int xdp_umem_pin_pages(struct xdp_umem *umem, unsigned long address) > { > unsigned int gup_flags = FOLL_WRITE; > long npgs; > - int err; > > umem->pgs = kcalloc(umem->npgs, sizeof(*umem->pgs), > GFP_KERNEL | __GFP_NOWARN); > @@ -112,20 +111,14 @@ static int xdp_umem_pin_pages(struct xdp_umem *umem, unsigned long address) > if (npgs != umem->npgs) { > if (npgs >= 0) { > umem->npgs = npgs; > - err = -ENOMEM; > - goto out_pin; > + xdp_umem_unpin_pages(umem); > + return -ENOMEM; > } > - err = npgs; > - goto out_pgs; > + kfree(umem->pgs); > + umem->pgs = NULL; > + return npgs; I'd like an explicit cast "(int)" here (-Wconversion). Please spin a v2 with the cast, with my: Acked-by: Björn Töpel <bjorn.topel@intel.com> added. Cheers! Björn > } > return 0; > - > -out_pin: > - xdp_umem_unpin_pages(umem); > -out_pgs: > - kfree(umem->pgs); > - umem->pgs = NULL; > - return err; > } > > static int xdp_umem_account_pages(struct xdp_umem *umem) >
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c index 56a28a686988..ff5173f72920 100644 --- a/net/xdp/xdp_umem.c +++ b/net/xdp/xdp_umem.c @@ -97,7 +97,6 @@ static int xdp_umem_pin_pages(struct xdp_umem *umem, unsigned long address) { unsigned int gup_flags = FOLL_WRITE; long npgs; - int err; umem->pgs = kcalloc(umem->npgs, sizeof(*umem->pgs), GFP_KERNEL | __GFP_NOWARN); @@ -112,20 +111,14 @@ static int xdp_umem_pin_pages(struct xdp_umem *umem, unsigned long address) if (npgs != umem->npgs) { if (npgs >= 0) { umem->npgs = npgs; - err = -ENOMEM; - goto out_pin; + xdp_umem_unpin_pages(umem); + return -ENOMEM; } - err = npgs; - goto out_pgs; + kfree(umem->pgs); + umem->pgs = NULL; + return npgs; } return 0; - -out_pin: - xdp_umem_unpin_pages(umem); -out_pgs: - kfree(umem->pgs); - umem->pgs = NULL; - return err; } static int xdp_umem_account_pages(struct xdp_umem *umem)