linux-kernel - Re: [PATCH 3/4] x86/umip: Identify the str and sldt instructions

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]

Message-ID: <20171114022647.GA1444@voyager>
Date:   Mon, 13 Nov 2017 18:26:47 -0800
From:   Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...e.de>,
        Andy Lutomirski <luto@...nel.org>,
        Tony Luck <tony.luck@...el.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        "Ravi V. Shankar" <ravi.v.shankar@...el.com>, x86@...nel.org,
        ricardo.neri@...el.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] x86/umip: Identify the str and sldt instructions

On Mon, Nov 13, 2017 at 09:12:03AM +0100, Ingo Molnar wrote:
> 
> * Ricardo Neri <ricardo.neri-calderon@...ux.intel.com> wrote:
> 
> > The instructions str and sldt are not emulated in any case. Thus, it made
> > sense to not implement functionality to identify them. However, a
> > subsequent commit will introduce functionality to warn about the use of
> > all the instructions that UMIP protect, not only those that are emulated.
> > A first step for that is the ability to identify them.
> > 
> > Plus, now that str and sldt are identified, we need to explicitly avoid
> > their emulation (i.e., not rely on unsuccessful identification). Group
> > togehter all the cases that we do not want to emulate: str, sldt and user
> > long mode processes.
> 
> Did you notice how in all your previous patches (both in the code and in the 
> changelogs) I have manually fixed up the capitalization of these instruction 
> mnenonics?

I am sorry, I tried to see where you made these changes but I could not find
any. I did a git diff of arch/x86/kernel/umip.c between the branch rneri/umip_v11
of my repository [1] and the master branch of the tip tree and I did not find
any differences.

Also, looking at the log of the master branch of the tip tree I see, for
instance:

commit 1e5db223696afa55e6a038fac638f759e1fdcc01
Author: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
Date:   Sun Nov 5 18:27:52 2017 -0800

    x86/umip: Add emulation code for UMIP instructions
    
    The feature User-Mode Instruction Prevention present in recent Intel
    processor prevents a group of instructions (sgdt, sidt, sldt, smsw, and
    str) from being executed with CPL > 0. Otherwise, a general protection
    fault is issued.
    ...

The instruction mnemonics were not capitalized. Is the master branch the one where
I can look at your fixes?

> 
> The capitalized form is much more readable, especially with seriously overloaded 
> acronyms such as 'str' ...

I see.
> 
> You now repeat the same bad pattern, in fact you regress existing code:
> 
> > -	/* SLDT AND STR are not emulated */
> 
> > +	/* Do not emulate sldt, str or user long mode processes. */
> 
> Please be more careful with such details, and please fix & resend this series.

Sure, I will submit a v2 with capitalized mnemonics in both the code and the
patch descriptions. I will be more careful in the future.

Thanks and BR,
Ricardo

[1]. https://github.com/ricardon/tip/commits/rneri/umip_v11