| Message ID | 20210707043811.5349-2-hefengqing@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | potential memleak and use after free in bpf verifier | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 10 of 10 maintainers |
| 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: 3290 this patch: 3290 |
| netdev/kdoc | success | Errors and warnings before: 1 this patch: 1 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 53 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 3387 this patch: 3387 |
| netdev/header_inline | success | Link |
On Tue, Jul 6, 2021 at 8:53 PM He Fengqing <hefengqing@huawei.com> wrote: > > Move bpf_prog_clone_free function into filter.h, so we can use > it in other file. > > Signed-off-by: He Fengqing <hefengqing@huawei.com> > --- > include/linux/filter.h | 15 +++++++++++++++ > kernel/bpf/core.c | 20 +------------------- > 2 files changed, 16 insertions(+), 19 deletions(-) > > diff --git a/include/linux/filter.h b/include/linux/filter.h > index 472f97074da0..f39e008a377d 100644 > --- a/include/linux/filter.h > +++ b/include/linux/filter.h > @@ -884,6 +884,21 @@ struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size, > gfp_t gfp_extra_flags); > void __bpf_prog_free(struct bpf_prog *fp); > > +static inline void bpf_prog_clone_free(struct bpf_prog *fp) > +{ > + /* aux was stolen by the other clone, so we cannot free > + * it from this path! It will be freed eventually by the > + * other program on release. > + * > + * At this point, we don't need a deferred release since > + * clone is guaranteed to not be locked. > + */ > + fp->aux = NULL; > + fp->stats = NULL; > + fp->active = NULL; > + __bpf_prog_free(fp); > +} > + > static inline void bpf_prog_unlock_free(struct bpf_prog *fp) > { > __bpf_prog_free(fp); > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > index 034ad93a1ad7..49b0311f48c1 100644 > --- a/kernel/bpf/core.c > +++ b/kernel/bpf/core.c > @@ -238,10 +238,7 @@ struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size, > /* We keep fp->aux from fp_old around in the new > * reallocated structure. > */ After the change, we can remove the comment above. > - fp_old->aux = NULL; > - fp_old->stats = NULL; > - fp_old->active = NULL; > - __bpf_prog_free(fp_old); > + bpf_prog_clone_free(fp_old); Please add a couple sentences in the commit log about this chanage. Thanks, Song
diff --git a/include/linux/filter.h b/include/linux/filter.h index 472f97074da0..f39e008a377d 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -884,6 +884,21 @@ struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size, gfp_t gfp_extra_flags); void __bpf_prog_free(struct bpf_prog *fp); +static inline void bpf_prog_clone_free(struct bpf_prog *fp) +{ + /* aux was stolen by the other clone, so we cannot free + * it from this path! It will be freed eventually by the + * other program on release. + * + * At this point, we don't need a deferred release since + * clone is guaranteed to not be locked. + */ + fp->aux = NULL; + fp->stats = NULL; + fp->active = NULL; + __bpf_prog_free(fp); +} + static inline void bpf_prog_unlock_free(struct bpf_prog *fp) { __bpf_prog_free(fp); diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 034ad93a1ad7..49b0311f48c1 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -238,10 +238,7 @@ struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size, /* We keep fp->aux from fp_old around in the new * reallocated structure. */ - fp_old->aux = NULL; - fp_old->stats = NULL; - fp_old->active = NULL; - __bpf_prog_free(fp_old); + bpf_prog_clone_free(fp_old); } return fp; @@ -1102,21 +1099,6 @@ static struct bpf_prog *bpf_prog_clone_create(struct bpf_prog *fp_other, return fp; } -static void bpf_prog_clone_free(struct bpf_prog *fp) -{ - /* aux was stolen by the other clone, so we cannot free - * it from this path! It will be freed eventually by the - * other program on release. - * - * At this point, we don't need a deferred release since - * clone is guaranteed to not be locked. - */ - fp->aux = NULL; - fp->stats = NULL; - fp->active = NULL; - __bpf_prog_free(fp); -} - void bpf_jit_prog_release_other(struct bpf_prog *fp, struct bpf_prog *fp_other) { /* We have to repoint aux->prog to self, as we don't
Move bpf_prog_clone_free function into filter.h, so we can use it in other file. Signed-off-by: He Fengqing <hefengqing@huawei.com> --- include/linux/filter.h | 15 +++++++++++++++ kernel/bpf/core.c | 20 +------------------- 2 files changed, 16 insertions(+), 19 deletions(-)