Date: Wed, 27 Sep 2017 13:47:13 +0200
From: Borislav Petkov <bp@suse.de>
To: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Cc: Ingo Molnar <mingo@redhat.com>, Thomas Gleixner <tglx@linutronix.de>,
        "H. Peter Anvin" <hpa@zytor.com>, Andy Lutomirski <luto@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Brian Gerst <brgerst@gmail.com>, Chris Metcalf <cmetcalf@mellanox.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Masami Hiramatsu <mhiramat@kernel.org>, Huang Rui <ray.huang@amd.com>,
        Jiri Slaby <jslaby@suse.cz>, Jonathan Corbet <corbet@lwn.net>,
        "Michael S. Tsirkin" <mst@redhat.com>,
        Paul Gortmaker <paul.gortmaker@windriver.com>,
        Vlastimil Babka <vbabka@suse.cz>, Chen Yucong <slaoub@gmail.com>,
        "Ravi V. Shankar" <ravi.v.shankar@intel.com>,
        Shuah Khan <shuah@kernel.org>, linux-kernel@vger.kernel.org,
        x86@kernel.org, Adam Buchbinder <adam.buchbinder@gmail.com>,
        Colin Ian King <colin.king@canonical.com>,
        Lorenzo Stoakes <lstoakes@gmail.com>,
        Qiaowei Ren <qiaowei.ren@intel.com>,
        Arnaldo Carvalho de Melo <acme@redhat.com>,
        Adrian Hunter <adrian.hunter@intel.com>,
        Kees Cook <keescook@chromium.org>,
        Thomas Garnier <thgarnie@google.com>,
        Dmitry Vyukov <dvyukov@google.com>
Subject: Re: [PATCH v8 12/28] x86/insn-eval: Add utility functions to get
 segment selector
Message-ID: <20170927114713.wbee7ze2ud2ekvbw@pd.tnic>
References: <20170819002809.111312-1-ricardo.neri-calderon@linux.intel.com>
 <20170819002809.111312-13-ricardo.neri-calderon@linux.intel.com>
 <20170926104353.vmpxybv3v5immc56@pd.tnic>
 <1506486104.8286.142.camel@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1506486104.8286.142.camel@linux.intel.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1518
Lines: 40

On Tue, Sep 26, 2017 at 09:21:44PM -0700, Ricardo Neri wrote:
> This is true except when we don't have an insn at all (well, it may be
> non-NULL but it will only contain garbage). The case to which I am
> referring is when we begin decoding our instruction. The first step is
> to copy_from_user the instruction and populate insn. For this we must
> calculate the linear address from where we copy using CS and rIP.

Where do we do that?

> Furthermore, in this only case we don't need to look at insn at all as
> the only register involved is rIP no segment override prefixes are
> allowed.

In any case, as it is now it sounds convoluted: you may or may not
have an insn, and yet you call get_overridden_seg_reg() on it but you
don't really need segment overrides because you only need CS and rIP
initially.

Sounds to me like this initial parsing should be done separately from
this function...

> I only used "(E)" (i.e., not the "(R|)" part) as these utility
> functions will deal mostly with protected mode, unless FS or GS are
> used in long mode.

eIP or rIP is simply much easier to type and parse. Those brackets, not
really.

> I only check for a NULL insn when needed (i.e., the contents of the
> instruction could change the used segment register).

... and those if (!insn) tests sprinkled around simply make the code
unreadable and if we can get rid of them, we should.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--