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

[Hongtao Liu]

On Thu, Mar 26, 2020 at 5:12 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 26.03.2020 03:23, Hongtao Liu wrote:
> > On Wed, Mar 25, 2020 at 6:03 PM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 25.03.2020 10:27, Hongtao Liu wrote:
> >>> On Thu, Mar 12, 2020 at 12:17 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>>> On Wed, Mar 11, 2020 at 3:55 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>> On 10.03.2020 17:05, H.J. Lu wrote:
> >>>>>> +  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))
> >>>>>> +    {
> >>>>>> +      if (last_insn.kind != last_insn_other
> >>>>>> +       && last_insn.seg == now_seg)
> >>>>>> +     {
> >>>>>> +       as_warn_where (last_insn.file, last_insn.line,
> >>>>>> +                      _("`%s` skips -mlfence-before-ret on `%s`"),
> >>>>>> +                      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;
> >>>>>
> >>>>> Shouldn't this depend on RET's operand size? Likewise wouldn't you
> >>>>> also need to insert 0x66/0x67 in certain cases?
> >>>>
> >>>> Hongtao, can you look into it?
> >>>
> >>> I suppose you mean OR's operand size?
> >>
> >> Not exactly - I mean RET's operand size ought to affect the one
> >> chosen for OR.
> >>
> >> Jan
> >
> >>> I wonder whether this shouldn't also enable a safe lfence_before_ret
> >>> mode (i.e. not the OR one), for RET also being an indirect branch. Of
> >>> course care would need to be taken to avoid clobbering an already set
> >>> lfence_before_ret mode.
> >
> > Also for this part, maybe i'll add some comments to indicate
> > -mlfence-before-indirect-branch doesn't include ret. Orelse it would
> > be weird for user when clobber happens, Is it ok for you?
>
> Well, extending the description / comments to be more precise is one
> solution, but only the the 2nd best one. I continue to think that
> there would better be an implication as the one suggested.
>
> Jan

Apologies for the delayed response.
I tried to re-arranged to use a common pattern (memory operand is
destination) and only exclude those which don't also read this
operand. But it turn out there still a lot of such instructions
include all mov instruction, store instruction for i387 and cet,
extract instructions, vgather instructions, vscatter instrcutions,
convert instrcutions and so on, so i didn't re-arrange them.
Other requests are done by the updated patch, also plus handling REP
CMP/SCAS specially since they would set EFLAGS which affects control
flow behavior.

  1. No load for INVPCID, Implict load for POPS/POPF/POPA/XLATB
  2. Add -mlfence-before-ret=shl, adjust operand size of or/not/shl to
  ret's.
  3. Ajust -mlfence-after-load=[yes/no] to
  -mlfence-after-load=[none|general|all]. -mlfence-after-load=[none/all]
  equal original -mlfence-after-load=[no/yes],
  -mlfence-after-load=general won't add lfence after REP CMPS/SCAS
  since they would affect control flow behavior.
  -mlfence-after-load=all will issue an warning when adding lfence
  after REP CMPS/SCAS.
  4. Adjust testcases and documents.

gas/Changelog:
        * config/tc-i386.c (lfence_after_load_kine): New.
        (lfence_before_ret_shl): Change from lfence_before_ret_not.
        (load_insn_p): No load for INVPCID, implict load for
        POPS/POPA/POPF/XLATB.
        (insert_after_load): Insert lfence under
        -mlfence-after-load=[general|all],issue an warning when encounter
        REP CMPS/SCAS.
        (insert_before_before): Replace -mlfence-before-ret=not to
        -mlfence-before-ret=shl.
        (md_parse_option): Adjust -mlfence-after-load=[yes|no] to
        -mlfence-after-load=[none|general|all], Replace
        -mlfence-before-ret=not to -mlfence-before-ret=shl. Enable
        -mlfence-before-ret=shl when
        -mlfence-beofre-indirect-branch=all.
        (md_show_usage): Ditto.
        * doc/c-i386.texi: Ditto.
        * testsuite/gas/i386/i386.exp: Add new testcases.
        * gas/testsuite/gas/i386/lfence-load-b.d: New.
        * gas/testsuite/gas/i386/lfence-load-b.e: New.
        * gas/testsuite/gas/i386/lfence-load.d: Modified.
        * gas/testsuite/gas/i386/lfence-load.e: New.
        * gas/testsuite/gas/i386/lfence-load.s: Modified.
        * gas/testsuite/gas/i386/lfence-ret-a.d: Modified.
        * gas/testsuite/gas/i386/lfence-ret-b.d: Modified.
        * gas/testsuite/gas/i386/lfence-ret-c.d: New.
        * gas/testsuite/gas/i386/lfence-ret-d.d: New.
        * gas/testsuite/gas/i386/lfence-ret.s: Modified
        * gas/testsuite/gas/i386/x86-64-lfence-load-b.d: New.
        * gas/testsuite/gas/i386/x86-64-lfence-load.d: Modified.
        * gas/testsuite/gas/i386/x86-64-lfence-load.s: Modified.
        * gas/testsuite/gas/i386/x86-64-lfence-ret-a.d: Modified.
        * gas/testsuite/gas/i386/x86-64-lfence-ret-b.d: Modified.
        * gas/testsuite/gas/i386/x86-64-lfence-ret-c.d: New.
        * gas/testsuite/gas/i386/x86-64-lfence-ret-d.d: New.


-- 
BR,
Hongtao
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Improve-mlfence-after-load.patch
Type: text/x-patch
Size: 41016 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20200416/2e80deb4/attachment-0001.bin>