| Message ID | 1539979293-16102-1-git-send-email-leitao@debian.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] HID: hiddev: fix potential Spectre v1 | expand |
On Fri, 19 Oct 2018, Breno Leitao wrote: > 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> > Cc: <stable@vger.kernel.org> Applied, thanks.
diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c index 23872d08308c..a746017fac17 100644 --- 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:
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> Cc: <stable@vger.kernel.org> -- v2: Contemplate cmd == HIDIOC{G,S}USAGES case