HID: hiddev: fix potential Spectre v1

uref->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->usage_index is compared to
field->maxusage and then used as an index to dereference field->usage
array. The same thing happens to the cmd == HIDIOC{G,S}USAGES cases, where
uref->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->usage_index >= field->maxusage)
		goto inval;
	i = field->usage[uref->usage_index].collection_index;
	return i;

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

Cc: <stable@vger.kernel.org>
Signed-off-by: Breno Leitao <leitao@debian.org>
--

v2: Contemplate cmd == HIDIOC{G,S}USAGES case
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
CVE-2017-5753

(cherry picked from commit f11274396a538b31bc010f782e05c2ce3f804c13)
Signed-off-by: Juerg Haefliger <juergh@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>

diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index cf307bdc3d53918a884ed4ed102447da97ce89d9..89761551c15d6b952ce10f85dfb463859933e712 100644 (file)
--- 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: