Re: [Bugme-new] [Bug 8297] New: ldusb returns bad data on subsequent reads if no data is available.

Oliver Neukum <[email protected]>
Newsgroups gmane.linux.usb.devel
Message-ID <[email protected]>
> > Problem Description:
> > Using my Go!Temp temperature probe with the ldusb driver I decided to cat the
> > device file /dev/ldusb0 to see what it would return.  Bad idea, I was presented
> > with an endless spewing of crap.
> > 
> > After a little debugging it seems the problem is that in ld_usb_open
> > dev->interrupt_in_done is set to 0, and the URB is submitted.  When ld_usb_read
> > is called if no data is in the buffer it waits for ld_usb_interrupt_in_callback
> > to complete by checking if dev->interrupt_in_done has been set to 1.  That works
> > fine if you only read once after calling open however, if you happen to read
> > more than once dev->interrupt_in_done is never reset so you read from the empty
> > buffer even though no data has arrived.

Does this patch work?

	Regards
		Oliver
Signed-off-by: Oliver Neukum <[email protected]>
---------
--- a/drivers/usb/misc/ldusb.c	2007-04-04 09:33:16.000000000 +0200
+++ b/drivers/usb/misc/ldusb.c	2007-04-04 09:33:22.000000000 +0200
@@ -162,6 +162,8 @@
 	size_t			interrupt_in_endpoint_size;
 	int			interrupt_in_running;
 	int			interrupt_in_done;
+	int			buffer_overflow;
+	spinlock_t		rbsl;
 
 	char*			interrupt_out_buffer;
 	struct usb_endpoint_descriptor* interrupt_out_endpoint;
@@ -227,10 +229,12 @@
 		} else {
 			dbg_info(&dev->intf->dev, "%s: nonzero status received: %d\n",
 				 __FUNCTION__, urb->status);
+			spin_lock(&dev->rbsl);
 			goto resubmit; /* maybe we can recover */
 		}
 	}
 
+	spin_lock(&dev->rbsl);
 	if (urb->actual_length > 0) {
 		next_ring_head = (dev->ring_head+1) % ring_buffer_size;
 		if (next_ring_head != dev->ring_tail) {
@@ -245,17 +249,20 @@
 			dev_warn(&dev->intf->dev,
 				 "Ring buffer overflow, %d bytes dropped\n",
 				 urb->actual_length);
+			dev->buffer_overflow = 1;
 	}
 
 resubmit:
 	/* resubmit if we're still running */
-	if (dev->interrupt_in_running && dev->intf) {
+	if (dev->interrupt_in_running && !dev->buffer_overflow && dev->intf) {
 		retval = usb_submit_urb(dev->interrupt_in_urb, GFP_ATOMIC);
-		if (retval)
+		if (retval) {
 			dev_err(&dev->intf->dev,
 				"usb_submit_urb failed (%d)\n", retval);
+			dev->buffer_overflow = 1;
+		}
 	}
-
+	spin_unlock(&dev->rbsl);
 exit:
 	dev->interrupt_in_done = 1;
 	wake_up_interruptible(&dev->read_wait);
@@ -327,6 +334,7 @@
 	/* initialize in direction */
 	dev->ring_head = 0;
 	dev->ring_tail = 0;
+	dev->buffer_overflow = 0;
 	usb_fill_int_urb(dev->interrupt_in_urb,
 			 interface_to_usbdev(interface),
 			 usb_rcvintpipe(interface_to_usbdev(interface),
@@ -436,6 +444,7 @@
 	size_t *actual_buffer;
 	size_t bytes_to_read;
 	int retval = 0;
+	int rv;
 
 	dev = file->private_data;
 
@@ -457,7 +466,10 @@
 	}
 
 	/* wait for data */
+	spin_lock_irq(&dev->rbsl);
 	if (dev->ring_head == dev->ring_tail) {
+		dev->interrupt_in_done = 0;
+		spin_unlock_irq(&dev->rbsl);
 		if (file->f_flags & O_NONBLOCK) {
 			retval = -EAGAIN;
 			goto unlock_exit;
@@ -465,6 +477,8 @@
 		retval = wait_event_interruptible(dev->read_wait, dev->interrupt_in_done);
 		if (retval < 0)
 			goto unlock_exit;
+	} else {
+		spin_unlock_irq(&dev->rbsl);
 	}
 
 	/* actual_buffer contains actual_length + interrupt_in_buffer */
@@ -483,6 +497,17 @@
 
 	retval = bytes_to_read;
 
+	spin_lock_irq(&dev->rbsl);
+	if (dev->buffer_overflow) {
+		dev->buffer_overflow = 0;
+		spin_unlock_irq(&dev->rbsl);
+		rv = usb_submit_urb(dev->interrupt_in_urb, GFP_KERNEL);
+		if (rv < 0)
+			dev->buffer_overflow = 1;
+	} else {
+		spin_unlock_irq(&dev->rbsl);
+	}
+
 unlock_exit:
 	/* unlock the device */
 	up(&dev->sem);
@@ -632,6 +657,7 @@
 		goto exit;
 	}
 	init_MUTEX(&dev->sem);
+	spin_lock_init(&dev->rbsl);
 	dev->intf = intf;
 	init_waitqueue_head(&dev->read_wait);
 	init_waitqueue_head(&dev->write_wait);

-------------------------------------------------------------------------
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.