3.16.63-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Breno Leitao leitao@debian.org
commit f11274396a538b31bc010f782e05c2ce3f804c13 upstream.
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.
Signed-off-by: Breno Leitao leitao@debian.org --
v2: Contemplate cmd == HIDIOC{G,S}USAGES case Signed-off-by: Jiri Kosina jkosina@suse.cz Signed-off-by: Ben Hutchings ben@decadent.org.uk --- drivers/hid/usbhid/hiddev.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
--- a/drivers/hid/usbhid/hiddev.c +++ b/drivers/hid/usbhid/hiddev.c @@ -521,14 +521,24 @@ static noinline int hiddev_ioctl_usage(s 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: