linux-kernel - Re: [PATCH 1/7] riscv: cleanup XIP

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fb039f36-07ad-488b-a988-c149a68ef177@ghiti.fr>
Date: Mon, 27 May 2024 14:16:56 +0200
From: Alexandre Ghiti <alex@...ti.fr>
To: Nam Cao <namcao@...utronix.de>, Paul Walmsley <paul.walmsley@...ive.com>,
 Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/7] riscv: cleanup XIP_FIXUP macro

Hi Nam,

On 10/05/2024 08:28, Nam Cao wrote:
> The XIP_FIXUP macro is used to fix addresses early during boot before MMU:
> generated code "thinks" the data section is in ROM while it is actually in
> RAM. So this macro correct the addresses in the data section.
>
> This macro determines if the address needs to be fixed by checking if it is
> within the range startting of ROM address up to the size of 2 * XIP_OFFSET


s/startting/starting

And the sentence lacks the final dot.


>
> This means addresses within the .text section would incorrectly get fixed.


Yes, but XIP_FIXUP() should only be applied to data symbols so I believe 
this ^ is not relevant.


> Also if the kernel size if bigger than (2 * XIP_OFFSET), some addresses
> would not be fixed up.

s/the kernel size if/the kernel size is

>
> XIP kernel can still work if the above 2 cases do not happen. But this
> macro is obviously incorrect.
>
> Rewrite this macro to only fix up addresses within the data section.
>
> Signed-off-by: Nam Cao <namcao@...utronix.de>
> ---
>   arch/riscv/include/asm/pgtable.h | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 58fd7b70b903..fbf342f4afee 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -139,11 +139,14 @@
>   
>   #ifdef CONFIG_XIP_KERNEL
>   #define XIP_FIXUP(addr) ({							\
> +	extern char _sdata[], _start[], _end[];					\
> +	uintptr_t __rom_start_data = CONFIG_XIP_PHYS_ADDR			\
> +				+ (uintptr_t)&_sdata - (uintptr_t)&_start;	\
> +	uintptr_t __rom_end_data = CONFIG_XIP_PHYS_ADDR				\
> +				+ (uintptr_t)&_end - (uintptr_t)&_start;	\
>   	uintptr_t __a = (uintptr_t)(addr);					\
> -	(__a >= CONFIG_XIP_PHYS_ADDR && \
> -	 __a < CONFIG_XIP_PHYS_ADDR + XIP_OFFSET * 2) ?	\
> -		__a - CONFIG_XIP_PHYS_ADDR + CONFIG_PHYS_RAM_BASE - XIP_OFFSET :\
> -		__a;								\
> +	(__a >= __rom_start_data && __a < __rom_end_data) ?			\
> +		__a - __rom_start_data + CONFIG_PHYS_RAM_BASE :	__a;		\
>   	})
>   #else
>   #define XIP_FIXUP(addr)		(addr)


Reviewed-by: Alexandre Ghiti <alexghiti@...osinc.com>

Thanks,

Alex