On Thu, Sep 12, 2024 at 09:28:44AM +0800, Hou Tao wrote:
> From: Hou Tao <houtao1@xxxxxxxxxx>
>
> When security_bpf_map_create() in map_create() fails, map_create() will
> call btf_put() and ->map_free() callback to free the map. It doesn't
> free the btf_record of map value, so add the missed btf_record_free()
> when map creation fails.
>
> However btf_record_free() needs to be called after ->map_free() just
> like bpf_map_free_deferred() did, because ->map_free() may use the
> btf_record to free the special fields in preallocated map value. So
> factor out bpf_map_free() helper to free the map, btf_record, and btf
> orderly and use the helper in both map_create() and
> bpf_map_free_deferred().
lgtm
Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx>
jirka
>
> Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
> ---
> kernel/bpf/syscall.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6988e432fc3d..1a0dc1d81118 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -735,15 +735,11 @@ void bpf_obj_free_fields(const struct btf_record *rec, void *obj)
> }
> }
>
> -/* called from workqueue */
> -static void bpf_map_free_deferred(struct work_struct *work)
> +static void bpf_map_free(struct bpf_map *map)
> {
> - struct bpf_map *map = container_of(work, struct bpf_map, work);
> struct btf_record *rec = map->record;
> struct btf *btf = map->btf;
>
> - security_bpf_map_free(map);
> - bpf_map_release_memcg(map);
> /* implementation dependent freeing */
> map->ops->map_free(map);
> /* Delay freeing of btf_record for maps, as map_free
> @@ -762,6 +758,16 @@ static void bpf_map_free_deferred(struct work_struct *work)
> btf_put(btf);
> }
>
> +/* called from workqueue */
> +static void bpf_map_free_deferred(struct work_struct *work)
> +{
> + struct bpf_map *map = container_of(work, struct bpf_map, work);
> +
> + security_bpf_map_free(map);
> + bpf_map_release_memcg(map);
> + bpf_map_free(map);
> +}
> +
> static void bpf_map_put_uref(struct bpf_map *map)
> {
> if (atomic64_dec_and_test(&map->usercnt)) {
> @@ -1413,8 +1419,7 @@ static int map_create(union bpf_attr *attr)
> free_map_sec:
> security_bpf_map_free(map);
> free_map:
> - btf_put(map->btf);
> - map->ops->map_free(map);
> + bpf_map_free(map);
> put_token:
> bpf_token_put(token);
> return err;
> --
> 2.29.2
>
