Re: [PATCH 1 of 1]: musb: fixed a potential NULL pointer dereference.

[Dr. H. Nikolaus Schaller]

Am 12.02.2014 um 17:20 schrieb Greg Kroah-Hartman:

> On Wed, Feb 12, 2014 at 04:35:12PM +0100, Dr. H. Nikolaus Schaller wrote:
>> 
>> Am 12.02.2014 um 15:29 schrieb Greg Kroah-Hartman:
>> 
>>> On Wed, Feb 12, 2014 at 11:08:22AM +0100, Dr. H. Nikolaus Schaller wrote:
>>>> fixed a potential NULL pointer dereference.
>>>> 
>>>>   Rationale:
>>>>   this is the only location in the musb driver where the
>>>>   otg->gadget pointer is dereferenced. Assuming that it
>>>>   is never NULL is not only potentially unsafe but was
>>>>   observed in the wild on a GTA04 (OMAP3/TPS65950 based
>>>>   board) when trying to boot a device tree based 3.14-rc2
>>>>   kernel with USB cable plugged in.
>>>> 
>>>>   DT boot appears to modify the order in which components
>>>>   (gadget driver) are loaded and linked and therefore
>>>>   an early musb interrupt triggers with a NULL gadget
>>>>   pointer ending in a kernel panic.
>>>> 
>>>>   Since a non-existing gadget can never be "active" we
>>>>   simply use a 0 value for musb->is_active.
>>>> 
>>>>   Signed-off-by: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>
>>> 
>>> Please don't attach patches, our tools don't like them at all.
>> 
>> Sorry. That was by accident. Here is the patch appended.
> 
> That's better, but we need a "clean" patch to be able to apply it for
> real :)

Ok, please take it as a "bug report" and documentation how a
workaround could look like - and not as a fix to be applied.

> 
>>> And shouldn't we fix the root problem here, not just gloss over the fact
>>> that this pointer is NULL at this point in time?  Fixing the real issue
>>> should be the correct solution, right?
>> 
>> Well,
>> I must admit that I have no idea what the root cause could be 
>> (because I neither know the musb nor the gadget subsystems)
>> or if it is intended and just something in the initialization
>> sequence has changed.
>> 
>> I have done one more test and it appears to be in a non-device-tree
>> 3.14-rc1 as well. We did not see it in a 3.12.7 kernel on the same hardware.
> 
> Ah, can you run 'git bisect' to track down the real problem here?  We
> don't like papering over problems like this, that doesn't help anyone
> out.

Well,I can't promise anything. Bisecting kernel panic problems means
compiling, flashing a SD cards, booting, looking at the result, telling git
the outcome and compiling a different commit.

I tried and failed immediately because the proposed bisects did
not even compile or boot on our hardware.

The problem appears to be that if we want to bisect e.g.

	(3.12.7 + private extensions) ... (3.14-rc1 + adapted private extensions)

that git bisect finds the common ancestor and presents some commit
in the middle. And that one does not "interpolate" between the required
private extensions but is some mainline version that we can't even
compile (first problem is with our defconfig not being available).

But we need these extensions to make the kernel run on our board and
are in the (uncompleted) middle of upstreaming them. So I think bisecting
works only on boards that are already completely supported by mainline.
Or at least at the "bisect good" commit.

So to me it appears that the faster approach would be if someone who
understands how musb interacts with the gadget setup/loading mechanism
helps to look into the code.

Hope we can find someone with more insights on the code than me,
Nikolaus Schaller

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/