[bug report] staging: vchiq_arm: use list_for_each_entry when accessing bulk_waiter_list

Dan Carpenter dan.carpenter at oracle.com
Fri Jun 19 14:31:32 UTC 2020


Hello Nicolas Saenz Julienne,

The patch 46e4b9ec4fa4: "staging: vchiq_arm: use list_for_each_entry
when accessing bulk_waiter_list" from Nov 20, 2018, leads to the
following static checker warning:

	drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:427 vchiq_blocking_bulk_transfer()
	warn: iterator used outside loop: 'waiter'

drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
   417          mutex_lock(&instance->bulk_waiter_list_mutex);
   418          list_for_each_entry(waiter, &instance->bulk_waiter_list, list) {
                ^^^^^^^^^^^^^^^^^^^^^^^^^^
The list iterator is always non-NULL.

   419                  if (waiter->pid == current->pid) {
   420                          list_del(&waiter->list);
   421                          break;
   422                  }
   423          }
   424          mutex_unlock(&instance->bulk_waiter_list_mutex);
   425  
   426          if (waiter) {
                    ^^^^^^
In the original code "waiter" was only non-NULL if we found the correct
pid, but now it's always non-NULL.

   427                  struct vchiq_bulk *bulk = waiter->bulk_waiter.bulk;
   428  
   429                  if (bulk) {
   430                          /* This thread has an outstanding bulk transfer. */
   431                          if ((bulk->data != data) ||
   432                                  (bulk->size != size)) {
   433                                  /* This is not a retry of the previous one.
   434                                   * Cancel the signal when the transfer
   435                                   * completes.
   436                                   */
   437                                  spin_lock(&bulk_waiter_spinlock);
   438                                  bulk->userdata = NULL;
   439                                  spin_unlock(&bulk_waiter_spinlock);
   440                          }
   441                  }
   442          }
   443  
   444          if (!waiter) {
                    ^^^^^^^
This is dead code now.  I'm a bit surprised this bug didn't show up
during testing.

   445                  waiter = kzalloc(sizeof(struct bulk_waiter_node), GFP_KERNEL);
   446                  if (!waiter) {
   447                          vchiq_log_error(vchiq_core_log_level,
   448                                  "%s - out of memory", __func__);
   449                          return VCHIQ_ERROR;
   450                  }
   451          }
   452  
   453          status = vchiq_bulk_transfer(handle, data, size, &waiter->bulk_waiter,

regards,
dan carpenter


More information about the devel mailing list