linux-hardening - Re: [PATCH] remove AND operation in choose_random_kstack

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: <d0959336-4430-4062-b909-54d553238468@app.fastmail.com>
Date: Mon, 17 Jun 2024 22:33:08 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Kees Cook" <kees@...nel.org>, "Mark Rutland" <mark.rutland@....com>
Cc: "Yuntao Liu" <liuyuntao12@...wei.com>, x86@...nel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-s390@...r.kernel.org, linux-hardening@...r.kernel.org,
 "Catalin Marinas" <catalin.marinas@....com>, "Will Deacon" <will@...nel.org>,
 "Heiko Carstens" <hca@...ux.ibm.com>, gor@...ux.ibm.com,
 "Alexander Gordeev" <agordeev@...ux.ibm.com>,
 "Christian Borntraeger" <borntraeger@...ux.ibm.com>,
 "Sven Schnelle" <svens@...ux.ibm.com>,
 "Thomas Gleixner" <tglx@...utronix.de>, "Ingo Molnar" <mingo@...hat.com>,
 "Borislav Petkov" <bp@...en8.de>,
 "Dave Hansen" <dave.hansen@...ux.intel.com>,
 "H. Peter Anvin" <hpa@...or.com>,
 "Gustavo A. R. Silva" <gustavoars@...nel.org>,
 "Leonardo Bras" <leobras@...hat.com>, "Mark Brown" <broonie@...nel.org>,
 imbrenda@...ux.ibm.com, pawan.kumar.gupta@...ux.intel.com
Subject: Re: [PATCH] remove AND operation in choose_random_kstack_offset()

On Mon, Jun 17, 2024, at 20:22, Kees Cook wrote:
> On Mon, Jun 17, 2024 at 04:52:15PM +0100, Mark Rutland wrote:
>> On Mon, Jun 17, 2024 at 01:37:21PM +0000, Yuntao Liu wrote:
>> > Since the offset would be bitwise ANDed with 0x3FF in
>> > add_random_kstack_offset(), so just remove AND operation here.
>> > 
>> > Signed-off-by: Yuntao Liu <liuyuntao12@...wei.com>
>> 
>> The comments in arm64 and x86 say that they're deliberately capping the
>> offset at fewer bits than the result of KSTACK_OFFSET_MAX() masking the
>> value with 0x3FF.
>> 
>> Maybe it's ok to expand that, but if that's the case the commit message
>> needs to explain why it's safe add extra bits (2 on arm64, 3 on s39 and
>> x86), and those comments need to be updated accordingly.
>> 
>> As-is, I do not think this patch is ok.
>
> Yeah, I agree: the truncation is intentional and tuned to the
> architecture.

It may be intentional, but it's clearly nonsense: there is nothing
inherent to the architecture that means we have can go only 256
bytes instead of 512 bytes into the 16KB available stack space.

As far as I can tell, any code just gets bloated to the point
where it fills up the available memory, regardless of how
much you give it. I'm sure one can find code paths today that
exceed the 16KB, so there is no point pretending that 15.75KB
is somehow safe to use while 15.00KB is not.

I'm definitely in favor of making this less architecture
specific, we just need to pick a good value, and we may well
end up deciding to use less than the default 1KB. We can also
go the opposite way and make the limit 4KB but then increase
the default stack size to 20KB for kernels that enable
randomization.

       Arnd