linux-kernel - Re: [PATCH 23/43] x86/mm/kaiser: Introduce user-mapped per-cpu areas

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: <20171127092659.vqwltw436urfswv7@gmail.com>
Date:   Mon, 27 Nov 2017 10:26:59 +0100
From:   Ingo Molnar <mingo@...nel.org>
To:     Borislav Petkov <bp@...en8.de>
Cc:     linux-kernel@...r.kernel.org,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Andy Lutomirski <luto@...capital.net>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H . Peter Anvin" <hpa@...or.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 23/43] x86/mm/kaiser: Introduce user-mapped per-cpu areas

* Borislav Petkov <bp@...en8.de> wrote:

> On Fri, Nov 24, 2017 at 06:23:51PM +0100, Ingo Molnar wrote:
> > From: Dave Hansen <dave.hansen@...ux.intel.com>
> > 
> > These patches are based on work from a team at Graz University of
> > Technology posted here: https://github.com/IAIK/KAISER
> > 
> > The KAISER approach keeps two copies of the page tables: one for running
> > in the kernel and one for running userspace.  But, there are a few
> > structures that are needed for switching in and out of the kernel and
> > a good subset of *those* are per-cpu data.
> > 
> > This patch creates a new kind of per-cpu data that is mapped and
> 
> Never say "This patch" in the commit message of a patch. It is
> tautologically useless.

It makes sense in some contexts though. For example:

  The compat variant of SYSCALL doesn't have this problem in the first
  place -- there are plenty of scratch registers, since we don't care
  about preserving r8-r15. This patch therefore doesn't touch SYSCALL32
  at all.

If we only had:

   We don't touch SYSCALL32 at all.

it would be ambiguous: does it describe the current status quo, or perhaps a 
decision we made when writing the patch? The 'this patch' variant adds extra 
emphasis that SYSCALL32 is fine and doesn't require any changes.

Also, even the above variant:

> > This patch creates a new kind of per-cpu data that is mapped and ...

is a bit clearer than:

> > Create a new kind of per-cpu data that is mapped and ...

Because it stresses that this is a new change. The latter form includes that 
information as well, but is pretty close to:

> > The kernel creates a new kind of per-cpu data that is mapped and ...

... which describes the status quo and not new behavior. Explicitly qualifying who 
does something makes it clearer.

I.e. redundancy sometimes helps readability. We don't want 'this patch' in every 
second sentence, but having it written out once, especially where we switch from 
describing existing behavior to describing new behavior (as is the case here) is 
perfectly fine and improves readability.

Thanks,

	Ingo