[PATCH 1/2] i386: Generate lfence with load/indirect branch/ret [CVE-2020-0551]

[Jan Beulich]

On 22.04.2020 05:33, Hongtao Liu wrote:
> On Tue, Apr 21, 2020 at 2:30 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 21.04.2020 04:24, Hongtao Liu wrote:
>>> On Mon, Apr 20, 2020 at 3:34 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 20.04.2020 09:20, Hongtao Liu wrote:
>>>>> On Thu, Apr 16, 2020 at 4:33 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>> On 16.04.2020 07:34, Hongtao Liu wrote:
>>>>>>> @@ -4506,6 +4520,22 @@ insert_lfence_after (void)
>>>>>>> {
>>>>>>>   if (lfence_after_load && load_insn_p ())
>>>>>>>     {
>>>>>>> +      /* Insert lfence after rep cmps/scas only under
>>>>>>> +       -mlfence-after-load=all.  */
>>>>>>> +      if (((i.tm.base_opcode | 0x1) == 0xa7
>>>>>>> +         || (i.tm.base_opcode | 0x1) == 0xaf)
>>>>>>> +        && i.prefix[REP_PREFIX])
>>>>>>
>>>>>> I'm afraid I don't understand why the REP forms need treating
>>>>>> differently from the non-REP ones of the same insns.
>>>>>>
>>>>>
>>>>> Not all REP forms, just REP CMPS/SCAS which would change EFLAGS.
>>>>
>>>> Well, of course just the two. But this doesn't answer my question
>>>> as to why there is such a special case.
>>>>
>>>
>>> There are also two REP string instructions that require special
>>> treatment. Specifically, the compare string (CMPS) and scan string
>>> (SCAS) instructions set EFLAGS in a manner that depends on the data
>>> being compared/scanned. When used with a REP prefix, the number of
>>> iterations may therefore vary depending on this data. If the data is a
>>> program secret chosen by the adversary using an LVI method, then this
>>> data-dependent behavior may leak some aspect of the secret. The
>>> solution is to unfold any REP CMPS and REP SCAS operations into a loop
>>> and insert an LFENCE after the CMPS/SCAS instruction. For example,
>>> REPNZ SCAS can be unfolded to:
>>>
>>> .RepLoop:
>>>   JRCXZ .ExitRepLoop
>>>   DEC rcx  # or ecx if the REPNZ SCAS uses a 32-bit address size
>>>   SCAS
>>>   LFENCE
>>>   JNZ .RepLoop
>>> .ExitRepLoop:
>>>   ...
>>>
>>> The request i get is to add options to handle or not handle REP
>>> CMPS/SCAS also plus issue a warning.
>>
>> But you don't handle them as per what you've written above, afaics.
>> Am I overlooking anything?
> 
> Well, that solution is not meant for gas, i put them here for
> convienence of understanding of why we need to handle REP CMPS/SCAS
> specially.

And how is it better then to issue a warning and leave the code
alone over still at least inserting an LFENCE after the insn? I.e.
I'm not sure I see the value of the separate "general" and "all"
sub-options then.

As to it not being meant for gas - why is that?

> @@ -4568,12 +4609,13 @@ insert_lfence_before (void)
>        return;
>      }
> 
> -  /* Output or/not and lfence before ret.  */
> +  /* Output or/not/shl and lfence before ret/lret/iret.  */
>    if (lfence_before_ret != lfence_before_ret_none
>        && (i.tm.base_opcode == 0xc2
>            || i.tm.base_opcode == 0xc3
>            || i.tm.base_opcode == 0xca
> -          || i.tm.base_opcode == 0xcb))
> +          || i.tm.base_opcode == 0xcb
> +          || i.tm.base_opcode == 0xcf))
>      {
>        if (last_insn.kind != last_insn_other
>            && last_insn.seg == now_seg)
> @@ -4583,33 +4625,59 @@ insert_lfence_before (void)
>                           last_insn.name, i.tm.name);
>            return;
>          }
> -      if (lfence_before_ret == lfence_before_ret_or)
> -        {
> -          /* orl: 0x830c2400.  */
> -          p = frag_more ((flag_code == CODE_64BIT ? 1 : 0) + 4 + 3);
> -          if (flag_code == CODE_64BIT)
> -            *p++ = 0x48;
> -          *p++ = 0x83;
> -          *p++ = 0xc;
> -          *p++ = 0x24;
> -          *p++ = 0x0;
> -        }
> +
> +      bfd_boolean lret = (i.tm.base_opcode | 0x1) == 0xcb;

"(i.tm.base_opcode | 0x5) == 0xcf" or "(i.tm.base_opcode & 8)"
to also cover IRET.

> +      bfd_boolean has_rexw = i.prefix[REX_PREFIX] & REX_W;
> +      char prefix = 0x0;
> +      /* Default operand size for far return is 32 bits,
> +         64 bits for near return.  */
> +      if (has_rexw)
> +        prefix = 0x48;
>        else
> +        prefix = i.prefix[DATA_PREFIX]
> +                 ? 0x66
> +                 : !lret && flag_code == CODE_64BIT ? 0x48 : 0x0;

Aiui the workaround is specifically for Intel CPUs. Intel CPUs
ignore operand size overrides on near RET. (Sorry, I should
have pointed out this fact earlier already.)

> +      if (lfence_before_ret == lfence_before_ret_not)
>          {
> -          p = frag_more ((flag_code == CODE_64BIT ? 2 : 0) + 6 + 3);
> -          /* notl: 0xf71424.  */
> -          if (flag_code == CODE_64BIT)
> -            *p++ = 0x48;
> +          /* not: 0xf71424, may add prefix
> +             for operand size overwrite or 64-bit code.  */

As said before - "override", not "overwrite" (there are several
instances to change).

Jan