linux-kernel - Re: [PATCH -tip v2 0/2] kprobes/x86: RO text code bugfix and cleanup

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: <20170817095530.zew2tppikufosf2u@gmail.com>
Date:   Thu, 17 Aug 2017 11:55:30 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     Ingo Molnar <mingo@...hat.com>, "H . Peter Anvin" <hpa@...or.com>,
        x86@...nel.org, Ananth N Mavinakayanahalli <ananth@...ibm.com>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>,
        "David S . Miller" <davem@...emloft.net>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH -tip v2 0/2] kprobes/x86: RO text code bugfix and cleanup

* Masami Hiramatsu <mhiramat@...nel.org> wrote:

> Hi,
> 
> This series fixes a kprobe-x86 bug related to RO text and
> cleans up addressof operators.
> 
> The first one is an obvious bug that misses to set memory
> RO when the function fails. And the second one is just a
> cleanup patch to remove addressof operators ("&") since
> it is meaningless anymore.
> 
> V2 has just a following update:
>  - [1/2] Use a helper variable instead of using p->ainsn.insn
>    directly.
> 
> Thanks,
> 
> ---
> 
> Masami Hiramatsu (2):
>       kprobes/x86: Don't forget to set memory back to RO on failure
>       kprobes/x86: Remove addressof operators
> 
> 
>  arch/x86/include/asm/kprobes.h |    4 ++--
>  arch/x86/kernel/kprobes/core.c |   15 +++++++++------
>  arch/x86/kernel/kprobes/opt.c  |    9 +++++----
>  3 files changed, 16 insertions(+), 12 deletions(-)

So I'm totally opposed to the whole approach of modifying the permissions of the 
kernel text virtual memory area.

Firstly, it's racy against other kernel subsystems: what happens if some other 
code patching mechanism does a similar 'mark RWX and then back to RX'? Who 
provides the synchronization? set_memory_*() certainly does not.

Secondly, it's racy against attackers: if an attacker can time the attack to the 
time when a kprobe is installed, then the kernel is still vulnerable.

So how about avoiding the problem altogether by patching the kernel not in its 
virtual text address, but in the direct mappings? Then page permissions won't have 
to be modified, and the whole solution will be more robust and secure.

Is there anything I'm missing?

Thanks,

	Ingo