| From 4f65245f2d178b9cba48350620d76faa4a098841 Mon Sep 17 00:00:00 2001 |
| From: "Gustavo A. R. Silva" <gustavo@embeddedor.com> |
| Date: Fri, 29 Jun 2018 17:08:44 -0500 |
| Subject: HID: hiddev: fix potential Spectre v1 |
| |
| From: Gustavo A. R. Silva <gustavo@embeddedor.com> |
| |
| commit 4f65245f2d178b9cba48350620d76faa4a098841 upstream. |
| |
| uref->field_index, uref->usage_index, finfo.field_index and cinfo.index can be |
| indirectly controlled by user-space, hence leading to a potential exploitation |
| of the Spectre variant 1 vulnerability. |
| |
| This issue was detected with the help of Smatch: |
| |
| drivers/hid/usbhid/hiddev.c:473 hiddev_ioctl_usage() warn: potential spectre issue 'report->field' (local cap) |
| drivers/hid/usbhid/hiddev.c:477 hiddev_ioctl_usage() warn: potential spectre issue 'field->usage' (local cap) |
| drivers/hid/usbhid/hiddev.c:757 hiddev_ioctl() warn: potential spectre issue 'report->field' (local cap) |
| drivers/hid/usbhid/hiddev.c:801 hiddev_ioctl() warn: potential spectre issue 'hid->collection' (local cap) |
| |
| Fix this by sanitizing such structure fields before using them to index |
| report->field, field->usage and hid->collection |
| |
| Notice that given that speculation windows are large, the policy is |
| to kill the speculation on the first load and not worry if it can be |
| completed with a dependent load/store [1]. |
| |
| [1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2 |
| |
| Cc: stable@vger.kernel.org |
| Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> |
| Signed-off-by: Jiri Kosina <jkosina@suse.cz> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/hid/usbhid/hiddev.c | 11 +++++++++++ |
| 1 file changed, 11 insertions(+) |
| |
| --- a/drivers/hid/usbhid/hiddev.c |
| +++ b/drivers/hid/usbhid/hiddev.c |
| @@ -35,6 +35,7 @@ |
| #include <linux/hiddev.h> |
| #include <linux/compat.h> |
| #include <linux/vmalloc.h> |
| +#include <linux/nospec.h> |
| #include "usbhid.h" |
| |
| #ifdef CONFIG_USB_DYNAMIC_MINORS |
| @@ -478,10 +479,14 @@ static noinline int hiddev_ioctl_usage(s |
| |
| if (uref->field_index >= report->maxfield) |
| goto inval; |
| + uref->field_index = array_index_nospec(uref->field_index, |
| + report->maxfield); |
| |
| field = report->field[uref->field_index]; |
| if (uref->usage_index >= field->maxusage) |
| goto inval; |
| + uref->usage_index = array_index_nospec(uref->usage_index, |
| + field->maxusage); |
| |
| uref->usage_code = field->usage[uref->usage_index].hid; |
| |
| @@ -508,6 +513,8 @@ static noinline int hiddev_ioctl_usage(s |
| |
| if (uref->field_index >= report->maxfield) |
| goto inval; |
| + uref->field_index = array_index_nospec(uref->field_index, |
| + report->maxfield); |
| |
| field = report->field[uref->field_index]; |
| |
| @@ -761,6 +768,8 @@ static long hiddev_ioctl(struct file *fi |
| |
| if (finfo.field_index >= report->maxfield) |
| break; |
| + finfo.field_index = array_index_nospec(finfo.field_index, |
| + report->maxfield); |
| |
| field = report->field[finfo.field_index]; |
| memset(&finfo, 0, sizeof(finfo)); |
| @@ -801,6 +810,8 @@ static long hiddev_ioctl(struct file *fi |
| |
| if (cinfo.index >= hid->maxcollection) |
| break; |
| + cinfo.index = array_index_nospec(cinfo.index, |
| + hid->maxcollection); |
| |
| cinfo.type = hid->collection[cinfo.index].type; |
| cinfo.usage = hid->collection[cinfo.index].usage; |