linux-kernel - Re: [PATCH -next] arm32: enable HAVE_LD_DEAD_CODE_DATA

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: Tue, 20 Feb 2024 17:53:00 +0800
From: "liuyuntao (F)" <liuyuntao12@...wei.com>
To: Arnd Bergmann <arnd@...db.de>, <linux-arm-kernel@...ts.infradead.org>,
	<linux-kernel@...r.kernel.org>
CC: Russell King <linux@...linux.org.uk>, Andrew Davis <afd@...com>, Andrew
 Morton <akpm@...ux-foundation.org>, "Kirill A. Shutemov"
	<kirill.shutemov@...ux.intel.com>, Geert Uytterhoeven
	<geert+renesas@...der.be>, Jonathan Corbet <corbet@....net>, Mike Rapoport
	<rppt@...nel.org>, Eric DeVolder <eric.devolder@...cle.com>, Rob Herring
	<robh@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, Linus Walleij
	<linus.walleij@...aro.org>
Subject: Re: [PATCH -next] arm32: enable HAVE_LD_DEAD_CODE_DATA_ELIMINATION



在 2024/2/20 16:40, Arnd Bergmann 写道:
> On Tue, Feb 20, 2024, at 09:15, Yuntao Liu wrote:
>> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S
>> b/arch/arm/boot/compressed/vmlinux.lds.S
>> index 3fcb3e62dc56..da21244aa892 100644
>> --- a/arch/arm/boot/compressed/vmlinux.lds.S
>> +++ b/arch/arm/boot/compressed/vmlinux.lds.S
>> @@ -89,7 +89,7 @@ SECTIONS
>>        * The EFI stub always executes from RAM, and runs strictly before
>> the
>>        * decompressor, so we can make an exception for its r/w data, and
>> keep it
>>        */
>> -    *(.data.efistub .bss.efistub)
>> +    *(.data.* .bss.*)
>>       __pecoff_data_end = .;
>>
>>       /*
> 
> This doesn't seem right to me, or maybe I misunderstand what
> the original version does. Have you tested with both
> CONFIG_EFI_STUB on and off, and booting with and without
> UEFI?

Yes, I have tested with CONFIG_EFI_STUB on and off, and booting with 
UEFI on a single-board computer, and it boots well.


> 
> If I read this right, you would move all .data and .bss
> into the stub here, not just the parts we actually want?

In the file "drivers/firmware/efi/libstub/Makefile", it is written:

---

#
# ARM discards the .data section because it disallows r/w data in the
# decompressor. So move our .data to .data.efistub and .bss to .bss.efistub,
# which are preserved explicitly by the decompressor linker script.
#
STUBCOPY_FLAGS-$(CONFIG_ARM)	+= --rename-section .data=.data.efistub	\
				   --rename-section .bss=.bss.efistub,load,alloc

---

I think that .data.efistub represents the entire .data section, the same 
applies to .bss as well,

so i move all .data and .bss into the stub here.


> 
>> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
>> index bd9127c4b451..de373c6c2ae8 100644
>> --- a/arch/arm/kernel/vmlinux.lds.S
>> +++ b/arch/arm/kernel/vmlinux.lds.S
>> @@ -74,7 +74,7 @@ SECTIONS
>>   	. = ALIGN(4);
>>   	__ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {
>>   		__start___ex_table = .;
>> -		ARM_MMU_KEEP(*(__ex_table))
>> +		ARM_MMU_KEEP(KEEP(*(__ex_table)))
>>   		__stop___ex_table = .;
>>   	}
>>
>> @@ -116,7 +116,7 @@ SECTIONS
>>   #endif
>>   	.init.pv_table : {
>>   		__pv_table_begin = .;
>> -		*(.pv_table)
>> +		KEEP(*(.pv_table))
>>   		__pv_table_end = .;
>>   	}
> 
> I guess this prevents discarding any function that has a reference
> from pv_table or ex_table, even if there are no other references,
> right?

Indeed so, if not keep ex_table,  the compilation process will result in

an error:

     no __ex_table in file: vmlinux

     Failed to sort kernel tables

and if not keep pv_table, It can be compiled successfully, but the QEMU 
boots will fail.

> 
> I don't know how to solve this other than forcing all the
> uaccess and virt_to_phys functions to be out of line
> helpers. For uaccess, there are probably very few functions
> that need this, so it should make little difference.
> 
> You might want to try changing CONFIG_ARM_PATCH_PHYS_VIRT
> into a method that just always adds an offset from C code
> instead of the boot time patching. That way the code would
> be a bit less efficient but you might be able to get
> a larger size reduction by dropping additional unused code.
> 
> Maybe test your patch both with and without
> ARM_PATCH_PHYS_VIRT to see what the best-case impact would
> be.
> 

This is a very good idea, I will give it a try.

>        Arnd