linux-kernel - Re: [PATCH 2/3] x86/asm/entry/32: Remove most of SYSCALL32 code, part 1

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]

Date:	Mon, 27 Jul 2015 18:05:05 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Denys Vlasenko <dvlasenk@...hat.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	"Krzysztof A. Sobiecki" <sobkas@...il.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Borislav Petkov <bp@...en8.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Andy Lutomirski <luto@...capital.net>,
	Oleg Nesterov <oleg@...hat.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Alexei Starovoitov <ast@...mgrid.com>,
	Will Drewry <wad@...omium.org>,
	Kees Cook <keescook@...omium.org>, x86@...nel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] x86/asm/entry/32: Remove most of SYSCALL32 code,
 part 1


* Denys Vlasenko <dvlasenk@...hat.com> wrote:

> SYSCALL32 code is nearly identical to SYSCALL32, except for initial
> section. Merge them.
> 
> The removal is split into two parts, to make review eaiser. This is part 1.
> 
> auditsys_entry_common and auditsys_exit macros are indented one more tab without
> any changes. This prevents diff from becoming unreadable.
> They will be removed in part 2.
> 
> Signed-off-by: Denys Vlasenko <dvlasenk@...hat.com>
> CC: Ingo Molnar <mingo@...nel.org>
> CC: Linus Torvalds <torvalds@...ux-foundation.org>
> CC: Krzysztof A. Sobiecki <sobkas@...il.com>
> CC: Steven Rostedt <rostedt@...dmis.org>
> CC: Borislav Petkov <bp@...en8.de>
> CC: "H. Peter Anvin" <hpa@...or.com>
> CC: Andy Lutomirski <luto@...capital.net>
> CC: Oleg Nesterov <oleg@...hat.com>
> CC: Frederic Weisbecker <fweisbec@...il.com>
> CC: Alexei Starovoitov <ast@...mgrid.com>
> CC: Will Drewry <wad@...omium.org>
> CC: Kees Cook <keescook@...omium.org>
> CC: x86@...nel.org
> CC: linux-kernel@...r.kernel.org
> ---
>  arch/x86/entry/entry_64_compat.S | 237 +++++++++++----------------------------
>  1 file changed, 63 insertions(+), 174 deletions(-)

Yeah, so I realize that this is already a 'cleaner', split up version of your 
original change - but the diffstat is still way too large: _please_ split it up 
into 2-3 further steps to make each step really, utterly obvious.

As you must have experienced it with recent regressions in the x86 entry code, the 
smaller our assembly changes, the easier our job of doing a revert is, when such a 
change regresses ...

I don't care if it ends up being 5-7 patches - that's far more preferable to 
having to decode difficult looking regressions.

For example in hindsight I regret that I did not insist on further splitting up 
this commit:

  53e9accf0f76 ("x86/asm/entry/32: Do not use R9 in SYSCALL32 entry point")

and that was a small patch already:

 arch/x86/entry/ia32entry.S | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

I'm not going to make that mistake again: if you want to change the x86 assembly 
code, you need to learn to decompose dangerous changes into maximally split up, 
atomic, bisectable steps.

Agreed?

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/