FAILED: patch "[PATCH] USB: gadget: f_hid: fix deadlock in f_hidg_write()" failed to apply to 4.9-stable tree

FAILED: patch "[PATCH] USB: gadget: f_hid: fix deadlock in f_hidg_write()" failed to apply to 4.9-stable tree

[gregkh]

The patch below does not apply to the 4.9-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 072684e8c58d17e853f8e8b9f6d9ce2e58d2b036 Mon Sep 17 00:00:00 2001
From: Radoslav Gerganov <rgerganov@vmware.com>
Date: Tue, 5 Mar 2019 10:10:34 +0000
Subject: [PATCH] USB: gadget: f_hid: fix deadlock in f_hidg_write()

In f_hidg_write() the write_spinlock is acquired before calling
usb_ep_queue() which causes a deadlock when dummy_hcd is being used.
This is because dummy_queue() callbacks into f_hidg_req_complete() which
tries to acquire the same spinlock. This is (part of) the backtrace when
the deadlock occurs:

  0xffffffffc06b1410 in f_hidg_req_complete
  0xffffffffc06a590a in usb_gadget_giveback_request
  0xffffffffc06cfff2 in dummy_queue
  0xffffffffc06a4b96 in usb_ep_queue
  0xffffffffc06b1eb6 in f_hidg_write
  0xffffffff8127730b in __vfs_write
  0xffffffff812774d1 in vfs_write
  0xffffffff81277725 in SYSC_write

Fix this by releasing the write_spinlock before calling usb_ep_queue()

Reviewed-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Tested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: stable@vger.kernel.org # 4.11+
Fixes: 749494b6bdbb ("usb: gadget: f_hid: fix: Move IN request allocation to set_alt()")
Signed-off-by: Radoslav Gerganov <rgerganov@vmware.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>

diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index 75b113a5b25c..f3816a5c861e 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -391,20 +391,20 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
 	req->complete = f_hidg_req_complete;
 	req->context  = hidg;
 
+	spin_unlock_irqrestore(&hidg->write_spinlock, flags);
+
 	status = usb_ep_queue(hidg->in_ep, req, GFP_ATOMIC);
 	if (status < 0) {
 		ERROR(hidg->func.config->cdev,
 			"usb_ep_queue error on int endpoint %zd\n", status);
-		goto release_write_pending_unlocked;
+		goto release_write_pending;
 	} else {
 		status = count;
 	}
-	spin_unlock_irqrestore(&hidg->write_spinlock, flags);
 
 	return status;
 release_write_pending:
 	spin_lock_irqsave(&hidg->write_spinlock, flags);
-release_write_pending_unlocked:
 	hidg->write_pending = 0;
 	spin_unlock_irqrestore(&hidg->write_spinlock, flags);
 

Re: FAILED: patch "[PATCH] USB: gadget: f_hid: fix deadlock in f_hidg_write()" failed to apply to 4.9-stable tree

[James Bottomley]

On Sat, 2019-03-30 at 20:57 +0100, gregkh@linuxfoundation.org wrote:
> The patch below does not apply to the 4.9-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git
> commit id to <stable@vger.kernel.org>.

[...]

> Cc: stable@vger.kernel.org # 4.11+
> Fixes: 749494b6bdbb ("usb: gadget: f_hid: fix: Move IN request
> allocation to set_alt()")

The cc: stable specifies 4.11+ because the commit it fixes went in in
the 4.11 merge window.  Is there something incorrect with the tagging
that makes you think it should apply to 4.9?

James

Re: FAILED: patch "[PATCH] USB: gadget: f_hid: fix deadlock in f_hidg_write()" failed to apply to 4.9-stable tree

[Greg KH]

On Sat, Mar 30, 2019 at 01:06:23PM -0700, James Bottomley wrote:
> On Sat, 2019-03-30 at 20:57 +0100, gregkh@linuxfoundation.org wrote:
> > The patch below does not apply to the 4.9-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git
> > commit id to <stable@vger.kernel.org>.
> 
> [...]
> 
> > Cc: stable@vger.kernel.org # 4.11+
> > Fixes: 749494b6bdbb ("usb: gadget: f_hid: fix: Move IN request
> > allocation to set_alt()")
> 
> The cc: stable specifies 4.11+ because the commit it fixes went in in
> the 4.11 merge window.  Is there something incorrect with the tagging
> that makes you think it should apply to 4.9?

Commit 749494b6bdbb originally showed up in 4.11, but it was then
backported to 4.10.2 and 4.9.90.  That is why I tried to apply it to
4.9.y and when it did not work, sent out the rejection notice.

thanks,

greg k-h

Re: FAILED: patch "[PATCH] USB: gadget: f_hid: fix deadlock in f_hidg_write()" failed to apply to 4.9-stable tree

[Sasha Levin]

On Sat, Mar 30, 2019 at 01:06:23PM -0700, James Bottomley wrote:
>On Sat, 2019-03-30 at 20:57 +0100, gregkh@linuxfoundation.org wrote:
>> The patch below does not apply to the 4.9-stable tree.
>> If someone wants it applied there, or to any other stable or longterm
>> tree, then please email the backport, including the original git
>> commit id to <stable@vger.kernel.org>.
>
>[...]
>
>> Cc: stable@vger.kernel.org # 4.11+
>> Fixes: 749494b6bdbb ("usb: gadget: f_hid: fix: Move IN request
>> allocation to set_alt()")
>
>The cc: stable specifies 4.11+ because the commit it fixes went in in
>the 4.11 merge window.  Is there something incorrect with the tagging
>that makes you think it should apply to 4.9?

The commit it fixes was tagged for stable and backported to kernels such
as 4.9.

--
Thanks,
Sasha

Re: FAILED: patch "[PATCH] USB: gadget: f_hid: fix deadlock in f_hidg_write()" failed to apply to 4.9-stable tree

[James Bottomley]

On Sat, 2019-03-30 at 21:23 +0100, Greg KH wrote:
> On Sat, Mar 30, 2019 at 01:06:23PM -0700, James Bottomley wrote:
> > On Sat, 2019-03-30 at 20:57 +0100, gregkh@linuxfoundation.org
> > wrote:
> > > The patch below does not apply to the 4.9-stable tree.
> > > If someone wants it applied there, or to any other stable or
> > > longterm
> > > tree, then please email the backport, including the original git
> > > commit id to <stable@vger.kernel.org>.
> > 
> > [...]
> > 
> > > Cc: stable@vger.kernel.org # 4.11+
> > > Fixes: 749494b6bdbb ("usb: gadget: f_hid: fix: Move IN request
> > > allocation to set_alt()")
> > 
> > The cc: stable specifies 4.11+ because the commit it fixes went in
> > in
> > the 4.11 merge window.  Is there something incorrect with the
> > tagging
> > that makes you think it should apply to 4.9?
> 
> Commit 749494b6bdbb originally showed up in 4.11, but it was then
> backported to 4.10.2 and 4.9.90.  That is why I tried to apply it to
> 4.9.y and when it did not work, sent out the rejection notice.

Turns out there's a missing patch:

commit 25cd9721c2b16ee0d775e36ec3af31f392003f80
Author: Krzysztof Opasiak <k.opasiak@samsung.com>
Date:   Tue Jan 31 18:12:31 2017 +0100

    usb: gadget: f_hid: fix: Don't access hidg->req without spinlock
held


It changes one of the match lines from

	status = usb_ep_queue(hidg->in_ep, hidg->req, GFP_ATOMIC);

to

	status = usb_ep_queue(hidg->in_ep, req, GFP_ATOMIC);

which is why you get the rejection.

James