[patch]mos7720 race fixes

Hi,

this fixes

- race with accessing urb->status
- freeing memory while in use
- the write room issue that bit in cdc-acm - 2 bytes in one go != 1 byte each in two goes
- memory allocation with GFP_KERNEL while in interrupt

It's relative to the earlier patch.

	Regards
		Oliver

Signed-off-by: Oliver Neukum <[email protected]>
-----

--- a/drivers/usb/serial/mos7720.c	2007-03-13 14:11:20.000000000 +0100
+++ b/drivers/usb/serial/mos7720.c	2007-03-13 15:14:23.000000000 +0100
@@ -73,6 +73,8 @@
 	struct async_icount	icount;
 	struct usb_serial_port	*port;	/* loop back to the owner */
 	struct urb		*write_urb_pool[NUM_URBS];
+	char			busy[NUM_URBS];
+	spinlock_t		pool_lock;
 };
 
 /* This structure holds all of the individual serial device information */
@@ -253,6 +255,7 @@
 {
 	struct moschip_port *mos7720_port;
 	struct tty_struct *tty;
+	int i;
 
 	if (urb->status) {
 		dbg("nonzero write bulk status received:%d", urb->status);
@@ -269,6 +272,15 @@
 
 	tty = mos7720_port->port->tty;
 
+	spin_lock(&mos7720_port->pool_lock);
+	for (i = 0; i < NUM_URBS; i++) {
+		if (urb == mos7720_port->write_urb_pool[i]) {
+			mos7720_port->busy[i] = 0;
+			break;
+		}
+	}
+	spin_unlock(&mos7720_port->pool_lock);
+
 	if (tty && mos7720_port->open)
 		tty_wakeup(tty);
 }
@@ -533,7 +545,7 @@
 	}
 
 	for (i = 0; i < NUM_URBS; ++i) {
-		if (mos7720_port->write_urb_pool[i] && mos7720_port->write_urb_pool[i]->status == -EINPROGRESS)
+		if (mos7720_port->write_urb_pool[i] && mos7720_port->busy[i])
 			chars += URB_TRANSFER_BUFFER_SIZE;
 	}
 	dbg("%s - returns %d", __FUNCTION__, chars);
@@ -555,12 +567,10 @@
 	if (mos7720_port == NULL)
 		return;
 
-	for (j = 0; j < NUM_URBS; ++j)
-		usb_kill_urb(mos7720_port->write_urb_pool[j]);
-
 	/* Freeing Write URBs */
 	for (j = 0; j < NUM_URBS; ++j) {
 		if (mos7720_port->write_urb_pool[j]) {
+			usb_kill_urb(mos7720_port->write_urb_pool[j]);
 			kfree(mos7720_port->write_urb_pool[j]->transfer_buffer);
 			usb_free_urb(mos7720_port->write_urb_pool[j]);
 		}
@@ -636,12 +646,16 @@
 	}
 
 	for (i = 0; i < NUM_URBS; ++i) {
-		if (mos7720_port->write_urb_pool[i] && mos7720_port->write_urb_pool[i]->status != -EINPROGRESS)
+		if (mos7720_port->write_urb_pool[i] && !mos7720_port->busy[i])
 			room += URB_TRANSFER_BUFFER_SIZE;
 	}
 
 	dbg("%s - returns %d", __FUNCTION__, room);
-	return room;
+	/* the tty layer assumes arbitrary granularity here,
+	 * which bites if a CR-LF combination is sent in two callbacks
+	 * hence we are pessimistic
+	 */
+	return room == 0 ? 0 : room - URB_TRANSFER_BUFFER_SIZE + 1;
 }
 
 static int mos7720_write(struct usb_serial_port *port,
@@ -651,6 +665,7 @@
 	int i;
 	int bytes_sent = 0;
 	int transfer_size;
+	unsigned long flags;
 
 	struct moschip_port *mos7720_port;
 	struct usb_serial *serial;
@@ -670,8 +685,9 @@
 	/* try to find a free urb in the list */
 	urb = NULL;
 
+	spin_lock_irqsave(&mos7720_port->pool_lock, flags);
 	for (i = 0; i < NUM_URBS; ++i) {
-		if (mos7720_port->write_urb_pool[i] && mos7720_port->write_urb_pool[i]->status != -EINPROGRESS) {
+		if (mos7720_port->write_urb_pool[i] && !mos7720_port->busy[i]) {
 			urb = mos7720_port->write_urb_pool[i];
 			dbg("URB:%d",i);
 			break;
@@ -683,14 +699,6 @@
 		goto exit;
 	}
 
-	if (urb->transfer_buffer == NULL) {
-		urb->transfer_buffer = kmalloc(URB_TRANSFER_BUFFER_SIZE,
-					       GFP_KERNEL);
-		if (urb->transfer_buffer == NULL) {
-			err("%s no more kernel memory...", __FUNCTION__);
-			goto exit;
-		}
-	}
 	transfer_size = min (count, URB_TRANSFER_BUFFER_SIZE);
 
 	memcpy(urb->transfer_buffer, current_position, transfer_size);
@@ -712,9 +720,11 @@
 		bytes_sent = status;
 		goto exit;
 	}
+	
 	bytes_sent = transfer_size;
-
+	mos7720_port->busy[i] = 1;
 exit:
+	spin_unlock_irqrestore(&mos7720_port->pool_lock, flags);
 	return bytes_sent;
 }
 
@@ -1567,6 +1577,7 @@
 		serial->port[i]->interrupt_in_endpointAddress = serial->port[0]->interrupt_in_endpointAddress;
 
 		mos7720_port->port = serial->port[i];
+		spin_lock_init(&mos7720_port->pool_lock);
 		usb_set_serial_port_data(serial->port[i], mos7720_port);
 
 		dbg("port number is %d", serial->port[i]->number);

-- 
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)
This signature is a legal requirement

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel