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