linux-kernel - [PATCH] usb_wwan: some improvement on write and resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 25 Apr 2014 08:57:15 +0800
From:	xiao jin <jin.xiao@...el.com>
To:	jhovold@...il.com, gregkh@...uxfoundation.org,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:	juan.zou@...el.com, yanmin.zhang@...el.com,
	david.a.cohen@...ux.intel.com, xiao jin <jin.xiao@...el.com>,
	"Zhang, Qi1" <qi1.zhang@...el.com>
Subject: [PATCH] usb_wwan: some improvement on write and resume

When enable usb serial for modem data, sometimes the tty is blocked
in tty_wait_until_sent because portdata->out_busy always is set and
have no chance to be cleared.

We have found two scenarios lead to portdata->out_busy problem.
1. usb_wwan_write set portdata->out_busy firstly, then try autopm
async with error. No out urb submit and no usb_wwan_outdat_callback
to this write, portdata->out_busy can't be cleared.
2. usb_wwan_resume run play_delayed() and spin_unlock, but
intfdata->suspended still is not set to zero. At this time
usb_wwan_write is called and anchor the urb to delay list. Then resume
keep running but the delayed urb have no chance to be commit until next
resume. If the time of next resume is far away, tty will be blocked
in tty_wait_until_sent during time.

The patch make some enhancement on write and resume.
1. clear portdata->out_busy if usb_wwan_write try autopm async with
error.
2. put play_Delayed and intfdata->suspended together in the spinlock,
it's to avoid the write race during resume.

Signed-off-by: xiao jin <jin.xiao@...el.com>
Signed-off-by: Zhang, Qi1 <qi1.zhang@...el.com>
Reviewed-by: David Cohen <david.a.cohen@...ux.intel.com>
---
 drivers/usb/serial/usb_wwan.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
index 640fe01..8e06afc 100644
--- a/drivers/usb/serial/usb_wwan.c
+++ b/drivers/usb/serial/usb_wwan.c
@@ -228,8 +228,10 @@ int usb_wwan_write(struct tty_struct *tty, struct usb_serial_port *port,
 			usb_pipeendpoint(this_urb->pipe), i);

 		err = usb_autopm_get_interface_async(port->serial->interface);
-		if (err < 0)
+		if (err < 0) {
+			clear_bit(i, &portdata->out_busy);
 			break;
+		}

 		/* send the data */
 		memcpy(this_urb->transfer_buffer, buf, todo);
@@ -661,17 +663,15 @@ int usb_wwan_resume(struct usb_serial *serial)
 		}
 	}

+	spin_lock_irq(&intfdata->susp_lock);
 	for (i = 0; i < serial->num_ports; i++) {
 		/* walk all ports */
 		port = serial->port[i];
 		portdata = usb_get_serial_port_data(port);

 		/* skip closed ports */
-		spin_lock_irq(&intfdata->susp_lock);
-		if (!portdata || !portdata->opened) {
-			spin_unlock_irq(&intfdata->susp_lock);
+		if (!portdata || !portdata->opened)
 			continue;
-		}

 		for (j = 0; j < N_IN_URB; j++) {
 			urb = portdata->in_urbs[j];
@@ -684,9 +684,7 @@ int usb_wwan_resume(struct usb_serial *serial)
 			}
 		}
 		play_delayed(port);
-		spin_unlock_irq(&intfdata->susp_lock);
 	}
-	spin_lock_irq(&intfdata->susp_lock);
 	intfdata->suspended = 0;
 	spin_unlock_irq(&intfdata->susp_lock);
 err_out:
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/