Hi Breno,
On 10/17/18 9:47 PM, 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 problem might show up in the cmd = HIDIOCGCOLLECTIONINDEX flow at function
> hiddev_ioctl_usage(), where uref->usage_index is compared to field->maxusage
> and then used as an index to dereference field->usage array.
>
> This is a summary of the current flow, which matches the traditional
> Spectre V1 issue:
>
> 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, thus, avoiding speculation in the first load.
>
> Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx>
> ---
> drivers/hid/usbhid/hiddev.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> index 23872d08308c..8829cbc1f6b1 100644
> --- a/drivers/hid/usbhid/hiddev.c
> +++ b/drivers/hid/usbhid/hiddev.c
> @@ -512,6 +512,9 @@ 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);
Good catch.
> } else if (uref->usage_index >= field->report_count)
> goto inval;
Although, notice that this is the same index, and it can be used to index field->value[]
at lines 526 and 532.
Thanks
--
Gustavo
