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
