Re: [PATCH] HID: bpf: Sync KF_SLEEPABLE flags in hid_bpf_syscall_kfuncs set

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [Critical] ZERO_SIZE_PTR dereference in hid_bpf_hw_request() causes a kernel panic.
- [High] Integer promotion bug in hid_bpf_hw_request() silently masks hardware failures.
--

commit 5998989ac7e53dfd091520e5a1a9f8e928c8cf45
Author: Zhao Mengmeng <zhaomengmeng@kylinos.cn>
    
    HID: bpf: Sync KF_SLEEPABLE flags in hid_bpf_syscall_kfuncs set
    
    This commit synchronizes the KF_SLEEPABLE flag for syscall HID-BPF functions
    in the hid_bpf_syscall_kfuncs set. This prevents the flag from being
    inadvertently dropped globally when pahole intersects flags across BTF IDs.
    
    Link: https://lore.kernel.org/sched-ext/177693500312.275653.17323765149266875001.b4-reply@b4/

> diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> index 50c7b45c59e3fb..62db80ead86e51 100644
> --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> @@ -585,11 +585,11 @@ static const struct btf_kfunc_id_set hid_bpf_kfunc_set = {
>  
>  /* for syscall HID-BPF */
>  BTF_KFUNCS_START(hid_bpf_syscall_kfunc_ids)
> -BTF_ID_FLAGS(func, hid_bpf_allocate_context, KF_ACQUIRE | KF_RET_NULL)
> -BTF_ID_FLAGS(func, hid_bpf_release_context, KF_RELEASE)
> -BTF_ID_FLAGS(func, hid_bpf_hw_request)

This isn't a bug in this commit, but since the BTF flags are being updated
for hid_bpf_hw_request(), I noticed a potential issue in that function.

Can a ZERO_SIZE_PTR dereference occur if a HID device has an empty report?

When a BPF program invokes hid_bpf_hw_request() with buf__sz >= 1, and the
device has no fields and no report ID, hid_report_len() can return 0:

drivers/hid/bpf/hid_bpf_dispatch.c:__hid_bpf_hw_check_params() {
	...
	report_len = hid_report_len(report);

	if (*buf__sz > report_len)
		*buf__sz = report_len;
	...
}

This clamps the size to 0. Back in hid_bpf_hw_request(), this 0 size
is passed to kmemdup():

drivers/hid/bpf/hid_bpf_dispatch.c:hid_bpf_hw_request() {
	...
	dma_data = kmemdup(buf, size, GFP_KERNEL);
	if (!dma_data)
		return -ENOMEM;

	ret = hid_ops->hid_hw_raw_request(ctx->hid,
					  dma_data[0],
	...
}

For 0-byte allocations, the slab allocator returns ZERO_SIZE_PTR (0x10),
not NULL. The !dma_data check will pass, and the function will dereference
dma_data[0], potentially causing a panic.

Additionally, can negative error codes from hid_hw_raw_request() be
permanently masked?

The variable ret is an int, while size is a size_t. When hid_hw_raw_request()
fails, it returns a negative error code (e.g., -EIO).

drivers/hid/bpf/hid_bpf_dispatch.c:hid_bpf_hw_request() {
	...
	int ret;
	size_t size = buf__sz;
	...
	ret = hid_ops->hid_hw_raw_request(...);
	
	if (ret > size)
		ret = size;
	...
}

Due to C integer promotion rules, the signed int ret is cast to an unsigned
size_t before comparison. Any negative error code is converted to a large
positive integer, causing ret > size to evaluate to true. The function then
executes the ret = size assignment, masking the hardware failure.

> -BTF_ID_FLAGS(func, hid_bpf_hw_output_report)
> -BTF_ID_FLAGS(func, hid_bpf_input_report)
> +BTF_ID_FLAGS(func, hid_bpf_allocate_context, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE)
> +BTF_ID_FLAGS(func, hid_bpf_release_context, KF_RELEASE | KF_SLEEPABLE)
> +BTF_ID_FLAGS(func, hid_bpf_hw_request, KF_SLEEPABLE)
> +BTF_ID_FLAGS(func, hid_bpf_hw_output_report, KF_SLEEPABLE)
> +BTF_ID_FLAGS(func, hid_bpf_input_report, KF_SLEEPABLE)
>  BTF_KFUNCS_END(hid_bpf_syscall_kfunc_ids)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260424-btf-define-v1-1-0f52dffad5aa@kylinos.cn?part=1