On 9/1/26 7:28 AM, [email protected] wrote:
> From: Yuan Chen <[email protected]>
>
> 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 <[email protected]>
> ---
> 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;
> }
