Re: [PATCH bpf-next v1 6/8] HID: bpf: Make syscall kfunc flags match the struct_ops set

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 7/22/26 4:49 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] TOCTOU NULL pointer dereference in `dispatch_hid_bpf_device_event` during dynamic HID-BPF attachment.
> --
> 
> --- Patch [6]: [PATCH bpf-next v1 6/8] HID: bpf: Make syscall kfunc flags match the struct_ops set ---
> Note: The format of this review is altered to provide a free-form summary
> due to text recitation restrictions. No direct patch diff quoting is used.
> 
> commit 460380780bad5817b193c7927dc05e04ce9f5480
> Author: Ihor Solodrai <ihor.solodrai@linux.dev>
> Subject: HID: bpf: Make syscall kfunc flags match the struct_ops set
> 
> This commit updates the kfunc flags for hid_bpf_syscall_kfunc_ids to match
> the hid_bpf_kfunc_ids set by adding the KF_SLEEPABLE flag. This ensures
> per-kfunc flag consistency across BTF ID sets at build time.
> 
> [Severity: High]
> This isn't a regression introduced by this patch, but while reviewing the

Not relevant to the change.

However HID maintainers may be interested, adding them to cc.


> HID-BPF event dispatching code, a potential time-of-check to time-of-use
> issue was noticed in drivers/hid/bpf/hid_bpf_dispatch.c.
> 
> Could this sequence lead to a null pointer dereference when a HID-BPF
> program is concurrently attached while a device is flooded with input
> reports?
> 
> In dispatch_hid_bpf_device_event(), the first lockless read of the device
> data happens during context initialization:
> 
>     .data = hdev->bpf.device_data,
> 
> Shortly after, a second lockless read is used as a guard:
> 
>     if (!hdev->bpf.device_data)
>         return data;
> 
> If __hid_bpf_allocate_data() is concurrently executed during a program
> attachment, it updates these pointers without memory barriers or taking the
> driver_input_lock:
> 
>     *data = alloc_data;
>     *size = alloc_size;
> 
> If the allocation completes exactly between the two reads in
> dispatch_hid_bpf_device_event(), the first read could evaluate to null
> while the second read sees the newly allocated pointer and evaluates to
> true. This would allow the code to bypass the guard and execute
> memset(ctx_kern.data, 0, hdev->bpf.allocated_data) with a null pointer.
> 
> Would it be beneficial to use READ_ONCE() or proper synchronization here to
> ensure consistent evaluation of the device data pointer?
>