Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 5 Jun 2020 07:57:16 +0000 (UTC)
From:      Hans Petter Selasky <hselasky@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r361827 - in head: lib/libusbhid sys/dev/usb
Message-ID:  <202006050757.0557vGS8098193@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: hselasky
Date: Fri Jun  5 07:57:16 2020
New Revision: 361827
URL: https://svnweb.freebsd.org/changeset/base/361827

Log:
  USB HID descriptors may push/pop the current state to allow
  description of items residing in a so-called union. FreeBSD currently
  only supports 4 such pop levels.
  
  If the push level is not restored within the processing of the same
  HID item, an invalid memory location may be used for subsequent HID
  item processing.
  
  Verify that the push level is always valid when processing HID items.
  
  Reported by:	Andy Nguyen (Google)
  MFC after:	3 days
  Sponsored by:	Mellanox Technologies

Modified:
  head/lib/libusbhid/parse.c
  head/sys/dev/usb/usb_hid.c

Modified: head/lib/libusbhid/parse.c
==============================================================================
--- head/lib/libusbhid/parse.c	Fri Jun  5 07:38:46 2020	(r361826)
+++ head/lib/libusbhid/parse.c	Fri Jun  5 07:57:16 2020	(r361827)
@@ -403,26 +403,28 @@ hid_get_item_raw(hid_data_t s, hid_item_t *h)
 				s->loc_count = dval & mask;
 				break;
 			case 10:	/* Push */
+				/* stop parsing, if invalid push level */
+				if ((s->pushlevel + 1) >= MAXPUSH)
+					return (0);
 				s->pushlevel ++;
-				if (s->pushlevel < MAXPUSH) {
-					s->cur[s->pushlevel] = *c;
-					/* store size and count */
-					c->report_size = s->loc_size;
-					c->report_count = s->loc_count;
-					/* update current item pointer */
-					c = &s->cur[s->pushlevel];
-				}
+				s->cur[s->pushlevel] = *c;
+				/* store size and count */
+				c->report_size = s->loc_size;
+				c->report_count = s->loc_count;
+				/* update current item pointer */
+				c = &s->cur[s->pushlevel];
 				break;
 			case 11:	/* Pop */
+				/* stop parsing, if invalid push level */
+				if (s->pushlevel == 0)
+					return (0);
 				s->pushlevel --;
-				if (s->pushlevel < MAXPUSH) {
-					c = &s->cur[s->pushlevel];
-					/* restore size and count */
-					s->loc_size = c->report_size;
-					s->loc_count = c->report_count;
-					c->report_size = 0;
-					c->report_count = 0;
-				}
+				c = &s->cur[s->pushlevel];
+				/* restore size and count */
+				s->loc_size = c->report_size;
+				s->loc_count = c->report_count;
+				c->report_size = 0;
+				c->report_count = 0;
 				break;
 			default:
 				break;

Modified: head/sys/dev/usb/usb_hid.c
==============================================================================
--- head/sys/dev/usb/usb_hid.c	Fri Jun  5 07:38:46 2020	(r361826)
+++ head/sys/dev/usb/usb_hid.c	Fri Jun  5 07:57:16 2020	(r361827)
@@ -436,36 +436,36 @@ hid_get_item(struct hid_data *s, struct hid_item *h)
 				s->loc_count = dval & mask;
 				break;
 			case 10:	/* Push */
-				s->pushlevel ++;
-				if (s->pushlevel < MAXPUSH) {
-					s->cur[s->pushlevel] = *c;
-					/* store size and count */
-					c->loc.size = s->loc_size;
-					c->loc.count = s->loc_count;
-					/* update current item pointer */
-					c = &s->cur[s->pushlevel];
-				} else {
-					DPRINTFN(0, "Cannot push "
-					    "item @ %d\n", s->pushlevel);
+				/* stop parsing, if invalid push level */
+				if ((s->pushlevel + 1) >= MAXPUSH) {
+					DPRINTFN(0, "Cannot push item @ %d\n", s->pushlevel);
+					return (0);
 				}
+				s->pushlevel ++;
+				s->cur[s->pushlevel] = *c;
+				/* store size and count */
+				c->loc.size = s->loc_size;
+				c->loc.count = s->loc_count;
+				/* update current item pointer */
+				c = &s->cur[s->pushlevel];
 				break;
 			case 11:	/* Pop */
-				s->pushlevel --;
-				if (s->pushlevel < MAXPUSH) {
-					/* preserve position */
-					oldpos = c->loc.pos;
-					c = &s->cur[s->pushlevel];
-					/* restore size and count */
-					s->loc_size = c->loc.size;
-					s->loc_count = c->loc.count;
-					/* set default item location */
-					c->loc.pos = oldpos;
-					c->loc.size = 0;
-					c->loc.count = 0;
-				} else {
-					DPRINTFN(0, "Cannot pop "
-					    "item @ %d\n", s->pushlevel);
+				/* stop parsing, if invalid push level */
+				if (s->pushlevel == 0) {
+					DPRINTFN(0, "Cannot pop item @ 0\n");
+					return (0);
 				}
+				s->pushlevel --;
+				/* preserve position */
+				oldpos = c->loc.pos;
+				c = &s->cur[s->pushlevel];
+				/* restore size and count */
+				s->loc_size = c->loc.size;
+				s->loc_count = c->loc.count;
+				/* set default item location */
+				c->loc.pos = oldpos;
+				c->loc.size = 0;
+				c->loc.count = 0;
 				break;
 			default:
 				DPRINTFN(0, "Global bTag=%d\n", bTag);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202006050757.0557vGS8098193>