Re: [PATCH v4] x86, kaslr: Access the correct kaslr_enabled variable

[Baoquan He <bhe@redhat.com>]

On 03/15/15 at 12:49am, Yinghai Lu wrote:

> Index: linux-2.6/arch/x86/kernel/setup.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/setup.c
> +++ linux-2.6/arch/x86/kernel/setup.c
> @@ -429,7 +429,18 @@ static void __init reserve_initrd(void)
>  
>  static void __init parse_kaslr_setup(u64 pa_data, u32 data_len)
>  {
> -	kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data));
> +	/* kaslr_setup_data is defined in aslr.c */
> +	unsigned char *data;
> +	unsigned long offset = sizeof(struct setup_data);
> +
> +	data = early_memremap(pa_data, offset + 1);
> +	if (!data) {

It's good to check the ret value as Boris suggested. However it could
fail since early_memremap self fail, e.g slot not found. In this case
making kaslr_enabled true may not be good.

As Minfei talked with you kaslr_setup_data is a global variable inside
kernel code, it has been ident mapped. Just derefencing the physical
address which is virtual address too and getting the real stored value
may be safer. And also parse_kaslr_setup is a function specified to
handle kaslr, it doesn't make me uncomfortable to implement with a
specific knowledge which here means the setup_data is a global varialbe
in kernel code and no need to do early_memremap since mapping has been
built .

Thanks
Baoquan

> +		kaslr_enabled = true;
> +		return;
> +	}
> +
> +	kaslr_enabled = *(data + offset);
> +	early_memunmap(data, offset + 1);
>  }
>  
>  static void __init parse_setup_data(void)