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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 31 Jul 2019 12:52:27 -0500
From:   "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To:     Doug Ledford <dledford@...hat.com>,
        "Luck, Tony" <tony.luck@...el.com>, Ira Weiny <ira.weiny@...el.com>
Cc:     Jason Gunthorpe <jgg@...pe.ca>, Leon Romanovsky <leon@...nel.org>,
        Parav Pandit <parav@...lanox.com>, linux-rdma@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] IB/core: Add mitigation for Spectre V1



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