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

[sashiko-bot@kernel.org]

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
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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722233518.778854-1-ihor.solodrai@linux.dev?part=6