Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> writes:
> On Fri, May 11, 2012 at 12:18:16AM +0300, Alexander Shishkin wrote:
>> Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> writes:
>>
>> > On Thu, May 10, 2012 at 04:25:45PM +0300, Dan Carpenter wrote:
>> >> Hello Alexander Shishkin,
>> >>
>> >> This is a semi-automatic email about new static checker warnings.
>> >>
>> >> The patch 62bb84ed0e4d: "usb: gadget: ci13xxx: convert to platform
>> >> device" from May 8, 2012, leads to the following Smatch complaint:
>> >>
>> >> drivers/usb/gadget/ci13xxx_pci.c:54 ci13xxx_pci_probe()
>> >> warn: variable dereferenced before check 'id' (see line 49)
>> >>
>> >> drivers/usb/gadget/ci13xxx_pci.c
>> >> 48 {
>> >> 49 struct ci13xxx_udc_driver *driver = (void *)id->driver_data;
>> >> ^^^^^^^^^^^^^^^^
>> >> New dereference.
>> >>
>> >> 50 struct platform_device *plat_ci;
>> >> 51 struct resource res[3];
>> >> 52 int retval = 0, nres = 2;
>> >> 53
>> >> 54 if (id == NULL)
>> >> ^^^^^^^^^^
>> >> Old check. My guess is the check isn't needed.
>> >
>> > Yes, that is correct, I've now removed that check.
>> >
>> > What does need to be fixed is the fact that id->driver_data could be
>> > null, and bad things can happen later in this function if that is true.
>>
>> Hmm, I must be missing something here. If id->driver_data is NULL,
>> platform_device_add_data() will set plat_ci's platform_data to NULL
>> (which it already is anyway) as well, and this is checked for in the
>> platform driver's probe. We don't dereference this "driver" pointer in
>> this function. What kind of bad things am I still missing here?
>
> You "obviously" do something with that data later right? I thought I
> originally found where that was dereferenced, but I can't seem to find
> it at the moment, it's in there somewhere...
>
> Oh yeah, in ci_udc_probe() you will error out, and I think somewhere
> else as well. Why not just catch that issue here, when it happens, as
> it can (think dynamic device ids added through sysfs by a user), so it
> should be caught as early as possible.
>
> You are right, it's not "bad things", but it's still sloppy and should
> be fixed up.
Ok, point taken. Sending a patch for this with the rest of the patchset
(v8 now).
Regards,
--
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
