From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vs1-xe43.google.com (mail-vs1-xe43.google.com [IPv6:2607:f8b0:4864:20::e43]) by sourceware.org (Postfix) with ESMTPS id 15B13385E006 for ; Thu, 26 Mar 2020 02:18:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 15B13385E006 Received: by mail-vs1-xe43.google.com with SMTP id r7so28780vsg.7 for ; Wed, 25 Mar 2020 19:18:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=3+58jb0ISSsHQQ9DxUAYrQFbVsAECXzqhsRIJUtL90g=; b=fWaL4HURksAAazxTxhjmYr19mWG23HGtsS+v7hlXctKqTiAQWbfHxutSrRjxKtVDZg +Ye29YxKYncgCj6g/IJvB4JTljcBMzDRR9vriSsRO9N1WA6ycRRXfbWiyolZOHUl+J83 +Ga1C4J9Yks8zVbytxNvfcUvjzU2TYqqsdmUzEh4GMdizQ/OSkh5QLumsTD53sEnYfys v2SXPhEv3h23QfmsjCKLkHmTK3PpaUJcz+SZtptQA6jBavlz4tpC7i1Mcg8fT66YeF6x 0ntyQUTRmcXYTsoHFuJ3EkcY2LwUelsEABatkjMZ/XaIay3tA9SwFD7hdF6Y0dpYMK4U JA+g== X-Gm-Message-State: ANhLgQ0KkffbzfXnoRe+PIHuCh7TGR81R8Od5WrMtMNQ+yumbWNHcVHF 9k6ASSRGIm1qyjeTTMgyqeGYQ1OfaoIRPkMjSuw= X-Google-Smtp-Source: ADFU+vsjLLrjGzZmgqLgu3pr0mjWMo3Bx8wvHYJe3b1M4WFJU+ea2jNupoRXdCmL2xMvGYlmlME83VsMscIqU03adsU= X-Received: by 2002:a67:2ed2:: with SMTP id u201mr5234986vsu.209.1585189095448; Wed, 25 Mar 2020 19:18:15 -0700 (PDT) MIME-Version: 1.0 References: <20200310160528.303613-1-hjl.tools@gmail.com> <20200310160528.303613-2-hjl.tools@gmail.com> <6dcb50a3-d3ad-6abc-8a5f-703373df95a1@suse.com> <70acc1ec-ce95-9df7-cdce-1d6c2305b1b4@suse.com> In-Reply-To: <70acc1ec-ce95-9df7-cdce-1d6c2305b1b4@suse.com> From: Hongtao Liu Date: Thu, 26 Mar 2020 10:23:03 +0800 Message-ID: Subject: Re: [PATCH 1/2] i386: Generate lfence with load/indirect branch/ret [CVE-2020-0551] To: Jan Beulich Cc: "H.J. Lu" , Binutils , "Lu, Hongjiu" Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Mar 2020 02:18:17 -0000 On Wed, Mar 25, 2020 at 6:03 PM Jan Beulich wrote: > > On 25.03.2020 10:27, Hongtao Liu wrote: > > On Thu, Mar 12, 2020 at 12:17 AM H.J. Lu wrote: > >> On Wed, Mar 11, 2020 at 3:55 AM Jan Beulich wrote: > >>> On 10.03.2020 17:05, H.J. Lu wrote: > >>>> + dest = i.operands - 1; > >>>> + > >>>> + /* Check fake imm8 operand and 3 source operands. */ > >>>> + if ((i.tm.opcode_modifier.immext > >>>> + || i.tm.opcode_modifier.vexsources == VEX3SOURCES) > >>>> + && i.types[dest].bitfield.imm8) > >>>> + dest--; > >>>> + > >>>> + /* add, or, adc, sbb, and, sub, xor, cmp, test, xchg, xadd */ > >>>> + if (!any_vex_p > >>>> + && (i.tm.base_opcode == 0x0 > >>>> + || i.tm.base_opcode == 0x1 > >>>> + || i.tm.base_opcode == 0x8 > >>>> + || i.tm.base_opcode == 0x9 > >>>> + || i.tm.base_opcode == 0x10 > >>>> + || i.tm.base_opcode == 0x11 > >>>> + || i.tm.base_opcode == 0x18 > >>>> + || i.tm.base_opcode == 0x19 > >>>> + || i.tm.base_opcode == 0x20 > >>>> + || i.tm.base_opcode == 0x21 > >>>> + || i.tm.base_opcode == 0x28 > >>>> + || i.tm.base_opcode == 0x29 > >>>> + || i.tm.base_opcode == 0x30 > >>>> + || i.tm.base_opcode == 0x31 > >>>> + || i.tm.base_opcode == 0x38 > >>>> + || i.tm.base_opcode == 0x39 > >>>> + || (i.tm.base_opcode >= 0x84 && i.tm.base_opcode <= 0x87) > >>>> + || i.tm.base_opcode == 0xfc0 > >>>> + || i.tm.base_opcode == 0xfc1)) > >>>> + return 1; > >>> > >>> Don't quite a few of these fit very well with ... > >>> > >> > >> Changed. > >> > >>>> + /* Check for load instruction. */ > >>>> + return (i.types[dest].bitfield.class != ClassNone > >>>> + || i.types[dest].bitfield.instance == Accum); > >>> > >>> ... this generic expression? It would seem to me that only TEST > >>> and XCHG need special casing, for allowing either operand order. > >>> Same seems to apply to quite a few of the special cases in the > >>> big "else" block further up, and even its if() [vldmxcsr] part. > >> > >> Hongtao, can you look into it? > >> > > > > Many instruction take mem operand both as input and output, they also > > need to be handled. But they're not fitting well in generic > > expressions, because they either only 1 operand, or mem operand is in > > the dest place. > > Well, my earlier reply wasn't quite precise enough, I think. Aiui > what you're after to exclude are insns only writing their memory > operand. With the pretty long list of excluded opcodes I wonder > whether this can be re-arranged to use a common pattern (memory > operand is destination) and only exclude the few ones which don't > also read this operand. Then again by using a few & and | the list > above could be shrunk significantly, and hence may no longer look > this odd (I notice the committed version has this reduced a little, > but not quite as much as would be possible). > Yes, understand. > >>>> + 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? -- BR, Hongtao