From eff815491349029cf24182bfa58613d665b3123e Mon Sep 17 00:00:00 2001
From: Hans Petter Selasky <hselasky@FreeBSD.org>
Date: Fri, 5 Jun 2020 07:57:16 +0000
Subject: [PATCH] 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
---
 lib/libusbhid/parse.c | 34 +++++++++++++++-------------
 sys/dev/usb/usb_hid.c | 52 +++++++++++++++++++++----------------------
 2 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/lib/libusbhid/parse.c b/lib/libusbhid/parse.c
index 885c72fe33b0..2913de594200 100644
--- a/lib/libusbhid/parse.c
+++ b/lib/libusbhid/parse.c
@@ -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;
diff --git a/sys/dev/usb/usb_hid.c b/sys/dev/usb/usb_hid.c
index 6a33089588f5..1a042d2957f2 100644
--- a/sys/dev/usb/usb_hid.c
+++ b/sys/dev/usb/usb_hid.c
@@ -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);