Re: [bug report] usb: core: Add "quirks" parameter for usbcore

2018-03-23 Thread Dan Carpenter 
On Fri, Mar 23, 2018 at 02:16:23PM +0800, Kai-Heng Feng wrote:
> Hi Dan,
> 
> Dan Carpenter  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`"
> 

You need to have the cross function database built.  It takes forever.
smatch_scripts/build_kernel_data.sh

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

Smatch tries not to warn about these if it's superfluous, but here it
thinks that val can be NULL from parse_one().  It's non-NULL from the
other two callers.

 kernel/params.c |parse_one | (struct kernel_param_ops)->set |  
PARAM_VALUE |  0 | val | 0,4096-2117778
 kernel/params.c | param_attr_store | (struct kernel_param_ops)->set |  
PARAM_VALUE |  0 | val | 5-5
 kernel/params.c |  param_array |  param_array param 7 |  
PARAM_VALUE |  0 | val | 4096-211

Anwyway, we should probably remove the NULL check just for consistency.

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

Re: [bug report] usb: core: Add "quirks" parameter for usbcore

2018-03-23 Thread Kai-Heng Feng 

Hi Dan,

Dan Carpenter  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(_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(, ":");
63  if (!field)
64  break;
65  
66  if (kstrtou16(field, 16, ))
67  break;
68  
69  field = strsep(, ":");
70  if (!field)
71  break;
72  
73  if (kstrtou16(field, 16, ))
74  break;
75  
76  field = strsep(, ",");
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

[bug report] usb: core: Add "quirks" parameter for usbcore

2018-03-22 Thread Dan Carpenter 
Hello Kai-Heng Feng,

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

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(_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(, ":");
63  if (!field)
64  break;
65  
66  if (kstrtou16(field, 16, ))
67  break;
68  
69  field = strsep(, ":");
70  if (!field)
71  break;
72  
73  if (kstrtou16(field, 16, ))
74  break;
75  
76  field = strsep(, ",");
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(_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