HID: hiddev: fix potential Spectre v1

leitao · Jiri Kosina · commit f11274396a53 · 2018-10-26T17:06:24.000+02:00
uref-&gt;usage_index can be indirectly controlled by userspace, hence leading
to a potential exploitation of the Spectre variant 1 vulnerability.

This field is used as an array index by the hiddev_ioctl_usage() function,
when 'cmd' is either HIDIOCGCOLLECTIONINDEX, HIDIOCGUSAGES or
HIDIOCSUSAGES.

For cmd == HIDIOCGCOLLECTIONINDEX case, uref-&gt;usage_index is compared to
field-&gt;maxusage and then used as an index to dereference field-&gt;usage
array. The same thing happens to the cmd == HIDIOC{G,S}USAGES cases, where
uref-&gt;usage_index is checked against an array maximum value and then it is
used as an index in an array.

This is a summary of the HIDIOCGCOLLECTIONINDEX case, which matches the
traditional Spectre V1 first load:

	copy_from_user(uref, user_arg, sizeof(*uref))
	if (uref-&gt;usage_index &gt;= field-&gt;maxusage)
		goto inval;
	i = field-&gt;usage[uref-&gt;usage_index].collection_index;
	return i;

This patch fixes this by sanitizing field uref-&gt;usage_index before using it
to index field-&gt;usage (HIDIOCGCOLLECTIONINDEX) or field-&gt;value in
HIDIOC{G,S}USAGES arrays, thus, avoiding speculation in the first load.

Cc: &lt;stable@vger.kernel.org&gt;
Signed-off-by: Breno Leitao &lt;leitao@debian.org&gt;
--

v2: Contemplate cmd == HIDIOC{G,S}USAGES case
Signed-off-by: Jiri Kosina &lt;jkosina@suse.cz&gt;
diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
@@ -512,14 +512,24 @@ static noinline int hiddev_ioctl_usage(struct hiddev *hiddev, unsigned int cmd,
 			if (cmd == HIDIOCGCOLLECTIONINDEX) {
 				if (uref->usage_index >= field->maxusage)
 					goto inval;
+				uref->usage_index =
+					array_index_nospec(uref->usage_index,
+							   field->maxusage);
 			} else if (uref->usage_index >= field->report_count)
 				goto inval;
 		}
 
-		if ((cmd == HIDIOCGUSAGES || cmd == HIDIOCSUSAGES) &&
-		    (uref_multi->num_values > HID_MAX_MULTI_USAGES ||
-		     uref->usage_index + uref_multi->num_values > field->report_count))
-			goto inval;
+		if (cmd == HIDIOCGUSAGES || cmd == HIDIOCSUSAGES) {
+			if (uref_multi->num_values > HID_MAX_MULTI_USAGES ||
+			    uref->usage_index + uref_multi->num_values >
+			    field->report_count)
+				goto inval;
+
+			uref->usage_index =
+				array_index_nospec(uref->usage_index,
+						   field->report_count -
+						   uref_multi->num_values);
+		}
 
 		switch (cmd) {
 		case HIDIOCGUSAGE:
