| Message ID | 20201208065036.9458-1-yanjun.zhu@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [1/1] xdp: avoid calling kfree twice | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 1 this patch: 1 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 32 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 1 this patch: 1 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
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)