Re: [PATCH 3/3] xhci: Free the command allocated for setting LPM if we return early

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Apr 03, 2023 at 10:01:22AM +0200, Mirsad Goran Todorovac wrote:
> On 3.4.2023. 9:57, Mirsad Goran Todorovac wrote:
> > On 3.4.2023. 9:24, Greg KH wrote:
> > > On Mon, Apr 03, 2023 at 09:17:21AM +0200, Mirsad Goran Todorovac wrote:
> > > > Hi, Mathias!
> > > > 
> > > > On 30.3.2023. 16:30, Mathias Nyman wrote:
> > > > > The command allocated to set exit latency LPM values need to be freed in
> > > > > case the command is never queued. This would be the case if there is no
> > > > > change in exit latency values, or device is missing.
> > > > > 
> > > > > Reported-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
> > > > > Link: https://lore.kernel.org/linux-usb/24263902-c9b3-ce29-237b-1c3d6918f4fe@alu.unizg.hr
> > > > > Tested-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
> > > > > Fixes: 5c2a380a5aa8 ("xhci: Allocate separate command structures for each LPM command")
> > > > > Cc: <Stable@vger.kernel.org>
> > > > > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > > > > ---
> > > > >    drivers/usb/host/xhci.c | 1 +
> > > > >    1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > > > > index bdb6dd819a3b..6307bae9cddf 100644
> > > > > --- a/drivers/usb/host/xhci.c
> > > > > +++ b/drivers/usb/host/xhci.c
> > > > > @@ -4442,6 +4442,7 @@ static int __maybe_unused xhci_change_max_exit_latency(struct xhci_hcd *xhci,
> > > > >        if (!virt_dev || max_exit_latency == virt_dev->current_mel) {
> > > > >            spin_unlock_irqrestore(&xhci->lock, flags);
> > > > > +        xhci_free_command(xhci, command);
> > > > >            return 0;
> > > > >        }
> > > > 
> > > > There seems to be a problem with applying this patch with "git am", as it
> > > > gives the following:
> > > > 
> > > > commit ff9de97baa02cb9362b7cb81e95bc9be424cab89
> > > > Author: @ <@>
> > > > Date:   Mon Apr 3 08:42:33 2023 +0200
> > > > 
> > > >      The command allocated to set exit latency LPM values need to be freed in
> > > >      case the command is never queued. This would be the case if there is no
> > > >      change in exit latency values, or device is missing.
> > > > 
> > > >      Fixes: 5c2a380a5aa8 ("xhci: Allocate separate command structures for each LPM command")
> > > >      Cc: <Stable@vger.kernel.org>
> > > >      Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > > 
> > > This is already commit f6caea485555 ("xhci: Free the command allocated
> > > for setting LPM if we return early") in Linus's tree, do you not see it
> > > there?
> > > 
> > > And how exactly did you save the message to apply it with 'git am'?  It
> > > worked for me.
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > 
> > git am ../mathias-xhci.mail
> > 
> > mtodorov@domac:~/linux/kernel/linux_torvalds$ cat ../mathias-xhci.mail
> > From: Mathias Nyman @ 2023-03-27  9:50 UTC (permalink / raw)
> >    To: mirsad.todorovac, linux-usb, linux-kernel
> >    Cc: gregkh, ubuntu-devel-discuss, stern, arnd, Mathias Nyman, Stable
> > 
> > The command allocated to set exit latency LPM values need to be freed in
> > case the command is never queued. This would be the case if there is no
> > change in exit latency values, or device is missing.
> > 
> > Fixes: 5c2a380a5aa8 ("xhci: Allocate separate command structures for each LPM command")
> > Cc: <Stable@vger.kernel.org>
> > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>

That is very odd, your mail program is not getting the full mbox
information here at all.  Try downloading it from lore.kernel.org as a
raw message:
	https://lore.kernel.org/all/20230330143056.1390020-4-mathias.nyman@linux.intel.com/raw
and applying that?

> > ---
> >   drivers/usb/host/xhci.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > index bdb6dd819a3b..6307bae9cddf 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -4442,6 +4442,7 @@ static int __maybe_unused xhci_change_max_exit_latency(struct xhci_hcd *xhci,
> > 
> >          if (!virt_dev || max_exit_latency == virt_dev->current_mel) {
> >                  spin_unlock_irqrestore(&xhci->lock, flags);
> > +               xhci_free_command(xhci, command);
> >                  return 0;
> >          }
> > 
> > -- 
> > 2.25.1
> > 
> > Sorry, no commit f6caea485555 in the "git pull":
> > 
> > mtodorov@domac:~/linux/kernel/linux_torvalds$ git log --oneline | grep f6caea485555
> > mtodorov@domac:~/linux/kernel/linux_torvalds$ git log --oneline | head -10
> > 10de4cefccf7 memstick: fix memory leak if card device is never registered
> > feeedf59897c platform/x86: think-lmi: Clean up display of current_value on Thinkstation
> > 86cebdbfb8d2 platform/x86: think-lmi: Fix memory leaks when parsing ThinkStation WMI strings
> > ff9de97baa02 The command allocated to set exit latency LPM values need
> > to be freed in case the command is never queued. This would be the case
> > if there is no change in exit latency values, or device is missing.
> > 2ac6d07f1a81 platform/x86: think-lmi: Fix memory leak when showing current settings
> > 7e364e56293b Linux 6.3-rc5
> > 6ab608fe852b Merge tag 'for-6.3-rc4-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
> > f95b8ea79c47 Revert "venus: firmware: Correct non-pix start and end addresses"
> > a10ca0950afe Merge tag 'driver-core-6.3-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core
> > 95d0b9d89d78 Merge tag 'powerpc-6.3-4' of git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
> > You have mail in /var/mail/mtodorov
> > mtodorov@domac:~/linux/kernel/linux_torvalds$
> > 
> > I don't see it here either. But it is not critical (no security issue).
> > 
> > Have a nice day!
> 
> P.S.
> 
> Correction.
> 
> Yes, I found it here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f6caea4855553a8b99ba3ec23ecdb5ed8262f26c
> 
> "Notice: this object is not reachable from any branch."
> 
> I see Murphy's law in action :-)

Ah, sorry, no, my fault, it's in my usb.git tree and hasn't been sent to
Linus yet, that will happen later this week.  It is also in the
linux-next tree if you want to look there.

thanks,

greg k-h