[bug report] HID: bpf: remove tracing HID-BPF capability

[bug report] HID: bpf: remove tracing HID-BPF capability

[Dan Carpenter]

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?

    62          }
    63          rcu_read_unlock();
    64  
    65          if (ret) {
                    ^^^
If so then ret is uninitialized

    66                  if (ret > ctx_kern.ctx.allocated_size)
    67                          return ERR_PTR(-EINVAL);
    68  
    69                  *size = ret;


regards,
dan carpenter

Re: [bug report] HID: bpf: remove tracing HID-BPF capability

[Benjamin Tissoires]

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