| 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
| ||
|
Message-Id: <1307547519-29246-1-git-send-email-toby.gray@realvnc.com>
Date: Wed, 8 Jun 2011 16:38:39 +0100
From: Toby Gray <toby.gray@...lvnc.com>
To: Oliver Neukum <oliver@...kum.name>,
Greg Kroah-Hartman <gregkh@...e.de>
Cc: linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
Toby Gray <toby.gray@...lvnc.com>
Subject: [PATCH] USB: cdc-acm: Prevent loss of data when filling tty buffer
When sending large quantities of data through a CDC ACM channel it is possible
for data to be lost when attempting to copy the data to the tty buffer. This
occurs due to the return value from tty_insert_flip_string not being checked.
This patch adds checking for how many bytes have been inserted into the tty
buffer and places data which hasn't been inserted into a pending buffer list.
The pending buffer list is pushed to the tty buffer when the tty unthrottles
Signed-off-by: Toby Gray <toby.gray@...lvnc.com>
---
drivers/usb/class/cdc-acm.c | 95 ++++++++++++++++++++++++++++++++++++------
drivers/usb/class/cdc-acm.h | 6 ++-
2 files changed, 85 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 395a347..9f1d3ff 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -361,23 +361,65 @@ 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 void acm_process_read_buf(struct acm *acm, struct acm_rb *rb)
{
struct tty_struct *tty;
+ int copied;
- if (!urb->actual_length)
+ if (!rb->size)
return;
tty = tty_port_tty_get(&acm->port);
- if (!tty)
+ if (!tty) {
+ /* drop all data */
+ rb->size = 0;
return;
+ }
- tty_insert_flip_string(tty, urb->transfer_buffer, urb->actual_length);
+ copied = tty_insert_flip_string(tty, rb->head, rb->size);
tty_flip_buffer_push(tty);
+ rb->head += copied;
+ rb->size -= copied;
+
tty_kref_put(tty);
}
+static void acm_process_pending_read_bufs(struct acm *acm)
+{
+ unsigned long flags;
+ struct acm_rb *buf;
+
+ spin_lock_irqsave(&acm->read_lock, flags);
+ while (!list_empty(&acm->pending_read_bufs)) {
+ buf = list_entry(acm->pending_read_bufs.next,
+ struct acm_rb, list);
+ list_del(&buf->list);
+ spin_unlock_irqrestore(&acm->read_lock, flags);
+
+ acm_process_read_buf(acm, buf);
+
+ spin_lock_irqsave(&acm->read_lock, flags);
+ if (buf->size > 0) {
+ /* Managed to fill tty buffer again, so readd to
+ * the head before giving up. */
+ list_add(&buf->list, &acm->pending_read_bufs);
+ spin_unlock_irqrestore(&acm->read_lock, flags);
+ return;
+ }
+
+ set_bit(buf->index, &acm->read_urbs_free);
+ }
+ /* Submit any free urbs if not throttled */
+ if (!acm->throttled && !acm->susp_count) {
+ spin_unlock_irqrestore(&acm->read_lock, flags);
+ acm_submit_read_urbs(acm, GFP_KERNEL);
+ } else {
+ spin_unlock_irqrestore(&acm->read_lock, flags);
+ }
+ return;
+}
+
static void acm_read_bulk_callback(struct urb *urb)
{
struct acm_rb *rb = urb->context;
@@ -386,24 +428,52 @@ static void acm_read_bulk_callback(struct urb *urb)
dev_vdbg(&acm->data->dev, "%s - urb %d, len %d\n", __func__,
rb->index, urb->actual_length);
- set_bit(rb->index, &acm->read_urbs_free);
if (!acm->dev) {
+ set_bit(rb->index, &acm->read_urbs_free);
dev_dbg(&acm->data->dev, "%s - disconnected\n", __func__);
return;
}
usb_mark_last_busy(acm->dev);
if (urb->status) {
+ set_bit(rb->index, &acm->read_urbs_free);
dev_dbg(&acm->data->dev, "%s - non-zero urb status: %d\n",
__func__, urb->status);
return;
}
- acm_process_read_urb(acm, urb);
+ rb->head = urb->transfer_buffer;
+ rb->size = urb->actual_length;
- /* throttle device if requested by tty */
spin_lock_irqsave(&acm->read_lock, flags);
- acm->throttled = acm->throttle_req;
+
+ if (!list_empty(&acm->pending_read_bufs)) {
+ /* Data from previous reads is still pending, so
+ * add to the end of that. */
+ list_add_tail(&rb->list, &acm->pending_read_bufs);
+ } else {
+ spin_unlock_irqrestore(&acm->read_lock, flags);
+
+ acm_process_read_buf(acm, rb);
+
+ spin_lock_irqsave(&acm->read_lock, flags);
+ /* Bulk endpoints are recommended by the ACM specification
+ * to be used for hardware flow control. If bulk endpoints
+ * are being used then check that all data was passed to
+ * the tty.
+ */
+ if (!acm->is_int_ep && rb->size > 0) {
+ /* acm->throttled might not yet be true as throttling is
+ * detect by the ldisc handler, however it will be
+ * detected eventually and therefore unthrottled.
+ */
+ list_add_tail(&rb->list, &acm->pending_read_bufs);
+ } else {
+ set_bit(rb->index, &acm->read_urbs_free);
+ }
+ }
+
+ /* throttle device if requested by tty */
if (!acm->throttled && !acm->susp_count) {
spin_unlock_irqrestore(&acm->read_lock, flags);
acm_submit_read_urb(acm, rb->index, GFP_ATOMIC);
@@ -651,26 +721,22 @@ static void acm_tty_throttle(struct tty_struct *tty)
return;
spin_lock_irq(&acm->read_lock);
- acm->throttle_req = 1;
+ acm->throttled = 1;
spin_unlock_irq(&acm->read_lock);
}
static void acm_tty_unthrottle(struct tty_struct *tty)
{
struct acm *acm = tty->driver_data;
- unsigned int was_throttled;
if (!ACM_READY(acm))
return;
spin_lock_irq(&acm->read_lock);
- was_throttled = acm->throttled;
acm->throttled = 0;
- acm->throttle_req = 0;
spin_unlock_irq(&acm->read_lock);
- if (was_throttled)
- acm_submit_read_urbs(acm, GFP_KERNEL);
+ acm_process_pending_read_bufs(acm);
}
static int acm_tty_break_ctl(struct tty_struct *tty, int state)
@@ -1078,6 +1144,7 @@ made_compressed_probe:
spin_lock_init(&acm->read_lock);
mutex_init(&acm->mutex);
acm->rx_endpoint = usb_rcvbulkpipe(usb_dev, epread->bEndpointAddress);
+ INIT_LIST_HEAD(&acm->pending_read_bufs);
acm->is_int_ep = usb_endpoint_xfer_int(epread);
if (acm->is_int_ep)
acm->bInterval = epread->bInterval;
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca7937f..995333b 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -72,8 +72,10 @@ struct acm_wb {
};
struct acm_rb {
+ struct list_head list;
int size;
unsigned char *base;
+ unsigned char *head;
dma_addr_t dma;
int index;
struct acm *instance;
@@ -96,6 +98,7 @@ struct acm {
struct acm_rb read_buffers[ACM_NR];
int rx_buflimit;
int rx_endpoint;
+ struct list_head pending_read_bufs;
spinlock_t read_lock;
int write_used; /* number of non-empty write buffers */
int transmitting;
@@ -113,8 +116,7 @@ struct acm {
unsigned int susp_count; /* number of suspended interfaces */
unsigned int combined_interfaces:1; /* control and data collapsed */
unsigned int is_int_ep:1; /* interrupt endpoints contrary to spec used */
- unsigned int throttled:1; /* actually throttled */
- unsigned int throttle_req:1; /* throttle requested */
+ unsigned int throttled:1; /* tty is throttling */
u8 bInterval;
struct acm_wb *delayed_wb; /* write queued for a device about to be woken */
};
--
1.7.0.4
--
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/
Powered by blists - more mailing lists