linux-kernel - Re: [PATCH] x86-64: simplify is32entry.S again

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] [day] [month] [year] [list]

Message-ID: <4C9CCF67.3000106@zytor.com>
Date:	Fri, 24 Sep 2010 09:18:47 -0700
From:	"H. Peter Anvin" <hpa@...or.com>
To:	Jan Beulich <JBeulich@...ell.com>
CC:	mingo@...e.hu, tglx@...utronix.de, akpm@...ux-foundation.org,
	Roland McGrath <roland@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86-64: simplify is32entry.S again

On 09/24/2010 05:35 AM, Jan Beulich wrote:
> Commits 36d001c70d8a0144ac1d038f6876c484849a74de and
> eefdca043e8391dcd719711716492063030b55ac fixed the same problem in two
> different, redundant of one another ways: Zero-extending the system
> call index from 32 to 64 bits and then doing comparison on the 64 bit
> register is just pointless.
> 
> Undo the second commit completely, as using REX prefixes where needed
> produces more efficient code than having an extra instruction in the
> stream (no matter how simple it is).
> 
> The first commit by itself also did quite a bit more than needed -
> comparison on the full 64-bit register is definitely unnecessary in
> all paths where the value is known to zero extended from 32 bits
> already (in one case this is even directly visible from the patch, as
> the zero extending instruction is the immediately preceding one). Undo
> those parts of the first commit too.

Yes, this optimizes the code slightly... it also removes any redundancy
in the protections which is part of why the bug could reappear in the
first place. Unless you have concrete numbers on system call entry
latency and that these longer instructions hurt, I *really don't* want that.

As Linus said, "the code was too subtle in the first place".

Furthermore, Roland was concerned about buggy ptrace users relying on
the zero-extension... if nothing else zero-extending in some paths and
not in others would open up for another instance of the same class of
problems (tested-is-not-what-is-executed).

So I'm going to NAK this patch unless provided with strong evidence that
the efficiency pays off.  Sorry.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
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/