Re: Patch "usb: dwc3: qcom: fix peripheral and OTG suspend" has been added to the 5.19-stable tree

Re: Patch "usb: dwc3: qcom: fix peripheral and OTG suspend" has been added to the 5.19-stable tree

[Greg Kroah-Hartman]

On Mon, Sep 05, 2022 at 09:36:07AM -0400, Sasha Levin wrote:
> On Mon, Sep 05, 2022 at 03:23:13PM +0200, Johan Hovold wrote:
> > On Mon, Sep 05, 2022 at 09:17:41AM -0400, Sasha Levin wrote:
> > > On Mon, Sep 05, 2022 at 03:13:09PM +0200, Johan Hovold wrote:
> > > >On Mon, Sep 05, 2022 at 09:04:44AM -0400, Sasha Levin wrote:
> > > >> On Mon, Sep 05, 2022 at 02:58:31PM +0200, Johan Hovold wrote:
> > > >> >On Mon, Sep 05, 2022 at 08:53:09AM -0400, Sasha Levin wrote:
> > > >> >>     Fixes: 6895ea55c385 ("usb: dwc3: qcom: Configure wakeup interrupts during suspend")
> > > >> >
> > > >> >This commit doesn't exist in 5.19 (and earlier trees), shouldn't your
> > > >> >scripts check for that?
> > > >>
> > > >> They do - it was backported to 5.19.
> > > >
> > > >What?! Why on earth would 6895ea55c385 ("usb: dwc3: qcom: Configure
> > > >wakeup interrupts during suspend") be backported to stable?
> > > >
> > > >Please drop that patch instead. It's essentially a new feature and is in
> > > >any case in no way stable material.
> > > 
> > > Right, it was picked up as a dependency of a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> > 
> > That's wrong too, it's not a dependency for that fix.
> 
> Right, it may not strictly be one, but we're trying to bring in
> dependencies as is without modifying the patch is it's far less error
> prone, and keeps future backports easy, as long as backporting those
> isn't riskier.
> 
> In this case, if I were to drop a872ab303d5d I'd also need to drop:
> 
> a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> 6498a96c8c9c ("usb: dwc3: qcom: fix runtime PM wakeup")
> 
> > So does this mean you're dropping the patches that should not be
> > backported?
> 
> Having said the above, at the end it's your patches and your call, let
> me know if you're okay with dropping a872ab303d5d, a872ab303d5d, and
> 6498a96c8c9c from all trees and I'll do that.

Yes, please drop all of those, they don't belong in stable kernels,
otherwise I would have added a cc: stable tag :)

thanks,

greg k-h

Re: Patch "usb: dwc3: qcom: fix peripheral and OTG suspend" has been added to the 5.19-stable tree

[Greg Kroah-Hartman]

