From 4ba05bdd5380853e6fbb4e655385d17e47f8920c Mon Sep 17 00:00:00 2001
From: Sven Brauch <mail@svenbrauch.de>
Date: Sun, 19 Jul 2015 21:11:14 +0200
Subject: [PATCH] Fix data loss in cdc-acm

When moving data from a read urb to the tty, check
if all data actually fit in the tty's buffer. If not,
do not resubmit the urb but keep it around and retry
submitting the rest of its buffer later.
This effectively means that data will never be lost, at
the cost that the sending device cannot send more
data when the tty buffer is full.

Signed-off-by: Sven Brauch <mail@svenbrauch.de>
---
 drivers/usb/class/cdc-acm.c | 93 +++++++++++++++++++++++++++++++++++++++++----
 drivers/usb/class/cdc-acm.h |  3 ++
 2 files changed, 89 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index b30e742..3ffb80b 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -398,14 +398,68 @@ static int acm_submit_read_urbs(struct acm *acm, gfp_t mem_flags)
 	return 0;
 }
 
-static void acm_process_read_urb(struct acm *acm, struct urb *urb)
+static int acm_process_read_urb(struct acm *acm, struct urb *urb)
 {
+	int size;
+
 	if (!urb->actual_length)
-		return;
+		return 0;
 
-	tty_insert_flip_string(&acm->port, urb->transfer_buffer,
-			urb->actual_length);
+	size = tty_insert_flip_string(&acm->port, urb->transfer_buffer,
+		urb->actual_length);
 	tty_flip_buffer_push(&acm->port);
+	return size;
+}
+
+static bool acm_push_leftover_data_to_tty(struct acm *acm) {
+	int j, k;
+	int urb_index;
+	unsigned long remaining, offset;
+	int written;
+	struct urb *urb;
+	struct acm_rb *rb;
+	bool may_submit_new = true;
+
+	/* TODO: Does this locking mechanism make any sense? I don't know.
+	         Some sort of lock is certainly required. */
+	unsigned long flags;
+	spin_lock_irqsave(&acm->resubmit_lock, flags);
+
+	/* Loop over the queued read urbs, and submit as much data as possible to the tty. */
+	for (j = 0; j < ACM_NR; j++) {
+		urb_index = acm->held_urbs[j];
+		remaining = acm->remaining_data_in_read_urbs[urb_index];
+		if (remaining == 0)
+			continue;
+
+		urb = acm->read_urbs[urb_index];
+		rb = urb->context;
+		offset = urb->actual_length - remaining;
+		written = tty_insert_flip_string(&acm->port, (char*) (urb->transfer_buffer) + offset,
+			remaining);
+		tty_flip_buffer_push(&acm->port);
+		acm->remaining_data_in_read_urbs[urb_index] = remaining - written;
+		if (remaining == written) {
+			/* The urb's buffer was fully submitted to the tty, it can be passed
+			 * to the USB subsystem again. */
+			set_bit(rb->index, &acm->read_urbs_free);
+			acm_submit_read_urb(acm, rb->index, GFP_ATOMIC);
+			acm->num_held_urbs--;
+		}
+		else {
+			/* There is no point in continuing now, the buffer is full */
+			may_submit_new = false;
+			break;
+		}
+	}
+	/* Move the remaining queued urbs to the beginning of the queue */
+	for (k = 0; k < acm->num_held_urbs; k++) {
+		acm->held_urbs[k] = acm->held_urbs[k+j];
+	}
+
+	spin_unlock_irqrestore(&acm->resubmit_lock, flags);
+
+	return may_submit_new;
 }
 
 static void acm_read_bulk_callback(struct urb *urb)
@@ -414,6 +468,11 @@ static void acm_read_bulk_callback(struct urb *urb)
 	struct acm *acm = rb->instance;
 	unsigned long flags;
 	int status = urb->status;
+	int size;
+	bool may_resubmit, may_submit_new;
+
+	/* First look if any filled urbs are still in the queue */
+	may_submit_new = acm_push_leftover_data_to_tty(acm);
 
 	dev_vdbg(&acm->data->dev, "%s - urb %d, len %d\n", __func__,
 					rb->index, urb->actual_length);
@@ -433,20 +492,36 @@ static void acm_read_bulk_callback(struct urb *urb)
 
 	usb_mark_last_busy(acm->dev);
 
-	acm_process_read_urb(acm, urb);
+	/*
+	 * Only try to post data of this new urb if the queue is empty,
+	 * otherwise put it to the end of the queue.
+	 */
+	size = may_submit_new ? acm_process_read_urb(acm, urb) : 0;
+
+	may_resubmit = true;
 	/*
 	 * Unthrottle may run on another CPU which needs to see events
 	 * in the same order. Submission has an implict barrier
 	 */
 	smp_mb__before_atomic();
-	set_bit(rb->index, &acm->read_urbs_free);
+	if (size != urb->actual_length) {
+		/* do not resubmit the urb, but mark it as filled instead */
+		acm->remaining_data_in_read_urbs[rb->index] = urb->actual_length - size;
+		acm->held_urbs[acm->num_held_urbs++] = rb->index;
+		may_resubmit = false;
+	}
+	else {
+		set_bit(rb->index, &acm->read_urbs_free);
+	}
 
 	/* throttle device if requested by tty */
 	spin_lock_irqsave(&acm->read_lock, flags);
 	acm->throttled = acm->throttle_req;
 	if (!acm->throttled) {
 		spin_unlock_irqrestore(&acm->read_lock, flags);
-		acm_submit_read_urb(acm, rb->index, GFP_ATOMIC);
+		if ( may_resubmit ) {
+			acm_submit_read_urb(acm, rb->index, GFP_ATOMIC);
+		}
 	} else {
 		spin_unlock_irqrestore(&acm->read_lock, flags);
 	}
@@ -770,6 +845,8 @@ static void acm_tty_unthrottle(struct tty_struct *tty)
 	acm->throttle_req = 0;
 	spin_unlock_irq(&acm->read_lock);
 
+	acm_push_leftover_data_to_tty(acm);
+
 	if (was_throttled)
 		acm_submit_read_urbs(acm, GFP_KERNEL);
 }
@@ -1357,6 +1434,8 @@ made_compressed_probe:
 		struct acm_rb *rb = &(acm->read_buffers[i]);
 		struct urb *urb;
 
+		acm->remaining_data_in_read_urbs[i] = 0;
+
 		rb->base = usb_alloc_coherent(acm->dev, readsize, GFP_KERNEL,
 								&rb->dma);
 		if (!rb->base)
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index dd9af38..bf62731 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -93,6 +93,9 @@ struct acm {
 	struct acm_wb wb[ACM_NW];
 	unsigned long read_urbs_free;
 	struct urb *read_urbs[ACM_NR];
+	int held_urbs[ACM_NR+1], num_held_urbs; /* fixed-size queue for not-yet-processed read urbs */
+	unsigned long remaining_data_in_read_urbs[ACM_NR]; /* amount of bytes in each held urb */
+	spinlock_t resubmit_lock;
 	struct acm_rb read_buffers[ACM_NR];
 	int rx_buflimit;
 	int rx_endpoint;
-- 
2.7.0

