Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1613453 > unrolled thread

[PATCH v3 0/4] cdc-acm: reassemble fragmented notifications

Started byTobias Herzog <t-herzog@gmx.de>
First post2017-03-30 22:20 +0200
Last post2017-03-31 11:40 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH v3 0/4] cdc-acm: reassemble fragmented notifications Tobias Herzog <t-herzog@gmx.de> - 2017-03-30 22:20 +0200
    [PATCH v3 1/4] cdc-acm: fix possible invalid access when processing notification Tobias Herzog <t-herzog@gmx.de> - 2017-03-30 22:20 +0200
      Re: [PATCH v3 1/4] cdc-acm: fix possible invalid access when  processing notification Oliver Neukum <oneukum@suse.com> - 2017-03-31 11:40 +0200

#1613453 — [PATCH v3 0/4] cdc-acm: reassemble fragmented notifications

FromTobias Herzog <t-herzog@gmx.de>
Date2017-03-30 22:20 +0200
Subject[PATCH v3 0/4] cdc-acm: reassemble fragmented notifications
Message-ID<tqOI1-3e5-9@gated-at.bofh.it>
USB devices may have very limitited endpoint packet sizes, so that
notifications can not be transferred within one single usb packet.
This patchset adds the ability to reassemble notifications that are
transmitted fragmented.


v3:
 * reordering patches (security issues first)
 * fixed possible alignment bug
 * allocate buffer with size=2^x
 * additional code comments + fixed typos in commit messages

v2:
 * reuse an allocated buffer for further notifications
 * fixed issues with endianess
 * check buffer allocation (kmalloc)
 * don't use hard coded size of notification-header
 * fixed typo + code structure (unneeded goto)

Tobias Herzog (4):
  cdc-acm: fix possible invalid access when processing notification
  cdc-acm: reassemble fragmented notifications
  cdc-acm: log message for serial state notification
  cdc-acm: remove unused element of struct acm

 drivers/usb/class/cdc-acm.c | 127 ++++++++++++++++++++++++++++++++------------
 drivers/usb/class/cdc-acm.h |   4 +-
 2 files changed, 97 insertions(+), 34 deletions(-)

-- 
2.1.4

[toc] | [next] | [standalone]


#1613456 — [PATCH v3 1/4] cdc-acm: fix possible invalid access when processing notification

FromTobias Herzog <t-herzog@gmx.de>
Date2017-03-30 22:20 +0200
Subject[PATCH v3 1/4] cdc-acm: fix possible invalid access when processing notification
Message-ID<tqOI2-3e5-25@gated-at.bofh.it>
In reply to#1613453
Notifications may only be 8 bytes long. Accessing the 9th and
10th byte of unimplemented/unknown notifications may be insecure.
Also check the length of known notifications before accessing anything
behind the 8th byte.

Signed-off-by: Tobias Herzog <t-herzog@gmx.de>
---
 drivers/usb/class/cdc-acm.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index e35b150..f554e2f 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -322,6 +322,12 @@ static void acm_ctrl_irq(struct urb *urb)
 		break;
 
 	case USB_CDC_NOTIFY_SERIAL_STATE:
+		if (le16_to_cpu(dr->wLength) != 2) {
+			dev_dbg(&acm->control->dev,
+				"%s - malformed serial state\n", __func__);
+			break;
+		}
+
 		newctrl = get_unaligned_le16(data);
 
 		if (!acm->clocal && (acm->ctrlin & ~newctrl & ACM_CTRL_DCD)) {
@@ -358,11 +364,10 @@ static void acm_ctrl_irq(struct urb *urb)
 
 	default:
 		dev_dbg(&acm->control->dev,
-			"%s - unknown notification %d received: index %d "
-			"len %d data0 %d data1 %d\n",
+			"%s - unknown notification %d received: index %d len %d\n",
 			__func__,
-			dr->bNotificationType, dr->wIndex,
-			dr->wLength, data[0], data[1]);
+			dr->bNotificationType, dr->wIndex, dr->wLength);
+
 		break;
 	}
 exit:
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]


#1613802 — Re: [PATCH v3 1/4] cdc-acm: fix possible invalid access when processing notification

FromOliver Neukum <oneukum@suse.com>
Date2017-03-31 11:40 +0200
SubjectRe: [PATCH v3 1/4] cdc-acm: fix possible invalid access when processing notification
Message-ID<tr1ce-2Nd-27@gated-at.bofh.it>
In reply to#1613456
Am Donnerstag, den 30.03.2017, 22:15 +0200 schrieb Tobias Herzog:
> Notifications may only be 8 bytes long. Accessing the 9th and
> 10th byte of unimplemented/unknown notifications may be insecure.
> Also check the length of known notifications before accessing anything
> behind the 8th byte.
> 
> Signed-off-by: Tobias Herzog <t-herzog@gmx.de>
Acked-by: Oliver Neukum <oneukum@suse.com>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web