On Mon, Sep 05, 2022 at 01:00:59PM -0400, Sasha Levin wrote:
> On Mon, Sep 05, 2022 at 05:00:10PM +0200, Johan Hovold wrote:
> > On Mon, Sep 05, 2022 at 09:36:07AM -0400, Sasha Levin wrote:
> > > On Mon, Sep 05, 2022 at 03:23:13PM +0200, Johan Hovold wrote:
> > > >On Mon, Sep 05, 2022 at 09:17:41AM -0400, Sasha Levin wrote:
> > > >> On Mon, Sep 05, 2022 at 03:13:09PM +0200, Johan Hovold wrote:
> > > >> >On Mon, Sep 05, 2022 at 09:04:44AM -0400, Sasha Levin wrote:
> > > >> >> On Mon, Sep 05, 2022 at 02:58:31PM +0200, Johan Hovold wrote:
> > > >> >> >On Mon, Sep 05, 2022 at 08:53:09AM -0400, Sasha Levin wrote:
> > > >> >> >>     Fixes: 6895ea55c385 ("usb: dwc3: qcom: Configure wakeup interrupts during suspend")
> > > >> >> >
> > > >> >> >This commit doesn't exist in 5.19 (and earlier trees), shouldn't your
> > > >> >> >scripts check for that?
> > > >> >>
> > > >> >> They do - it was backported to 5.19.
> > > >> >
> > > >> >What?! Why on earth would 6895ea55c385 ("usb: dwc3: qcom: Configure
> > > >> >wakeup interrupts during suspend") be backported to stable?
> > > >> >
> > > >> >Please drop that patch instead. It's essentially a new feature and is in
> > > >> >any case in no way stable material.
> > > >>
> > > >> Right, it was picked up as a dependency of a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> > > >
> > > >That's wrong too, it's not a dependency for that fix.
> > > 
> > > Right, it may not strictly be one, but we're trying to bring in
> > > dependencies as is without modifying the patch is it's far less error
> > > prone, and keeps future backports easy, as long as backporting those
> > > isn't riskier.
> > 
> > It should only be some context that have changed. Backporting a known
> > broken and non-trivial feature patch for that can't be right. It is
> > certainly riskier.
> > 
> > > In this case, if I were to drop a872ab303d5d I'd also need to drop:
> > > 
> > > a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> > > 6498a96c8c9c ("usb: dwc3: qcom: fix runtime PM wakeup")
> > > 
> > > >So does this mean you're dropping the patches that should not be
> > > >backported?
> > > 
> > > Having said the above, at the end it's your patches and your call, let
> > > me know if you're okay with dropping a872ab303d5d, a872ab303d5d, and
> > 
> > You mentioned a872ab303d5d twice here.
> > 
> > > 6498a96c8c9c from all trees and I'll do that.
> > 
> > This one didn't have a CC stable tag so not sure why you're backporting
> > that one either.
> > 
> > Just pick
> > 
> > 	a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> > 
> > which was the only patch I had marked for stable and fix up the trivial
> > context change (an unrelated function has been added after the new
> > helper in mainline).
> 
> Okay, this should be done. Please take a look at the queue to confirm.

At a quick glance, looks good to me.  I'll go through the other USB cc:
stable patches tomorrow and make sure.

thanks,

greg k-h

Re: Patch "usb: dwc3: qcom: fix peripheral and OTG suspend" has been added to the 5.19-stable tree

[Johan Hovold]

On Mon, Sep 05, 2022 at 01:00:59PM -0400, Sasha Levin wrote:
> On Mon, Sep 05, 2022 at 05:00:10PM +0200, Johan Hovold wrote:
> >On Mon, Sep 05, 2022 at 09:36:07AM -0400, Sasha Levin wrote:
> >> On Mon, Sep 05, 2022 at 03:23:13PM +0200, Johan Hovold wrote:
> >> >On Mon, Sep 05, 2022 at 09:17:41AM -0400, Sasha Levin wrote:
> >> >> On Mon, Sep 05, 2022 at 03:13:09PM +0200, Johan Hovold wrote:
> >> >> >On Mon, Sep 05, 2022 at 09:04:44AM -0400, Sasha Levin wrote:
> >> >> >> On Mon, Sep 05, 2022 at 02:58:31PM +0200, Johan Hovold wrote:
> >> >> >> >On Mon, Sep 05, 2022 at 08:53:09AM -0400, Sasha Levin wrote:
> >> >> >> >>     Fixes: 6895ea55c385 ("usb: dwc3: qcom: Configure wakeup interrupts during suspend")
> >> >> >> >
> >> >> >> >This commit doesn't exist in 5.19 (and earlier trees), shouldn't your
> >> >> >> >scripts check for that?
> >> >> >>
> >> >> >> They do - it was backported to 5.19.
> >> >> >
> >> >> >What?! Why on earth would 6895ea55c385 ("usb: dwc3: qcom: Configure
> >> >> >wakeup interrupts during suspend") be backported to stable?
> >> >> >
> >> >> >Please drop that patch instead. It's essentially a new feature and is in
> >> >> >any case in no way stable material.
> >> >>
> >> >> Right, it was picked up as a dependency of a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> >> >
> >> >That's wrong too, it's not a dependency for that fix.
> >>
> >> Right, it may not strictly be one, but we're trying to bring in
> >> dependencies as is without modifying the patch is it's far less error
> >> prone, and keeps future backports easy, as long as backporting those
> >> isn't riskier.
> >
> >It should only be some context that have changed. Backporting a known
> >broken and non-trivial feature patch for that can't be right. It is
> >certainly riskier.
> >
> >> In this case, if I were to drop a872ab303d5d I'd also need to drop:
> >>
> >> a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> >> 6498a96c8c9c ("usb: dwc3: qcom: fix runtime PM wakeup")
> >>
> >> >So does this mean you're dropping the patches that should not be
> >> >backported?
> >>
> >> Having said the above, at the end it's your patches and your call, let
> >> me know if you're okay with dropping a872ab303d5d, a872ab303d5d, and
> >
> >You mentioned a872ab303d5d twice here.
> >
> >> 6498a96c8c9c from all trees and I'll do that.
> >
> >This one didn't have a CC stable tag so not sure why you're backporting
> >that one either.
> >
> >Just pick
> >
> >	a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> >
> >which was the only patch I had marked for stable and fix up the trivial
> >context change (an unrelated function has been added after the new
> >helper in mainline).
> 
> Okay, this should be done. Please take a look at the queue to confirm.

I only checked the pending-5.19 branch, but that one still has 

	360e8230516d ("usb: dwc3: qcom: Add helper functions to enable,disable wake irqs")

which should be dropped as well.

Johan

Re: Patch "usb: dwc3: qcom: fix peripheral and OTG suspend" has been added to the 5.19-stable tree

[Greg Kroah-Hartman]

On Tue, Sep 06, 2022 at 07:48:22AM +0200, Johan Hovold wrote:
> On Mon, Sep 05, 2022 at 01:00:59PM -0400, Sasha Levin wrote:
> > On Mon, Sep 05, 2022 at 05:00:10PM +0200, Johan Hovold wrote:
> > >On Mon, Sep 05, 2022 at 09:36:07AM -0400, Sasha Levin wrote:
> > >> On Mon, Sep 05, 2022 at 03:23:13PM +0200, Johan Hovold wrote:
> > >> >On Mon, Sep 05, 2022 at 09:17:41AM -0400, Sasha Levin wrote:
> > >> >> On Mon, Sep 05, 2022 at 03:13:09PM +0200, Johan Hovold wrote:
> > >> >> >On Mon, Sep 05, 2022 at 09:04:44AM -0400, Sasha Levin wrote:
> > >> >> >> On Mon, Sep 05, 2022 at 02:58:31PM +0200, Johan Hovold wrote:
> > >> >> >> >On Mon, Sep 05, 2022 at 08:53:09AM -0400, Sasha Levin wrote:
> > >> >> >> >>     Fixes: 6895ea55c385 ("usb: dwc3: qcom: Configure wakeup interrupts during suspend")
> > >> >> >> >
> > >> >> >> >This commit doesn't exist in 5.19 (and earlier trees), shouldn't your
> > >> >> >> >scripts check for that?
> > >> >> >>
> > >> >> >> They do - it was backported to 5.19.
> > >> >> >
> > >> >> >What?! Why on earth would 6895ea55c385 ("usb: dwc3: qcom: Configure
> > >> >> >wakeup interrupts during suspend") be backported to stable?
> > >> >> >
> > >> >> >Please drop that patch instead. It's essentially a new feature and is in
> > >> >> >any case in no way stable material.
> > >> >>
> > >> >> Right, it was picked up as a dependency of a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> > >> >
> > >> >That's wrong too, it's not a dependency for that fix.
> > >>
> > >> Right, it may not strictly be one, but we're trying to bring in
> > >> dependencies as is without modifying the patch is it's far less error
> > >> prone, and keeps future backports easy, as long as backporting those
> > >> isn't riskier.
> > >
> > >It should only be some context that have changed. Backporting a known
> > >broken and non-trivial feature patch for that can't be right. It is
> > >certainly riskier.
> > >
> > >> In this case, if I were to drop a872ab303d5d I'd also need to drop:
> > >>
> > >> a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> > >> 6498a96c8c9c ("usb: dwc3: qcom: fix runtime PM wakeup")
> > >>
> > >> >So does this mean you're dropping the patches that should not be
> > >> >backported?
> > >>
> > >> Having said the above, at the end it's your patches and your call, let
> > >> me know if you're okay with dropping a872ab303d5d, a872ab303d5d, and
> > >
> > >You mentioned a872ab303d5d twice here.
> > >
> > >> 6498a96c8c9c from all trees and I'll do that.
> > >
> > >This one didn't have a CC stable tag so not sure why you're backporting
> > >that one either.
> > >
> > >Just pick
> > >
> > >	a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> > >
> > >which was the only patch I had marked for stable and fix up the trivial
> > >context change (an unrelated function has been added after the new
> > >helper in mainline).
> > 
> > Okay, this should be done. Please take a look at the queue to confirm.
> 
> I only checked the pending-5.19 branch, but that one still has 
> 
> 	360e8230516d ("usb: dwc3: qcom: Add helper functions to enable,disable wake irqs")
> 
> which should be dropped as well.

Now dropped.  It was needed for a different dwc3 patch, so I dropped
that too and will figure them out later today...

thanks,

greg k-h

Re: Patch "usb: dwc3: qcom: fix peripheral and OTG suspend" has been added to the 5.19-stable tree

[Johan Hovold]

On Tue, Sep 06, 2022 at 08:46:21AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Sep 06, 2022 at 07:48:22AM +0200, Johan Hovold wrote:
> > On Mon, Sep 05, 2022 at 01:00:59PM -0400, Sasha Levin wrote:
> > > On Mon, Sep 05, 2022 at 05:00:10PM +0200, Johan Hovold wrote:

> > > >Just pick
> > > >
> > > >	a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> > > >
> > > >which was the only patch I had marked for stable and fix up the trivial
> > > >context change (an unrelated function has been added after the new
> > > >helper in mainline).
> > > 
> > > Okay, this should be done. Please take a look at the queue to confirm.
> > 
> > I only checked the pending-5.19 branch, but that one still has 
> > 
> > 	360e8230516d ("usb: dwc3: qcom: Add helper functions to enable,disable wake irqs")
> > 
> > which should be dropped as well.
> 
> Now dropped.  It was needed for a different dwc3 patch, so I dropped
> that too and will figure them out later today...

I believe that wasn't a real dependency either and only changed the
context somewhat.

The only dwc3 fix that should be needed is the one mentioned above:

	a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")

which is self-contained, even if the context (as in unrelated functions
following the helper) may need to be adjusted.

Johan

Re: Patch "usb: dwc3: qcom: fix peripheral and OTG suspend" has been added to the 5.19-stable tree

[Greg Kroah-Hartman]

On Tue, Sep 06, 2022 at 09:01:04AM +0200, Johan Hovold wrote:
> On Tue, Sep 06, 2022 at 08:46:21AM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Sep 06, 2022 at 07:48:22AM +0200, Johan Hovold wrote:
> > > On Mon, Sep 05, 2022 at 01:00:59PM -0400, Sasha Levin wrote:
> > > > On Mon, Sep 05, 2022 at 05:00:10PM +0200, Johan Hovold wrote:
> 
> > > > >Just pick
> > > > >
> > > > >	a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> > > > >
> > > > >which was the only patch I had marked for stable and fix up the trivial
> > > > >context change (an unrelated function has been added after the new
> > > > >helper in mainline).
> > > > 
> > > > Okay, this should be done. Please take a look at the queue to confirm.
> > > 
> > > I only checked the pending-5.19 branch, but that one still has 
> > > 
> > > 	360e8230516d ("usb: dwc3: qcom: Add helper functions to enable,disable wake irqs")
> > > 
> > > which should be dropped as well.
> > 
> > Now dropped.  It was needed for a different dwc3 patch, so I dropped
> > that too and will figure them out later today...
> 
> I believe that wasn't a real dependency either and only changed the
> context somewhat.
> 
> The only dwc3 fix that should be needed is the one mentioned above:
> 
> 	a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> 
> which is self-contained, even if the context (as in unrelated functions
> following the helper) may need to be adjusted.

Ok, I've tried to apply the dwc3 patches that were tagged for stable
now, and you should have some FAILED emails showing what did not apply.

thanks,

greg k-h

Re: Patch "usb: dwc3: qcom: fix peripheral and OTG suspend" has been added to the 5.19-stable tree

[Johan Hovold]

On Tue, Sep 06, 2022 at 01:22:40PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Sep 06, 2022 at 09:01:04AM +0200, Johan Hovold wrote:

> > The only dwc3 fix that should be needed is the one mentioned above:
> > 
> > 	a872ab303d5d ("usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup")
> > 
> > which is self-contained, even if the context (as in unrelated functions
> > following the helper) may need to be adjusted.
> 
> Ok, I've tried to apply the dwc3 patches that were tagged for stable
> now, and you should have some FAILED emails showing what did not apply.

Yeah, there are some trivial conflicts due to changes in context. I've
just sent you backports to 5.15, which will hopefully apply to older
trees as well.

Johan

Re: Patch "usb: dwc3: qcom: fix peripheral and OTG suspend" has been added to the 5.19-stable tree

[Johan Hovold]

On Sat, Sep 24, 2022 at 11:14:57AM +0200, Greg Kroah-Hartman wrote:
> On Sat, Sep 24, 2022 at 10:56:43AM +0200, Johan Hovold wrote:

> > Just drop all the dwc3 patches from the pending-5.19 branch.
> 
> Now dropped, except for the one I added yesterday :)

Thanks. Didn't see the DMA one at first.

Johan

Re: Patch "usb: dwc3: qcom: fix peripheral and OTG suspend" has been added to the 5.19-stable tree

[Greg Kroah-Hartman]

On Sat, Sep 24, 2022 at 10:56:43AM +0200, Johan Hovold wrote:
> FFS, Sasha...
> 
> On Fri, Sep 23, 2022 at 12:55:34PM -0400, Sasha Levin wrote:
> > This is a note to let you know that I've just added the patch titled
> > 
> >     usb: dwc3: qcom: fix peripheral and OTG suspend
> > 
> > to the 5.19-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > 
> > The filename of the patch is:
> >      usb-dwc3-qcom-fix-peripheral-and-otg-suspend.patch
> > and it can be found in the queue-5.19 subdirectory.
> > 
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@vger.kernel.org> know about it.
> 
> Stop pulling in these dwc3 patches! I already spent an afternoon doing
> backports for all the stable trees so that you wouldn't pull in half the
> world as dependencies.
> 
> This fixes an issue with a commit in 6.0, which should not be backported
> in the first place.
> 
> Just as I told you two weeks ago when these were dropped the last time:
> 
> 	https://lore.kernel.org/lkml/YxbfJqPb8x0WjmqR@hovoldconsulting.com/
> 
> If you can't fix up the trivial context change yourself, then use one of
> the backports of:
> 
> 	https://lore.kernel.org/lkml/20220906120702.19219-3-johan@kernel.org/
> 
> or simply just leave it out as the issue can't be triggered in 5.19
> currently.
> 
> Just drop all the dwc3 patches from the pending-5.19 branch.

Now dropped, except for the one I added yesterday :)

thanks,

greg k-h

Re: Patch "usb: dwc3: qcom: fix peripheral and OTG suspend" has been added to the 5.19-stable tree

[Johan Hovold]

FFS, Sasha...

On Fri, Sep 23, 2022 at 12:55:34PM -0400, Sasha Levin wrote:
> This is a note to let you know that I've just added the patch titled
> 
>     usb: dwc3: qcom: fix peripheral and OTG suspend
> 
> to the 5.19-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> 
> The filename of the patch is:
>      usb-dwc3-qcom-fix-peripheral-and-otg-suspend.patch
> and it can be found in the queue-5.19 subdirectory.
> 
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.

Stop pulling in these dwc3 patches! I already spent an afternoon doing
backports for all the stable trees so that you wouldn't pull in half the
world as dependencies.

This fixes an issue with a commit in 6.0, which should not be backported
in the first place.

Just as I told you two weeks ago when these were dropped the last time:

	https://lore.kernel.org/lkml/YxbfJqPb8x0WjmqR@hovoldconsulting.com/

If you can't fix up the trivial context change yourself, then use one of
the backports of:

	https://lore.kernel.org/lkml/20220906120702.19219-3-johan@kernel.org/

or simply just leave it out as the issue can't be triggered in 5.19
currently.

Just drop all the dwc3 patches from the pending-5.19 branch.

> commit a989a972abf4ae3f79bbff2d9dc9956eda25e2f0
> Author: Johan Hovold <johan+linaro@kernel.org>
> Date:   Thu Aug 4 17:09:58 2022 +0200
> 
>     usb: dwc3: qcom: fix peripheral and OTG suspend
>     
>     [ Upstream commit c5f14abeb52b0177b940fd734133d383da3521d8 ]
>     
>     A recent commit implementing wakeup support in host mode instead broke
>     suspend for peripheral and OTG mode.
>     
>     The hack that was added in the suspend path to determine the speed of
>     any device connected to the USB2 bus not only accesses internal driver
>     data for a child device, but also dereferences a NULL pointer or
>     accesses freed data when the controller is not acting as host.
>     
>     There's no quick fix to the layering violation, but since reverting
>     would leave us with broken suspend in host mode with wakeup triggering
>     immediately, let's keep the hack for now.
>     
>     Fix the immediate issues by only checking the host bus speed and
>     enabling wakeup interrupts when acting as host.
>     
>     Fixes: 6895ea55c385 ("usb: dwc3: qcom: Configure wakeup interrupts during suspend")
>     Reported-by: kernel test robot <lkp@intel.com>
>     Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>     Link: https://lore.kernel.org/r/20220804151001.23612-7-johan+linaro@kernel.org
>     Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>     Signed-off-by: Sasha Levin <sashal@kernel.org>

Johan