linux-kernel - Re: [PATCH 1/3] init: Declare rodata_enabled and mark_rodata

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: <15e7109f-8362-4216-a56c-f6591a3ebaf9@csgroup.eu>
Date: Fri, 22 Dec 2023 18:23:16 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Kees Cook <kees@...nel.org>, Michael Ellerman <mpe@...erman.id.au>,
	Nicholas Piggin <npiggin@...il.com>, Arnd Bergmann <arnd@...db.de>, Luis
 Chamberlain <mcgrof@...nel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
	"linux-modules@...r.kernel.org" <linux-modules@...r.kernel.org>, Kees Cook
	<keescook@...omium.org>
Subject: Re: [PATCH 1/3] init: Declare rodata_enabled and mark_rodata_ro() at
 all time



Le 22/12/2023 à 06:35, Kees Cook a écrit :
> [Vous ne recevez pas souvent de courriers de kees@...nel.org. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> On December 21, 2023 4:16:56 AM PST, Michael Ellerman <mpe@...erman.id.au> wrote:
>> Cc +Kees
>>
>> Christophe Leroy <christophe.leroy@...roup.eu> writes:
>>> Declaring rodata_enabled and mark_rodata_ro() at all time
>>> helps removing related #ifdefery in C files.
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
>>> ---
>>>   include/linux/init.h |  4 ----
>>>   init/main.c          | 21 +++++++--------------
>>>   2 files changed, 7 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/include/linux/init.h b/include/linux/init.h
>>> index 01b52c9c7526..d2b47be38a07 100644
>>> --- a/include/linux/init.h
>>> +++ b/include/linux/init.h
>>> @@ -168,12 +168,8 @@ extern initcall_entry_t __initcall_end[];
>>>
>>>   extern struct file_system_type rootfs_fs_type;
>>>
>>> -#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX)
>>>   extern bool rodata_enabled;
>>> -#endif
>>> -#ifdef CONFIG_STRICT_KERNEL_RWX
>>>   void mark_rodata_ro(void);
>>> -#endif
>>>
>>>   extern void (*late_time_init)(void);
>>>
>>> diff --git a/init/main.c b/init/main.c
>>> index e24b0780fdff..807df08c501f 100644
>>> --- a/init/main.c
>>> +++ b/init/main.c
>>> @@ -1396,10 +1396,9 @@ static int __init set_debug_rodata(char *str)
>>>   early_param("rodata", set_debug_rodata);
>>>   #endif
>>>
>>> -#ifdef CONFIG_STRICT_KERNEL_RWX
>>>   static void mark_readonly(void)
>>>   {
>>> -    if (rodata_enabled) {
>>> +    if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled) {
> 
> I think this will break without rodata_enabled actual existing on other architectures. (Only declaration was made visible, not the definition, which is above here and still behind ifdefs?)

The compiler constant-folds IS_ENABLED(CONFIG_STRICT_KERNEL_RWX).
When it is false, the second part is dropped.

Exemple:

bool test(void)
{
	if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled)
		return true;
	else
		return false;
}

With CONFIG_STRICT_KERNEL_RWX set, it directly returns the content of 
rodata_enabled:

00000160 <test>:
  160:	3d 20 00 00 	lis     r9,0
			162: R_PPC_ADDR16_HA	rodata_enabled
  164:	88 69 00 00 	lbz     r3,0(r9)
			166: R_PPC_ADDR16_LO	rodata_enabled
  168:	4e 80 00 20 	blr

With CONFIG_STRICT_KERNEL_RWX unset, it returns 0 and doesn't reference 
rodata_enabled at all:

000000bc <test>:
   bc:	38 60 00 00 	li      r3,0
   c0:	4e 80 00 20 	blr

Many places in the kernel use this approach to minimise amount of #ifdefs.

Christophe