linux-hardening - Re: [PATCH v3 1/1] [ARM] Add support for TLS register based stack protector canary access

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: <202110260959.25C44F4@keescook>
Date:   Tue, 26 Oct 2021 10:13:08 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Ard Biesheuvel <ardb@...nel.org>, Qing Zhao <qing.zhao@...cle.com>
Cc:     linux-hardening@...r.kernel.org,
        Keith Packard <keithpac@...zon.com>,
        thomas.preudhomme@...est.fr, adhemerval.zanella@...aro.org,
        Richard Sandiford <richard.sandiford@....com>,
        gcc-patches@....gnu.org
Subject: Re: [PATCH v3 1/1] [ARM] Add support for TLS register based stack
 protector canary access

On Tue, Oct 26, 2021 at 10:18:36AM +0200, Ard Biesheuvel wrote:
> Add support for accessing the stack canary value via the TLS register,
> so that multiple threads running in the same address space can use
> distinct canary values. This is intended for the Linux kernel running in
> SMP mode, where processes entering the kernel are essentially threads
> running the same program concurrently: using a global variable for the
> canary in that context is problematic because it can never be rotated,
> and so the OS is forced to use the same value as long as it remains up.
> 
> Using the TLS register to index the stack canary helps with this, as it
> allows each CPU to context switch the TLS register along with the rest
> of the process, permitting each process to use its own value for the
> stack canary.
> 
> 2021-10-21 Ard Biesheuvel <ardb@...nel.org>
> 
> 	* config/arm/arm-opts.h (enum stack_protector_guard): New
> 	* config/arm/arm-protos.h (arm_stack_protect_tls_canary_mem):
> 	New
> 	* config/arm/arm.c (TARGET_STACK_PROTECT_GUARD): Define
> 	(arm_option_override_internal): Handle and put in error checks
> 	for stack protector guard options.
> 	(arm_option_reconfigure_globals): Likewise
> 	(arm_stack_protect_tls_canary_mem): New
> 	(arm_stack_protect_guard): New
> 	* config/arm/arm.md (stack_protect_set): New
> 	(stack_protect_set_tls): Likewise
> 	(stack_protect_test): Likewise
> 	(stack_protect_test_tls): Likewise
> 	* config/arm/arm.opt (-mstack-protector-guard): New
> 	(-mstack-protector-guard-offset): New.
> 
> Signed-off-by: Ard Biesheuvel <ardb@...nel.org>

I can't speak to the specific implementation details here, but this
builds for me, and behaves as expected. I get a working kernel[1],
and have verified[2] that we have per-task canaries for arm32. :) Yay!

Tested-by: Kees Cook <keescook@...omium.org>

Who's best to review and commit this? Qing, is something you're able to
review?

Thanks!

-Kees

[1] https://lore.kernel.org/linux-arm-kernel/20211021142516.1843042-1-ardb@kernel.org/
[2] https://lore.kernel.org/linux-hardening/20211022223826.330653-3-keescook@chromium.org/

-- 
Kees Cook