Re: [PATCH V2] IB/core: Add mitigation for Spectre V1

["Gustavo A. R. Silva" <gustavo@embeddedor.com>]

On 7/31/19 9:52 AM, Doug Ledford wrote:

> 
> I'm not sure this is the best fix for this.  However, here is where I
> get to admit that I largely ignored the whole Spectre V1 thing, so I'm
> not sure I completely understand the vulnerability and the limits to
> that.  But, looking at the function, it seems we can do an early return
> without ever taking any of the mutexes in the function in the case of id
>> = IB_UMAD_MAX_AGENTS, so if we did that, would that separate the
> checking of id far enough from the usage of it as an array index that we
> wouldn't need the clamp to prevent speculative prefetch?  About how far,
> in code terms, does this speculative prefetch occur?
> 
> This is the patch I was thinking of:
> 

>  
> @@ -884,11 +885,18 @@ static int ib_umad_unreg_agent(struct ib_umad_file *file, u32 __user *arg)
>  
>         if (get_user(id, arg))
>                 return -EFAULT;
> +       if (id >= IB_UMAD_MAX_AGENTS)
> +               return -EINVAL;
>  
>         mutex_lock(&file->port->file_mutex);
>         mutex_lock(&file->mutex);
>  
> -       if (id >= IB_UMAD_MAX_AGENTS || !__get_agent(file, id)) {
> +       /*
> +        * Is our check of id far enough away, code wise, to prevent
> +        * speculative prefetch?
> +        */
> +       id = array_index_nospec(id, IB_UMAD_MAX_AGENTS);
> +       if (!__get_agent(file, id)) {
>                 ret = -EINVAL;
>                 goto out;
>         }
> 

This is insufficient. The speculation windows are large:

"Speculative  execution  on  modern  CPUs  can  run  several
hundred  instructions  ahead." [1]

[1] https://spectreattack.com/spectre.pdf

--
Gustavo



--
Gustavo