On Fri, Sep 15, 2023 at 12:57:58AM +0000, Yuran Pereira wrote:
> Hello Alan,
>
> Thank you for the detailed explanation.
>
> Apologies for the delay replying.
> Please, feel free to submit the patch.
No need; Andy Shevchenko already submitted the same patch some time ago
and it has been merged.
Alan Stern
> ________________________________
> De: Alan Stern <[email protected]>
> Enviado: 9 de setembro de 2023 14:36
> Para: Yuran Pereira <[email protected]>; Andy Shevchenko <[email protected]>
> Cc: [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>
> Assunto: Re: [PATCH] USB: core: Fix a NULL pointer dereference
>
> On Sat, Sep 09, 2023 at 06:28:12AM +0000, Yuran Pereira wrote:
> > Hello Alan,
> >
> > Thank you for elucidating that.
> >
> > So, this bug is present on the mainline tree which is where syzkaller
> > found it. My patch was also based on the mainline tree.
> >
> > I just ran the same reproducer against a kernel compiled from the usb
> > tree, and, as you suggested, the test you mentioned does in fact,
> > prevent the bug from occurring.
> >
> > Please forgive my ignorance; I am a new contributor to the community.
> > But in this situation how should I proceed? Is there even a need to
> > submit a patch, or will the code currently present in the usb tree
> > eventually be reflected in the mainline?
>
> The first step is to find the difference between the mainline and USB
> trees that is responsible for this change in behavior. A quick check of
> the Git logs shows that the change was caused by commit d21fdd07cea4
> ("driver core: Return proper error code when dev_set_name() fails"),
> written by Andy Shevchenko. As a result of this commit, the code in
> device_add() now says:
>
> if (dev_name(dev))
> error = 0;
> /* subsystems can specify simple device enumeration */
> else if (dev->bus && dev->bus->dev_name)
> error = dev_set_name(dev, "%s%u", dev->bus->dev_name, dev->id);
> if (error)
> goto name_error;
>
> This obviously omits a final "else" clause; it should say:
>
> if (dev_name(dev))
> error = 0;
> /* subsystems can specify simple device enumeration */
> else if (dev->bus && dev->bus->dev_name)
> error = dev_set_name(dev, "%s%u", dev->bus->dev_name, dev->id);
> + else
> + error = -EINVAL;
> if (error)
> goto name_error;
>
> So to answer your questions: No, the code in the USB tree will not find
> its way into mainline. The opposite will happen: The mainline code will
> land in the USB tree. Which means that yes, there is a need to submit a
> patch. You can go ahead and write this up for submission, or I can
> submit it for you. Or you can check with Andy and see if he wants to
> fix the problem in a different way.
>
> Alan Stern