Hi Dan,

Dan Carpenter <dan.carpen...@oracle.com> wrote:

Hello Kai-Heng Feng,

This is a semi-automatic email about new static checker warnings.

I ran Smatch but didn't see the error message:
$ make -j`nproc` CHECK="~/smatch/smatch -p=kernel" C=1 bzImage modules | tee warns.txt`"

Also, "val" will never get passed as null in quirks_param_set() by the .set() caller, so it's actually superfluous.

Kai-Heng


The patch 027bd6cafd9a: "usb: core: Add "quirks" parameter for
usbcore" from Mar 20, 2018, leads to the following Smatch complaint:

    drivers/usb/core/quirks.c:136 quirks_param_set()
    error: we previously assumed 'val' could be null (see line 37)

drivers/usb/core/quirks.c
    36  
    37          if (!val || !*val) {
                    ^^^^
Patch adds check for NULL

    38                  quirk_count = 0;
    39                  kfree(quirk_list);
    40                  quirk_list = NULL;
    41                  goto unlock;
    42          }
    43  
    44          for (quirk_count = 1, i = 0; val[i]; i++)
    45                  if (val[i] == ',')
    46                          quirk_count++;
    47  
    48          if (quirk_list) {
    49                  kfree(quirk_list);
    50                  quirk_list = NULL;
    51          }
    52  
    53          quirk_list = kcalloc(quirk_count, sizeof(struct quirk_entry),
    54                               GFP_KERNEL);
    55          if (!quirk_list) {
    56                  mutex_unlock(&quirk_mutex);
    57                  return -ENOMEM;
    58          }
    59  
    60          for (i = 0, p = (char *)val; p && *p;) {
    61                  /* Each entry consists of VID:PID:flags */
    62                  field = strsep(&p, ":");
    63                  if (!field)
    64                          break;
    65  
    66                  if (kstrtou16(field, 16, &vid))
    67                          break;
    68  
    69                  field = strsep(&p, ":");
    70                  if (!field)
    71                          break;
    72  
    73                  if (kstrtou16(field, 16, &pid))
    74                          break;
    75  
    76                  field = strsep(&p, ",");
    77                  if (!field || !*field)
    78                          break;
    79  
    80                  /* Collect the flags */
    81                  for (flags = 0; *field; field++) {
    82                          switch (*field) {
    83                          case 'a':
    84                                  flags |= USB_QUIRK_STRING_FETCH_255;
    85                                  break;
    86                          case 'b':
    87                                  flags |= USB_QUIRK_RESET_RESUME;
    88                                  break;
    89                          case 'c':
    90                                  flags |= USB_QUIRK_NO_SET_INTF;
    91                                  break;
    92                          case 'd':
    93                                  flags |= USB_QUIRK_CONFIG_INTF_STRINGS;
    94                                  break;
    95                          case 'e':
    96                                  flags |= USB_QUIRK_RESET;
    97                                  break;
    98                          case 'f':
    99                                  flags |= USB_QUIRK_HONOR_BNUMINTERFACES;
   100                                  break;
   101                          case 'g':
   102                                  flags |= USB_QUIRK_DELAY_INIT;
   103                                  break;
   104                          case 'h':
   105                                  flags |= 
USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL;
   106                                  break;
   107                          case 'i':
   108                                  flags |= USB_QUIRK_DEVICE_QUALIFIER;
   109                                  break;
   110                          case 'j':
   111                                  flags |= USB_QUIRK_IGNORE_REMOTE_WAKEUP;
   112                                  break;
   113                          case 'k':
   114                                  flags |= USB_QUIRK_NO_LPM;
   115                                  break;
   116                          case 'l':
   117                                  flags |= 
USB_QUIRK_LINEAR_FRAME_INTR_BINTERVAL;
   118                                  break;
   119                          case 'm':
   120                                  flags |= USB_QUIRK_DISCONNECT_SUSPEND;
   121                                  break;
   122                          /* Ignore unrecognized flag characters */
   123                          }
   124                  }
   125  
   126                  quirk_list[i++] = (struct quirk_entry)
   127                          { .vid = vid, .pid = pid, .flags = flags };
   128          }
   129  
   130          if (i < quirk_count)
   131                  quirk_count = i;
   132  
   133  unlock:
   134          mutex_unlock(&quirk_mutex);
   135  
   136          return param_set_copystring(val, kp);
                                            ^^^
Patch adds new dereference (inside the function).

   137  }
   138  

regards,
dan carpenter


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to