Re: [PATCH v5 15/39] arm64: idreg-override: Prepare for place relative reloc patching

[Marc Zyngier <maz@kernel.org>]

On Fri, 24 Nov 2023 10:18:55 +0000,
Ard Biesheuvel <ardb@google.com> wrote:
> 
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> The ID reg override handling code uses a rather elaborate data structure
> that relies on statically initialized absolute address values in pointer
> fields. This means that this code cannot run until relocation fixups
> have been applied, and this is unfortunate, because it means we cannot
> discover overrides for KASLR or LVA/LPA without creating the kernel
> mapping and performing the relocations first.
> 
> This can be solved by switching to place-relative relocations, which can
> be applied by the linker at build time. This means some additional
> arithmetic is required when dereferencing these pointers, as we can no
> longer dereference the pointer members directly.
> 
> So let's implement this for idreg-override.c in a preliminary way, i.e.,
> convert all the references in code to use a special accessor that
> produces the correct absolute value at runtime.
> 
> To preserve the strong type checking for the static initializers, use
> union types for representing the hybrid quantities.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/arm64/kernel/idreg-override.c | 98 +++++++++++++-------
>  1 file changed, 65 insertions(+), 33 deletions(-)
> 
> diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c
> index 536bc33859bc..4e32a44560bf 100644
> --- a/arch/arm64/kernel/idreg-override.c
> +++ b/arch/arm64/kernel/idreg-override.c
> @@ -21,14 +21,32 @@
>  
>  static u64 __boot_status __initdata;
>  
> +// temporary __prel64 related definitions
> +// to be removed when this code is moved under pi/
> +
> +#define __prel64_initconst	__initconst
> +
> +typedef void *prel64_t;
> +
> +static void *prel64_to_pointer(const prel64_t *p)
> +{
> +	return *p;
> +}
> +

Having played with this series a bit to see how the E2H0 override
support would fit in, I found that this cast to a void* could hide
stupid bugs that should normally be caught at compile time.

Having hacked on it a bit, I came up with this (partial patch on top
of the full series):

diff --git a/arch/arm64/kernel/pi/idreg-override.c b/arch/arm64/kernel/pi/idreg-override.c
index 84647ebb87ee..d60322477e44 100644
--- a/arch/arm64/kernel/pi/idreg-override.c
+++ b/arch/arm64/kernel/pi/idreg-override.c
@@ -246,12 +246,12 @@ static void __init match_options(const char *cmdline)
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(regs); i++) {
-		const struct ftr_set_desc *reg = prel64_to_pointer(&regs[i].reg_prel);
+		const struct ftr_set_desc *reg = prel64_pointer(regs[i].reg);
 		struct arm64_ftr_override *override;
 		int len = strlen(reg->name);
 		int f;
 
-		override = prel64_to_pointer(&reg->override_prel);
+		override = prel64_pointer(reg->override);
 
 		// set opt[] to '<name>.'
 		memcpy(opt, reg->name, len);
diff --git a/arch/arm64/kernel/pi/pi.h b/arch/arm64/kernel/pi/pi.h
index 04a1f576baee..9d922322999b 100644
--- a/arch/arm64/kernel/pi/pi.h
+++ b/arch/arm64/kernel/pi/pi.h
@@ -15,6 +15,8 @@ static inline void *prel64_to_pointer(const prel64_t *offset)
 	return (void *)offset + *offset;
 }
 
+#define prel64_pointer(__d)	(typeof(__d))prel64_to_pointer(&__d##_prel)
+
 extern bool dynamic_scs_is_enabled;
 
 extern pgd_t init_idmap_pg_dir[];


which preserves the type-checking, at the expense of the implicit
*_prel field access. Not sure if that's something you have considered,
but I thought I'd raise it here.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel