[PATCH net] usb: cdc-wdm: fix reading stuck on device close

[PATCH net] usb: cdc-wdm: fix reading stuck on device close

[Sergey Ryazanov]

cdc-wdm tracks whether a response reading request is in-progress and
blocks the next request from being sent until the previous request is
completed. As soon as last user closes the cdc-wdm device file, the
driver cancels any ongoing requests, resets the pending response
counter, but leaves the response reading in-progress flag
(WDM_RESPONDING) untouched.

So if the user closes the device file during the response receive
request is being performed, no more data will be obtained from the
modem. The request will be cancelled, effectively preventing the
WDM_RESPONDING flag from being reseted. Keeping the flag set will
prevent a new response receive request from being sent, permanently
blocking the read path. The read path will staying blocked until the
module will be reloaded or till the modem will be re-attached.

This stuck has been observed with a Huawei E3372 modem attached to an
OpenWrt router and using the comgt utility to set up a network
connection.

Fix this issue by clearing the WDM_RESPONDING flag on the device file
close.

Without this fix, the device reading stuck can be easily reproduced in a
few connection establishing attempts. With this fix, a load test for
modem connection re-establishing worked for several hours without any
issues.

Fixes: 922a5eadd5a3 ("usb: cdc-wdm: Fix race between autosuspend and
reading from the device")
Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---

Technically, cdc-wdm belongs to the USB subsystem even though it serves
WWAN devices. I think this fix should be applied (backported) to LTS
versions too. So I targeted it to the 'net' tree, but send it to both
lists to get a feedback. Greg, Jakub, what is the best tree for this
fix?

---
 drivers/usb/class/cdc-wdm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 7f2c83f299d3..eebe782380fb 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -774,6 +774,7 @@ static int wdm_release(struct inode *inode, struct file *file)
 			poison_urbs(desc);
 			spin_lock_irq(&desc->iuspin);
 			desc->resp_count = 0;
+			clear_bit(WDM_RESPONDING, &desc->flags);
 			spin_unlock_irq(&desc->iuspin);
 			desc->manage_power(desc->intf, 0);
 			unpoison_urbs(desc);
-- 
2.35.1

Re: [PATCH net] usb: cdc-wdm: fix reading stuck on device close

[Greg Kroah-Hartman]

On Sun, May 01, 2022 at 08:58:28PM +0300, Sergey Ryazanov wrote:
> cdc-wdm tracks whether a response reading request is in-progress and
> blocks the next request from being sent until the previous request is
> completed. As soon as last user closes the cdc-wdm device file, the
> driver cancels any ongoing requests, resets the pending response
> counter, but leaves the response reading in-progress flag
> (WDM_RESPONDING) untouched.
> 
> So if the user closes the device file during the response receive
> request is being performed, no more data will be obtained from the
> modem. The request will be cancelled, effectively preventing the
> WDM_RESPONDING flag from being reseted. Keeping the flag set will
> prevent a new response receive request from being sent, permanently
> blocking the read path. The read path will staying blocked until the
> module will be reloaded or till the modem will be re-attached.
> 
> This stuck has been observed with a Huawei E3372 modem attached to an
> OpenWrt router and using the comgt utility to set up a network
> connection.
> 
> Fix this issue by clearing the WDM_RESPONDING flag on the device file
> close.
> 
> Without this fix, the device reading stuck can be easily reproduced in a
> few connection establishing attempts. With this fix, a load test for
> modem connection re-establishing worked for several hours without any
> issues.
> 
> Fixes: 922a5eadd5a3 ("usb: cdc-wdm: Fix race between autosuspend and
> reading from the device")

Nit, Fixes: lines should only be one line, I'll fix this up when
applying it.

> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
> ---
> 
> Technically, cdc-wdm belongs to the USB subsystem even though it serves
> WWAN devices. I think this fix should be applied (backported) to LTS
> versions too. So I targeted it to the 'net' tree, but send it to both
> lists to get a feedback. Greg, Jakub, what is the best tree for this
> fix?
> 
> ---
>  drivers/usb/class/cdc-wdm.c | 1 +
>  1 file changed, 1 insertion(+)

scripts/get_maintainer.pl is pretty clear this goes through the USB
tree.  I'll queue it up in a few days, thanks,

greg k-h

Re: [PATCH net] usb: cdc-wdm: fix reading stuck on device close

[Sergey Ryazanov]

On Sun, May 1, 2022 at 9:09 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Sun, May 01, 2022 at 08:58:28PM +0300, Sergey Ryazanov wrote:
>> cdc-wdm tracks whether a response reading request is in-progress and
>> blocks the next request from being sent until the previous request is
>> completed. As soon as last user closes the cdc-wdm device file, the
>> driver cancels any ongoing requests, resets the pending response
>> counter, but leaves the response reading in-progress flag
>> (WDM_RESPONDING) untouched.
>>
>> So if the user closes the device file during the response receive
>> request is being performed, no more data will be obtained from the
>> modem. The request will be cancelled, effectively preventing the
>> WDM_RESPONDING flag from being reseted. Keeping the flag set will
>> prevent a new response receive request from being sent, permanently
>> blocking the read path. The read path will staying blocked until the
>> module will be reloaded or till the modem will be re-attached.
>>
>> This stuck has been observed with a Huawei E3372 modem attached to an
>> OpenWrt router and using the comgt utility to set up a network
>> connection.
>>
>> Fix this issue by clearing the WDM_RESPONDING flag on the device file
>> close.
>>
>> Without this fix, the device reading stuck can be easily reproduced in a
>> few connection establishing attempts. With this fix, a load test for
>> modem connection re-establishing worked for several hours without any
>> issues.
>>
>> Fixes: 922a5eadd5a3 ("usb: cdc-wdm: Fix race between autosuspend and
>> reading from the device")
>
> Nit, Fixes: lines should only be one line, I'll fix this up when
> applying it.

Ouch. Sorry.

>> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
>> ---
>>
>> Technically, cdc-wdm belongs to the USB subsystem even though it serves
>> WWAN devices. I think this fix should be applied (backported) to LTS
>> versions too. So I targeted it to the 'net' tree, but send it to both
>> lists to get a feedback. Greg, Jakub, what is the best tree for this
>> fix?
>>
>> ---
>>  drivers/usb/class/cdc-wdm.c | 1 +
>>  1 file changed, 1 insertion(+)
>
> scripts/get_maintainer.pl is pretty clear this goes through the USB
> tree.  I'll queue it up in a few days, thanks,

Thank you for your clarification, will keep in mind.

-- 
Sergey

Re: [PATCH net] usb: cdc-wdm: fix reading stuck on device close

[Oliver Neukum]

On 01.05.22 19:58, Sergey Ryazanov wrote:
> cdc-wdm tracks whether a response reading request is in-progress and
> blocks the next request from being sent until the previous request is
> completed. As soon as last user closes the cdc-wdm device file, the
> driver cancels any ongoing requests, resets the pending response
> counter, but leaves the response reading in-progress flag
> (WDM_RESPONDING) untouched.
>
> So if the user closes the device file during the response receive
> request is being performed, no more data will be obtained from the
> modem. The request will be cancelled, effectively preventing the
> WDM_RESPONDING flag from being reseted. Keeping the flag set will
> prevent a new response receive request from being sent, permanently
> blocking the read path. The read path will staying blocked until the
> module will be reloaded or till the modem will be re-attached.
>
> This stuck has been observed with a Huawei E3372 modem attached to an
> OpenWrt router and using the comgt utility to set up a network
> connection.
>
> Fix this issue by clearing the WDM_RESPONDING flag on the device file
> close.
>
> Without this fix, the device reading stuck can be easily reproduced in a
> few connection establishing attempts. With this fix, a load test for
> modem connection re-establishing worked for several hours without any
> issues.
>
> Fixes: 922a5eadd5a3 ("usb: cdc-wdm: Fix race between autosuspend and
> reading from the device")
> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
>
Acked-by: Oliver Neukum <oneukum@suse.com>