Hi,
On Jun 20 2024, Dan Carpenter wrote:
> Hello Benjamin Tissoires,
>
> Commit 4a86220e046d ("HID: bpf: remove tracing HID-BPF capability")
> from Jun 8, 2024 (linux-next), leads to the following Smatch static
> checker warning:
>
> drivers/hid/bpf/hid_bpf_dispatch.c:65 dispatch_hid_bpf_device_event()
> error: uninitialized symbol 'ret'.
>
> drivers/hid/bpf/hid_bpf_dispatch.c
> 26 dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type, u8 *data,
> 27 u32 *size, int interrupt)
> 28 {
> 29 struct hid_bpf_ctx_kern ctx_kern = {
> 30 .ctx = {
> 31 .hid = hdev,
> 32 .allocated_size = hdev->bpf.allocated_data,
> 33 .size = *size,
> 34 },
> 35 .data = hdev->bpf.device_data,
> 36 };
> 37 struct hid_bpf_ops *e;
> 38 int ret;
> 39
> 40 if (type >= HID_REPORT_TYPES)
> 41 return ERR_PTR(-EINVAL);
> 42
> 43 /* no program has been attached yet */
> 44 if (!hdev->bpf.device_data)
> 45 return data;
> 46
> 47 memset(ctx_kern.data, 0, hdev->bpf.allocated_data);
> 48 memcpy(ctx_kern.data, data, *size);
> 49
> 50 rcu_read_lock();
> 51 list_for_each_entry_rcu(e, &hdev->bpf.prog_list, list) {
> 52 if (e->hid_device_event) {
> 53 ret = e->hid_device_event(&ctx_kern.ctx, type);
> 54 if (ret < 0) {
> 55 rcu_read_unlock();
> 56 return ERR_PTR(ret);
> 57 }
> 58
> 59 if (ret)
> 60 ctx_kern.ctx.retval = ret;
> 61 }
>
> Can all the e->hid_device_event pointers be NULL?
They can, but then hdev->bpf.device_data will also set to NULL in 99% of
the normal cases. The only case where device_data is set to a value and
hid_device_event is never set anywhere is after the last bpf filter has
been removed.
>
> 62 }
> 63 rcu_read_unlock();
> 64
> 65 if (ret) {
> ^^^
> If so then ret is uninitialized
Yep, good point. Let me fix that in my followup series.
>
> 66 if (ret > ctx_kern.ctx.allocated_size)
> 67 return ERR_PTR(-EINVAL);
> 68
> 69 *size = ret;
>
>
> regards,
> dan carpenter
Thanks!
Cheers,
Benjamin
