Re: [PATCH bpf-next v3 2/4] bpf: Fix use-after-free of program BTF in mem-alloc destructor

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 9/1/26 7:28 AM, chenyuan_fl@163.com wrote:
> From: Yuan Chen <chenyuan@kylinos.cn>
> 
> bpf_ma_set_dtor() duplicates the map's btf_record for the bpf_mem_alloc
> destructor. btf_record_dup() only borrows the BTF references held by
> the fields: kptrs and list_head/rb_root fields point at the program
> BTF, whose lifetime is independent of the map. The duplicated record is
> used from the deferred mem-alloc destructor workqueue, which can run
> after the program BTF is gone (bpf_map_free() drops the map's
> reference, and the RCU callback may run first). Reading the borrowed
> descriptors there is a use-after-free, detected by KASAN as
> "slab-use-after-free in btf_is_kernel".
> 
> Keep a reference on the borrowed program BTFs for the lifetime of the
> duplicated record. The record is freed from a preemptible worker, so
> the last btf_put() (which only schedules RCU destruction) does not make
> the field descriptors safe to read; snapshot the borrowed BTFs, free
> the record, then drop the references.
> 
> The rhtab kptr selftests exercise this path on every map teardown and
> triggered the bug under KASAN; with this fix they pass cleanly.

Does htab trigger the same kasan?

> 
> Fixes: 1df97a7453ee ("bpf: Register dtor for freeing special fields")
> Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
> ---
>  kernel/bpf/hashtab.c | 66 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index aaedda3730f3..30bcc573772e 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -493,11 +493,76 @@ static void htab_pcpu_mem_dtor(void *obj, void *ctx)
>  		bpf_obj_free_fields(hrec->record, per_cpu_ptr(pptr, cpu));
>  }
>  
> +/*
> + * The duplicated record borrows program BTFs, but is freed from a deferred
> + * workqueue that may run after the program BTF is gone. Hold a reference for
> + * the record's lifetime and snapshot the borrowed BTFs, since
> + * btf_record_free() frees the record.
> + */
> +
> +/* Program BTF borrowed by the field, or NULL. */
> +static struct btf *htab_field_borrowed_btf(const struct btf_field *field)
> +{
> +	struct btf *btf = NULL;
> +
> +	switch (field->type) {
> +	case BPF_KPTR_UNREF:
> +	case BPF_KPTR_REF:
> +	case BPF_KPTR_PERCPU:
> +	case BPF_UPTR:
> +		btf = field->kptr.btf;
> +		break;
> +	case BPF_LIST_HEAD:
> +	case BPF_RB_ROOT:
> +		btf = field->graph_root.btf;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	if (btf && !btf_is_kernel(btf))
> +		return btf;
> +	return NULL;
> +}
> +
> +/* Snapshot the program BTFs @rec borrows into @btfs; returns their count. */
> +static int htab_record_prog_btfs_snapshot(struct btf_record *rec,
> +					  struct btf **btfs)
> +{
> +	int i, n = 0;
> +
> +	if (IS_ERR_OR_NULL(rec))
> +		return 0;
> +
> +	for (i = 0; i < rec->cnt; i++) {
> +		struct btf *btf = htab_field_borrowed_btf(&rec->fields[i]);
> +
> +		if (btf)
> +			btfs[n++] = btf;
> +	}
> +	return n;
> +}
> +
> +static void htab_record_prog_btf_get(struct btf_record *rec)
> +{
> +	struct btf *btfs[BTF_FIELDS_MAX];

it looks like temporary array and htab_record_prog_btfs_snapshot()
are unnecessary:

for (i = 0; i < rec->cnt; i++) {
        struct btf *btf = htab_field_borrowed_btf(&rec->fields[i]);

        if (btf)
	        btf_get(btf);
}

> +	int i, n;
> +
> +	n = htab_record_prog_btfs_snapshot(rec, btfs);
> +	for (i = 0; i < n; i++)
> +		btf_get(btfs[i]);
> +}
> +
>  static void htab_dtor_ctx_free(void *ctx)
>  {
>  	struct htab_btf_record *hrec = ctx;
> +	struct btf *btfs[BTF_FIELDS_MAX];
> +	int i, n;
>  
> +	n = htab_record_prog_btfs_snapshot(hrec->record, btfs);
>  	btf_record_free(hrec->record);
> +	for (i = 0; i < n; i++)
> +		btf_put(btfs[i]);
>  	kfree(ctx);
>  }
>  
> @@ -521,6 +586,7 @@ static int bpf_ma_set_dtor(struct bpf_map *map, struct bpf_mem_alloc *ma,
>  		kfree(hrec);
>  		return err;
>  	}
> +	htab_record_prog_btf_get(hrec->record);
>  	bpf_mem_alloc_set_dtor(ma, dtor, htab_dtor_ctx_free, hrec);
>  	return 0;
>  }