Re: [PATCHv7 01/13] bpf: Introduce kfuncs to parser buffer content

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]

> diff --git a/kernel/bpf/bpf_buffer_parser.c b/kernel/bpf/bpf_buffer_parser.c
> new file mode 100644
> index 000000000000..5d5c06833079
> --- /dev/null
> +++ b/kernel/bpf/bpf_buffer_parser.c

[ ... ]

> +static void release_bpf_parser_context(struct kref *kref)
> +{
> +	struct bpf_parser_context *ctx = container_of(kref, struct bpf_parser_context, ref);
> +
> +	if (ctx->buf) {
> +		vfree(ctx->buf->buf);
> +		kfree(ctx->buf);
> +	}

If release_bpf_parser_context() runs while bpf_buffer_parser() has
stored the sentinel (void *)1 in ctx->buf via:

    old_val = cmpxchg(&context->buf, NULL, (void *)1);

then ctx->buf is non-NULL but not a valid struct bpf_parser_buf
pointer, and vfree(ctx->buf->buf) will dereference address 1.

The BPF verifier prevents this today by holding the acquired reference
across the kfunc call, but release_bpf_parser_context() has no
awareness of the sentinel value.  Would it be worth adding a check
here to handle the sentinel, or using a separate flag/mutex instead
of overloading ctx->buf?

[ ... ]

> +__bpf_kfunc int bpf_buffer_parser(char *buf, int buf_sz,
> +		struct bpf_parser_context *context)
> +{

[ ... ]

> +	ret = copy_from_kernel_nofault(b, buf, buf_sz);
> +	if (!!ret) {

The !!ret is equivalent to just testing ret here.  Was the double
negation intentional?

> +		context->buf = NULL;
> +		vfree(b);
> +		return ret;
> +	}

[ ... ]

> +static const struct btf_kfunc_id_set buffer_parser_kfunc_set = {
> +        .owner = THIS_MODULE,
> +        .set   = &buffer_parser_ids,
> +};

These two lines use spaces for indentation instead of tabs.

> +
> +

There is a double blank line here.

> +BTF_ID_LIST(buffer_parser_dtor_ids)

[ ... ]

> +	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &buffer_parser_kfunc_set);
> +	return  ret ?: register_btf_id_dtor_kfuncs(buffer_parser_dtors,

There is a double space before ret.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/23393379437