[PATCH v2] arm64: Add support to supply 'kaslr-seed' to secondary kernel

[PATCH v2] arm64: Add support to supply 'kaslr-seed' to secondary kernel

[Bhupesh Sharma]

This patch adds the support to supply 'kaslr-seed' to secondary kernel,
when we do a 'kexec warm reboot to another kernel' (although the
behaviour remains the same for the 'kdump' case as well) on arm64
platforms using the 'kexec_load' invocation method.

Lets consider the case where the primary kernel working on the arm64
platform supports kaslr (i.e 'CONFIG_RANDOMIZE_BASE' was set to y and
we have a compliant EFI firmware which supports EFI_RNG_PROTOCOL and
hence can pass a non-zero (valid) seed to the primary kernel).

Now the primary kernel reads the 'kaslr-seed' and wipes it to 0 and
uses the seed value to randomize for e.g. the module base address
offset.

In the case of 'kexec_load' (or even kdump for brevity),
we rely on the user-space kexec-tools to pass an appropriate dtb to the
secondary kernel and since 'kaslr-seed' is wiped to 0 by the primary
kernel, the secondary will essentially work with *nokaslr* as
'kaslr-seed' is set to 0 when it is passed to the secondary kernel.

This can be true even in case the secondary kernel had
'CONFIG_RANDOMIZE_BASE' and 'CONFIG_RANDOMIZE_MODULE_REGION_FULL' set to
y.

This patch addresses this issue by first checking if the device tree
provided by the firmware to the kernel supports the 'kaslr-seed'
property and verifies that it is really wiped to 0. If this condition is
met, it fixes up the 'kaslr-seed' property by using the getrandom()
syscall to get a suitable random number.

I verified this patch on my Qualcomm arm64 board and here are some test
results:

1. Ensure that the primary kernel is boot'ed with 'kaslr-seed'
   dts property and it is really wiped to 0:

   [root@qualcomm-amberwing]# dtc -I dtb -O dts /sys/firmware/fdt | grep -A 10 -i chosen
   	chosen {
		kaslr-seed = <0x0 0x0>;
		...
	}

2. Now issue 'kexec_load' to load the secondary kernel (let's assume
   that we are using the same kernel as the secondary kernel):
   # kexec -l /boot/vmlinuz-`uname -r` --initrd=/boot/initramfs-`uname
     -r`.img --reuse-cmdline -d

3. Issue 'kexec -e' to warm boot to the secondary:
   # kexec -e

4. Now after the secondary boots, confirm that the load address of the
   modules is randomized in every successive boot:

   [root@qualcomm-amberwing]# cat /proc/modules
   sunrpc 524288 1 - Live 0xffff0307db190000
   vfat 262144 1 - Live 0xffff0307db110000
   fat 262144 1 vfat, Live 0xffff0307db090000
   crc32_ce 262144 0 - Live 0xffff0307d8c70000
   ...

Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
---
Changes since v1:
 - Addressed Akashi's comments regarding the goto label path.
 - v1 can be viewed here: https://marc.info/?l=kexec&m=152373724406110&w=2

 kexec/arch/arm64/kexec-arm64.c | 138 +++++++++++++++++++++++++++++------------
 1 file changed, 100 insertions(+), 38 deletions(-)

diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
index 62f37585b788..a206c172b1aa 100644
--- a/kexec/arch/arm64/kexec-arm64.c
+++ b/kexec/arch/arm64/kexec-arm64.c
@@ -15,6 +15,11 @@
 #include <linux/elf-em.h>
 #include <elf.h>
 
+#include <unistd.h>
+#include <syscall.h>
+#include <errno.h>
+#include <linux/random.h>
+
 #include "kexec.h"
 #include "kexec-arm64.h"
 #include "crashdump.h"
@@ -392,11 +397,13 @@ static int fdt_setprop_range(void *fdt, int nodeoffset,
 static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
 {
 	uint32_t address_cells, size_cells;
-	int range_len;
-	int nodeoffset;
+	uint64_t fdt_val64;
+	uint64_t *prop;
 	char *new_buf = NULL;
+	int len, range_len;
+	int nodeoffset;
 	int new_size;
-	int result;
+	int result, kaslr_seed;
 
 	result = fdt_check_header(dtb->buf);
 
@@ -407,47 +414,103 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
 
 	result = set_bootargs(dtb, command_line);
 
-	if (on_crash) {
-		/* determine #address-cells and #size-cells */
-		result = get_cells_size(dtb->buf, &address_cells, &size_cells);
-		if (result) {
-			fprintf(stderr,
-				"kexec: cannot determine cells-size.\n");
-			result = -EINVAL;
-			goto on_error;
-		}
+	/* determine #address-cells and #size-cells */
+	result = get_cells_size(dtb->buf, &address_cells, &size_cells);
+	if (result) {
+		fprintf(stderr, "kexec: cannot determine cells-size.\n");
+		result = -EINVAL;
+		goto on_error;
+	}
 
-		if (!cells_size_fitted(address_cells, size_cells,
-					&elfcorehdr_mem)) {
-			fprintf(stderr,
-				"kexec: elfcorehdr doesn't fit cells-size.\n");
+	if (!cells_size_fitted(address_cells, size_cells,
+				&elfcorehdr_mem)) {
+		fprintf(stderr, "kexec: elfcorehdr doesn't fit cells-size.\n");
+		result = -EINVAL;
+		goto on_error;
+	}
+
+	if (!cells_size_fitted(address_cells, size_cells,
+				&crash_reserved_mem)) {
+		fprintf(stderr, "kexec: usable memory range doesn't fit cells-size.\n");
+		result = -EINVAL;
+		goto on_error;
+	}
+
+	/* duplicate dt blob */
+	range_len = sizeof(uint32_t) * (address_cells + size_cells);
+	new_size = fdt_totalsize(dtb->buf)
+		+ fdt_prop_len(PROP_ELFCOREHDR, range_len)
+		+ fdt_prop_len(PROP_USABLE_MEM_RANGE, range_len);
+
+	new_buf = xmalloc(new_size);
+	result = fdt_open_into(dtb->buf, new_buf, new_size);
+	if (result) {
+		dbgprintf("%s: fdt_open_into failed: %s\n", __func__,
+				fdt_strerror(result));
+		result = -ENOSPC;
+		goto on_error;
+	}
+
+	/* fixup 'kaslr-seed' with a random value, if supported */
+	nodeoffset = fdt_path_offset(new_buf, "/chosen");
+	prop = fdt_getprop_w(new_buf, nodeoffset,
+			"kaslr-seed", &len);
+	if (!prop || len != sizeof(uint64_t)) {
+		dbgprintf("%s: no kaslr-seed found\n",
+				__func__);
+		/* for kexec warm reboot case, we don't need to fixup
+		 * other dtb properties
+		 */
+		if (!on_crash) {
+			dump_reservemap(dtb);
+			if (new_buf)
+				free(new_buf);
+
+			return result;
+		}
+	} else {
+		kaslr_seed = fdt64_to_cpu(*prop);
+
+		/* kaslr_seed must be wiped clean by primary
+		 * kernel during boot
+		 */
+		if (kaslr_seed != 0) {
+			dbgprintf("%s: kaslr-seed is not wiped to 0.\n",
+					__func__);
 			result = -EINVAL;
 			goto on_error;
 		}
 
-		if (!cells_size_fitted(address_cells, size_cells,
-					&crash_reserved_mem)) {
-			fprintf(stderr,
-				"kexec: usable memory range doesn't fit cells-size.\n");
+		/*
+		 * Invoke the getrandom system call with
+		 * GRND_NONBLOCK, to make sure we
+		 * have a valid random seed to pass to the
+		 * secondary kernel.
+		 */
+		result = syscall(SYS_getrandom, &fdt_val64,
+				sizeof(fdt_val64),
+				GRND_NONBLOCK);
+
+		if(result == -1) {
+			dbgprintf("%s: Reading random bytes failed.\n",
+					__func__);
 			result = -EINVAL;
 			goto on_error;
 		}
 
-		/* duplicate dt blob */
-		range_len = sizeof(uint32_t) * (address_cells + size_cells);
-		new_size = fdt_totalsize(dtb->buf)
-			+ fdt_prop_len(PROP_ELFCOREHDR, range_len)
-			+ fdt_prop_len(PROP_USABLE_MEM_RANGE, range_len);
-
-		new_buf = xmalloc(new_size);
-		result = fdt_open_into(dtb->buf, new_buf, new_size);
+		nodeoffset = fdt_path_offset(new_buf, "/chosen");
+		result = fdt_setprop_inplace(new_buf,
+				nodeoffset, "kaslr-seed",
+				&fdt_val64, sizeof(fdt_val64));
 		if (result) {
-			dbgprintf("%s: fdt_open_into failed: %s\n", __func__,
-				fdt_strerror(result));
-			result = -ENOSPC;
+			dbgprintf("%s: fdt_setprop failed: %s\n",
+					__func__, fdt_strerror(result));
+			result = -EINVAL;
 			goto on_error;
 		}
+	}
 
+	if (on_crash) {
 		/* add linux,elfcorehdr */
 		nodeoffset = fdt_path_offset(new_buf, "/chosen");
 		result = fdt_setprop_range(new_buf, nodeoffset,
@@ -455,7 +518,7 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
 				address_cells, size_cells);
 		if (result) {
 			dbgprintf("%s: fdt_setprop failed: %s\n", __func__,
-				fdt_strerror(result));
+					fdt_strerror(result));
 			result = -EINVAL;
 			goto on_error;
 		}
@@ -467,18 +530,17 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
 				address_cells, size_cells);
 		if (result) {
 			dbgprintf("%s: fdt_setprop failed: %s\n", __func__,
-				fdt_strerror(result));
+					fdt_strerror(result));
 			result = -EINVAL;
 			goto on_error;
 		}
-
-		fdt_pack(new_buf);
-		dtb->buf = new_buf;
-		dtb->size = fdt_totalsize(new_buf);
 	}
 
-	dump_reservemap(dtb);
+	fdt_pack(new_buf);
+	dtb->buf = new_buf;
+	dtb->size = fdt_totalsize(new_buf);
 
+	dump_reservemap(dtb);
 
 	return result;
 
-- 
2.7.4


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2] arm64: Add support to supply 'kaslr-seed' to secondary kernel

[Bhupesh Sharma]

Hello Akashi,

On Thu, Apr 26, 2018 at 2:27 PM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
> This patch adds the support to supply 'kaslr-seed' to secondary kernel,
> when we do a 'kexec warm reboot to another kernel' (although the
> behaviour remains the same for the 'kdump' case as well) on arm64
> platforms using the 'kexec_load' invocation method.
>
> Lets consider the case where the primary kernel working on the arm64
> platform supports kaslr (i.e 'CONFIG_RANDOMIZE_BASE' was set to y and
> we have a compliant EFI firmware which supports EFI_RNG_PROTOCOL and
> hence can pass a non-zero (valid) seed to the primary kernel).
>
> Now the primary kernel reads the 'kaslr-seed' and wipes it to 0 and
> uses the seed value to randomize for e.g. the module base address
> offset.
>
> In the case of 'kexec_load' (or even kdump for brevity),
> we rely on the user-space kexec-tools to pass an appropriate dtb to the
> secondary kernel and since 'kaslr-seed' is wiped to 0 by the primary
> kernel, the secondary will essentially work with *nokaslr* as
> 'kaslr-seed' is set to 0 when it is passed to the secondary kernel.
>
> This can be true even in case the secondary kernel had
> 'CONFIG_RANDOMIZE_BASE' and 'CONFIG_RANDOMIZE_MODULE_REGION_FULL' set to
> y.
>
> This patch addresses this issue by first checking if the device tree
> provided by the firmware to the kernel supports the 'kaslr-seed'
> property and verifies that it is really wiped to 0. If this condition is
> met, it fixes up the 'kaslr-seed' property by using the getrandom()
> syscall to get a suitable random number.
>
> I verified this patch on my Qualcomm arm64 board and here are some test
> results:
>
> 1. Ensure that the primary kernel is boot'ed with 'kaslr-seed'
>    dts property and it is really wiped to 0:
>
>    [root@qualcomm-amberwing]# dtc -I dtb -O dts /sys/firmware/fdt | grep -A 10 -i chosen
>         chosen {
>                 kaslr-seed = <0x0 0x0>;
>                 ...
>         }
>
> 2. Now issue 'kexec_load' to load the secondary kernel (let's assume
>    that we are using the same kernel as the secondary kernel):
>    # kexec -l /boot/vmlinuz-`uname -r` --initrd=/boot/initramfs-`uname
>      -r`.img --reuse-cmdline -d
>
> 3. Issue 'kexec -e' to warm boot to the secondary:
>    # kexec -e
>
> 4. Now after the secondary boots, confirm that the load address of the
>    modules is randomized in every successive boot:
>
>    [root@qualcomm-amberwing]# cat /proc/modules
>    sunrpc 524288 1 - Live 0xffff0307db190000
>    vfat 262144 1 - Live 0xffff0307db110000
>    fat 262144 1 vfat, Live 0xffff0307db090000
>    crc32_ce 262144 0 - Live 0xffff0307d8c70000
>    ...
>
> Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
> ---
> Changes since v1:
>  - Addressed Akashi's comments regarding the goto label path.
>  - v1 can be viewed here: https://marc.info/?l=kexec&m=152373724406110&w=2

Ping. Any comments on this v2?

Thanks,
Bhupesh

>  kexec/arch/arm64/kexec-arm64.c | 138 +++++++++++++++++++++++++++++------------
>  1 file changed, 100 insertions(+), 38 deletions(-)
>
> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> index 62f37585b788..a206c172b1aa 100644
> --- a/kexec/arch/arm64/kexec-arm64.c
> +++ b/kexec/arch/arm64/kexec-arm64.c
> @@ -15,6 +15,11 @@
>  #include <linux/elf-em.h>
>  #include <elf.h>
>
> +#include <unistd.h>
> +#include <syscall.h>
> +#include <errno.h>
> +#include <linux/random.h>
> +
>  #include "kexec.h"
>  #include "kexec-arm64.h"
>  #include "crashdump.h"
> @@ -392,11 +397,13 @@ static int fdt_setprop_range(void *fdt, int nodeoffset,
>  static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
>  {
>         uint32_t address_cells, size_cells;
> -       int range_len;
> -       int nodeoffset;
> +       uint64_t fdt_val64;
> +       uint64_t *prop;
>         char *new_buf = NULL;
> +       int len, range_len;
> +       int nodeoffset;
>         int new_size;
> -       int result;
> +       int result, kaslr_seed;
>
>         result = fdt_check_header(dtb->buf);
>
> @@ -407,47 +414,103 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
>
>         result = set_bootargs(dtb, command_line);
>
> -       if (on_crash) {
> -               /* determine #address-cells and #size-cells */
> -               result = get_cells_size(dtb->buf, &address_cells, &size_cells);
> -               if (result) {
> -                       fprintf(stderr,
> -                               "kexec: cannot determine cells-size.\n");
> -                       result = -EINVAL;
> -                       goto on_error;
> -               }
> +       /* determine #address-cells and #size-cells */
> +       result = get_cells_size(dtb->buf, &address_cells, &size_cells);
> +       if (result) {
> +               fprintf(stderr, "kexec: cannot determine cells-size.\n");
> +               result = -EINVAL;
> +               goto on_error;
> +       }
>
> -               if (!cells_size_fitted(address_cells, size_cells,
> -                                       &elfcorehdr_mem)) {
> -                       fprintf(stderr,
> -                               "kexec: elfcorehdr doesn't fit cells-size.\n");
> +       if (!cells_size_fitted(address_cells, size_cells,
> +                               &elfcorehdr_mem)) {
> +               fprintf(stderr, "kexec: elfcorehdr doesn't fit cells-size.\n");
> +               result = -EINVAL;
> +               goto on_error;
> +       }
> +
> +       if (!cells_size_fitted(address_cells, size_cells,
> +                               &crash_reserved_mem)) {
> +               fprintf(stderr, "kexec: usable memory range doesn't fit cells-size.\n");
> +               result = -EINVAL;
> +               goto on_error;
> +       }
> +
> +       /* duplicate dt blob */
> +       range_len = sizeof(uint32_t) * (address_cells + size_cells);
> +       new_size = fdt_totalsize(dtb->buf)
> +               + fdt_prop_len(PROP_ELFCOREHDR, range_len)
> +               + fdt_prop_len(PROP_USABLE_MEM_RANGE, range_len);
> +
> +       new_buf = xmalloc(new_size);
> +       result = fdt_open_into(dtb->buf, new_buf, new_size);
> +       if (result) {
> +               dbgprintf("%s: fdt_open_into failed: %s\n", __func__,
> +                               fdt_strerror(result));
> +               result = -ENOSPC;
> +               goto on_error;
> +       }
> +
> +       /* fixup 'kaslr-seed' with a random value, if supported */
> +       nodeoffset = fdt_path_offset(new_buf, "/chosen");
> +       prop = fdt_getprop_w(new_buf, nodeoffset,
> +                       "kaslr-seed", &len);
> +       if (!prop || len != sizeof(uint64_t)) {
> +               dbgprintf("%s: no kaslr-seed found\n",
> +                               __func__);
> +               /* for kexec warm reboot case, we don't need to fixup
> +                * other dtb properties
> +                */
> +               if (!on_crash) {
> +                       dump_reservemap(dtb);
> +                       if (new_buf)
> +                               free(new_buf);
> +
> +                       return result;
> +               }
> +       } else {
> +               kaslr_seed = fdt64_to_cpu(*prop);
> +
> +               /* kaslr_seed must be wiped clean by primary
> +                * kernel during boot
> +                */
> +               if (kaslr_seed != 0) {
> +                       dbgprintf("%s: kaslr-seed is not wiped to 0.\n",
> +                                       __func__);
>                         result = -EINVAL;
>                         goto on_error;
>                 }
>
> -               if (!cells_size_fitted(address_cells, size_cells,
> -                                       &crash_reserved_mem)) {
> -                       fprintf(stderr,
> -                               "kexec: usable memory range doesn't fit cells-size.\n");
> +               /*
> +                * Invoke the getrandom system call with
> +                * GRND_NONBLOCK, to make sure we
> +                * have a valid random seed to pass to the
> +                * secondary kernel.
> +                */
> +               result = syscall(SYS_getrandom, &fdt_val64,
> +                               sizeof(fdt_val64),
> +                               GRND_NONBLOCK);
> +
> +               if(result == -1) {
> +                       dbgprintf("%s: Reading random bytes failed.\n",
> +                                       __func__);
>                         result = -EINVAL;
>                         goto on_error;
>                 }
>
> -               /* duplicate dt blob */
> -               range_len = sizeof(uint32_t) * (address_cells + size_cells);
> -               new_size = fdt_totalsize(dtb->buf)
> -                       + fdt_prop_len(PROP_ELFCOREHDR, range_len)
> -                       + fdt_prop_len(PROP_USABLE_MEM_RANGE, range_len);
> -
> -               new_buf = xmalloc(new_size);
> -               result = fdt_open_into(dtb->buf, new_buf, new_size);
> +               nodeoffset = fdt_path_offset(new_buf, "/chosen");
> +               result = fdt_setprop_inplace(new_buf,
> +                               nodeoffset, "kaslr-seed",
> +                               &fdt_val64, sizeof(fdt_val64));
>                 if (result) {
> -                       dbgprintf("%s: fdt_open_into failed: %s\n", __func__,
> -                               fdt_strerror(result));
> -                       result = -ENOSPC;
> +                       dbgprintf("%s: fdt_setprop failed: %s\n",
> +                                       __func__, fdt_strerror(result));
> +                       result = -EINVAL;
>                         goto on_error;
>                 }
> +       }
>
> +       if (on_crash) {
>                 /* add linux,elfcorehdr */
>                 nodeoffset = fdt_path_offset(new_buf, "/chosen");
>                 result = fdt_setprop_range(new_buf, nodeoffset,
> @@ -455,7 +518,7 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
>                                 address_cells, size_cells);
>                 if (result) {
>                         dbgprintf("%s: fdt_setprop failed: %s\n", __func__,
> -                               fdt_strerror(result));
> +                                       fdt_strerror(result));
>                         result = -EINVAL;
>                         goto on_error;
>                 }
> @@ -467,18 +530,17 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
>                                 address_cells, size_cells);
>                 if (result) {
>                         dbgprintf("%s: fdt_setprop failed: %s\n", __func__,
> -                               fdt_strerror(result));
> +                                       fdt_strerror(result));
>                         result = -EINVAL;
>                         goto on_error;
>                 }
> -
> -               fdt_pack(new_buf);
> -               dtb->buf = new_buf;
> -               dtb->size = fdt_totalsize(new_buf);
>         }
>
> -       dump_reservemap(dtb);
> +       fdt_pack(new_buf);
> +       dtb->buf = new_buf;
> +       dtb->size = fdt_totalsize(new_buf);
>
> +       dump_reservemap(dtb);
>
>         return result;
>
> --
> 2.7.4
>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2] arm64: Add support to supply 'kaslr-seed' to secondary kernel

[Bhupesh Sharma]

Hello Simon,

On 05/15/2018 12:50 PM, Bhupesh Sharma wrote:
> Hello Akashi,
> 
> On Thu, Apr 26, 2018 at 2:27 PM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
>> This patch adds the support to supply 'kaslr-seed' to secondary kernel,
>> when we do a 'kexec warm reboot to another kernel' (although the
>> behaviour remains the same for the 'kdump' case as well) on arm64
>> platforms using the 'kexec_load' invocation method.
>>
>> Lets consider the case where the primary kernel working on the arm64
>> platform supports kaslr (i.e 'CONFIG_RANDOMIZE_BASE' was set to y and
>> we have a compliant EFI firmware which supports EFI_RNG_PROTOCOL and
>> hence can pass a non-zero (valid) seed to the primary kernel).
>>
>> Now the primary kernel reads the 'kaslr-seed' and wipes it to 0 and
>> uses the seed value to randomize for e.g. the module base address
>> offset.
>>
>> In the case of 'kexec_load' (or even kdump for brevity),
>> we rely on the user-space kexec-tools to pass an appropriate dtb to the
>> secondary kernel and since 'kaslr-seed' is wiped to 0 by the primary
>> kernel, the secondary will essentially work with *nokaslr* as
>> 'kaslr-seed' is set to 0 when it is passed to the secondary kernel.
>>
>> This can be true even in case the secondary kernel had
>> 'CONFIG_RANDOMIZE_BASE' and 'CONFIG_RANDOMIZE_MODULE_REGION_FULL' set to
>> y.
>>
>> This patch addresses this issue by first checking if the device tree
>> provided by the firmware to the kernel supports the 'kaslr-seed'
>> property and verifies that it is really wiped to 0. If this condition is
>> met, it fixes up the 'kaslr-seed' property by using the getrandom()
>> syscall to get a suitable random number.
>>
>> I verified this patch on my Qualcomm arm64 board and here are some test
>> results:
>>
>> 1. Ensure that the primary kernel is boot'ed with 'kaslr-seed'
>>     dts property and it is really wiped to 0:
>>
>>     [root@qualcomm-amberwing]# dtc -I dtb -O dts /sys/firmware/fdt | grep -A 10 -i chosen
>>          chosen {
>>                  kaslr-seed = <0x0 0x0>;
>>                  ...
>>          }
>>
>> 2. Now issue 'kexec_load' to load the secondary kernel (let's assume
>>     that we are using the same kernel as the secondary kernel):
>>     # kexec -l /boot/vmlinuz-`uname -r` --initrd=/boot/initramfs-`uname
>>       -r`.img --reuse-cmdline -d
>>
>> 3. Issue 'kexec -e' to warm boot to the secondary:
>>     # kexec -e
>>
>> 4. Now after the secondary boots, confirm that the load address of the
>>     modules is randomized in every successive boot:
>>
>>     [root@qualcomm-amberwing]# cat /proc/modules
>>     sunrpc 524288 1 - Live 0xffff0307db190000
>>     vfat 262144 1 - Live 0xffff0307db110000
>>     fat 262144 1 vfat, Live 0xffff0307db090000
>>     crc32_ce 262144 0 - Live 0xffff0307d8c70000
>>     ...
>>
>> Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
>> ---
>> Changes since v1:
>>   - Addressed Akashi's comments regarding the goto label path.
>>   - v1 can be viewed here: https://marc.info/?l=kexec&m=152373724406110&w=2
> 
> Ping. Any comments on this v2?
> 
> Thanks,
> Bhupesh
> 
>>   kexec/arch/arm64/kexec-arm64.c | 138 +++++++++++++++++++++++++++++------------
>>   1 file changed, 100 insertions(+), 38 deletions(-)
>>
>> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
>> index 62f37585b788..a206c172b1aa 100644
>> --- a/kexec/arch/arm64/kexec-arm64.c
>> +++ b/kexec/arch/arm64/kexec-arm64.c
>> @@ -15,6 +15,11 @@
>>   #include <linux/elf-em.h>
>>   #include <elf.h>
>>
>> +#include <unistd.h>
>> +#include <syscall.h>
>> +#include <errno.h>
>> +#include <linux/random.h>
>> +
>>   #include "kexec.h"
>>   #include "kexec-arm64.h"
>>   #include "crashdump.h"
>> @@ -392,11 +397,13 @@ static int fdt_setprop_range(void *fdt, int nodeoffset,
>>   static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
>>   {
>>          uint32_t address_cells, size_cells;
>> -       int range_len;
>> -       int nodeoffset;
>> +       uint64_t fdt_val64;
>> +       uint64_t *prop;
>>          char *new_buf = NULL;
>> +       int len, range_len;
>> +       int nodeoffset;
>>          int new_size;
>> -       int result;
>> +       int result, kaslr_seed;
>>
>>          result = fdt_check_header(dtb->buf);
>>
>> @@ -407,47 +414,103 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
>>
>>          result = set_bootargs(dtb, command_line);
>>
>> -       if (on_crash) {
>> -               /* determine #address-cells and #size-cells */
>> -               result = get_cells_size(dtb->buf, &address_cells, &size_cells);
>> -               if (result) {
>> -                       fprintf(stderr,
>> -                               "kexec: cannot determine cells-size.\n");
>> -                       result = -EINVAL;
>> -                       goto on_error;
>> -               }
>> +       /* determine #address-cells and #size-cells */
>> +       result = get_cells_size(dtb->buf, &address_cells, &size_cells);
>> +       if (result) {
>> +               fprintf(stderr, "kexec: cannot determine cells-size.\n");
>> +               result = -EINVAL;
>> +               goto on_error;
>> +       }
>>
>> -               if (!cells_size_fitted(address_cells, size_cells,
>> -                                       &elfcorehdr_mem)) {
>> -                       fprintf(stderr,
>> -                               "kexec: elfcorehdr doesn't fit cells-size.\n");
>> +       if (!cells_size_fitted(address_cells, size_cells,
>> +                               &elfcorehdr_mem)) {
>> +               fprintf(stderr, "kexec: elfcorehdr doesn't fit cells-size.\n");
>> +               result = -EINVAL;
>> +               goto on_error;
>> +       }
>> +
>> +       if (!cells_size_fitted(address_cells, size_cells,
>> +                               &crash_reserved_mem)) {
>> +               fprintf(stderr, "kexec: usable memory range doesn't fit cells-size.\n");
>> +               result = -EINVAL;
>> +               goto on_error;
>> +       }
>> +
>> +       /* duplicate dt blob */
>> +       range_len = sizeof(uint32_t) * (address_cells + size_cells);
>> +       new_size = fdt_totalsize(dtb->buf)
>> +               + fdt_prop_len(PROP_ELFCOREHDR, range_len)
>> +               + fdt_prop_len(PROP_USABLE_MEM_RANGE, range_len);
>> +
>> +       new_buf = xmalloc(new_size);
>> +       result = fdt_open_into(dtb->buf, new_buf, new_size);
>> +       if (result) {
>> +               dbgprintf("%s: fdt_open_into failed: %s\n", __func__,
>> +                               fdt_strerror(result));
>> +               result = -ENOSPC;
>> +               goto on_error;
>> +       }
>> +
>> +       /* fixup 'kaslr-seed' with a random value, if supported */
>> +       nodeoffset = fdt_path_offset(new_buf, "/chosen");
>> +       prop = fdt_getprop_w(new_buf, nodeoffset,
>> +                       "kaslr-seed", &len);
>> +       if (!prop || len != sizeof(uint64_t)) {
>> +               dbgprintf("%s: no kaslr-seed found\n",
>> +                               __func__);
>> +               /* for kexec warm reboot case, we don't need to fixup
>> +                * other dtb properties
>> +                */
>> +               if (!on_crash) {
>> +                       dump_reservemap(dtb);
>> +                       if (new_buf)
>> +                               free(new_buf);
>> +
>> +                       return result;
>> +               }
>> +       } else {
>> +               kaslr_seed = fdt64_to_cpu(*prop);
>> +
>> +               /* kaslr_seed must be wiped clean by primary
>> +                * kernel during boot
>> +                */
>> +               if (kaslr_seed != 0) {
>> +                       dbgprintf("%s: kaslr-seed is not wiped to 0.\n",
>> +                                       __func__);
>>                          result = -EINVAL;
>>                          goto on_error;
>>                  }
>>
>> -               if (!cells_size_fitted(address_cells, size_cells,
>> -                                       &crash_reserved_mem)) {
>> -                       fprintf(stderr,
>> -                               "kexec: usable memory range doesn't fit cells-size.\n");
>> +               /*
>> +                * Invoke the getrandom system call with
>> +                * GRND_NONBLOCK, to make sure we
>> +                * have a valid random seed to pass to the
>> +                * secondary kernel.
>> +                */
>> +               result = syscall(SYS_getrandom, &fdt_val64,
>> +                               sizeof(fdt_val64),
>> +                               GRND_NONBLOCK);
>> +
>> +               if(result == -1) {
>> +                       dbgprintf("%s: Reading random bytes failed.\n",
>> +                                       __func__);
>>                          result = -EINVAL;
>>                          goto on_error;
>>                  }
>>
>> -               /* duplicate dt blob */
>> -               range_len = sizeof(uint32_t) * (address_cells + size_cells);
>> -               new_size = fdt_totalsize(dtb->buf)
>> -                       + fdt_prop_len(PROP_ELFCOREHDR, range_len)
>> -                       + fdt_prop_len(PROP_USABLE_MEM_RANGE, range_len);
>> -
>> -               new_buf = xmalloc(new_size);
>> -               result = fdt_open_into(dtb->buf, new_buf, new_size);
>> +               nodeoffset = fdt_path_offset(new_buf, "/chosen");
>> +               result = fdt_setprop_inplace(new_buf,
>> +                               nodeoffset, "kaslr-seed",
>> +                               &fdt_val64, sizeof(fdt_val64));
>>                  if (result) {
>> -                       dbgprintf("%s: fdt_open_into failed: %s\n", __func__,
>> -                               fdt_strerror(result));
>> -                       result = -ENOSPC;
>> +                       dbgprintf("%s: fdt_setprop failed: %s\n",
>> +                                       __func__, fdt_strerror(result));
>> +                       result = -EINVAL;
>>                          goto on_error;
>>                  }
>> +       }
>>
>> +       if (on_crash) {
>>                  /* add linux,elfcorehdr */
>>                  nodeoffset = fdt_path_offset(new_buf, "/chosen");
>>                  result = fdt_setprop_range(new_buf, nodeoffset,
>> @@ -455,7 +518,7 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
>>                                  address_cells, size_cells);
>>                  if (result) {
>>                          dbgprintf("%s: fdt_setprop failed: %s\n", __func__,
>> -                               fdt_strerror(result));
>> +                                       fdt_strerror(result));
>>                          result = -EINVAL;
>>                          goto on_error;
>>                  }
>> @@ -467,18 +530,17 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
>>                                  address_cells, size_cells);
>>                  if (result) {
>>                          dbgprintf("%s: fdt_setprop failed: %s\n", __func__,
>> -                               fdt_strerror(result));
>> +                                       fdt_strerror(result));
>>                          result = -EINVAL;
>>                          goto on_error;
>>                  }
>> -
>> -               fdt_pack(new_buf);
>> -               dtb->buf = new_buf;
>> -               dtb->size = fdt_totalsize(new_buf);
>>          }
>>
>> -       dump_reservemap(dtb);
>> +       fdt_pack(new_buf);
>> +       dtb->buf = new_buf;
>> +       dtb->size = fdt_totalsize(new_buf);
>>
>> +       dump_reservemap(dtb);
>>
>>          return result;
>>
>> --
>> 2.7.4
>>

Ping. Seems we don't have comments on this v2 (since the last month or so).

Having the capability to pass an appropriate 'kaslr-seed' value to the 
secondary kernel is important for booting KASLR enabled secondary 
kernels - which is an important use case for kexec for distributions 
like Fedora.

Kindly consider applying this patch which has been properly tested on a 
couple of arm64 machines both for KASLR and non-KASLR boot cases.

Right now, I am forced to keep this patch locally and refer KASLR users 
on arm64 to my public github tree which has this patch committed on top 
of the upstream kexec-tools. I would prefer to route them to the 
upstream kexec-tools itself (rather than my github tree), once this 
patch is applied there.

Thanks for your help.

Regards,
Bhupesh

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2] arm64: Add support to supply 'kaslr-seed' to secondary kernel

[Simon Horman]

On Mon, Jun 18, 2018 at 11:26:11AM +0530, Bhupesh Sharma wrote:
> Hello Simon,
> 
> On 05/15/2018 12:50 PM, Bhupesh Sharma wrote:
> > Hello Akashi,
> > 
> > On Thu, Apr 26, 2018 at 2:27 PM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
> > > This patch adds the support to supply 'kaslr-seed' to secondary kernel,
> > > when we do a 'kexec warm reboot to another kernel' (although the
> > > behaviour remains the same for the 'kdump' case as well) on arm64
> > > platforms using the 'kexec_load' invocation method.
> > > 
> > > Lets consider the case where the primary kernel working on the arm64
> > > platform supports kaslr (i.e 'CONFIG_RANDOMIZE_BASE' was set to y and
> > > we have a compliant EFI firmware which supports EFI_RNG_PROTOCOL and
> > > hence can pass a non-zero (valid) seed to the primary kernel).
> > > 
> > > Now the primary kernel reads the 'kaslr-seed' and wipes it to 0 and
> > > uses the seed value to randomize for e.g. the module base address
> > > offset.
> > > 
> > > In the case of 'kexec_load' (or even kdump for brevity),
> > > we rely on the user-space kexec-tools to pass an appropriate dtb to the
> > > secondary kernel and since 'kaslr-seed' is wiped to 0 by the primary
> > > kernel, the secondary will essentially work with *nokaslr* as
> > > 'kaslr-seed' is set to 0 when it is passed to the secondary kernel.
> > > 
> > > This can be true even in case the secondary kernel had
> > > 'CONFIG_RANDOMIZE_BASE' and 'CONFIG_RANDOMIZE_MODULE_REGION_FULL' set to
> > > y.
> > > 
> > > This patch addresses this issue by first checking if the device tree
> > > provided by the firmware to the kernel supports the 'kaslr-seed'
> > > property and verifies that it is really wiped to 0. If this condition is
> > > met, it fixes up the 'kaslr-seed' property by using the getrandom()
> > > syscall to get a suitable random number.
> > > 
> > > I verified this patch on my Qualcomm arm64 board and here are some test
> > > results:
> > > 
> > > 1. Ensure that the primary kernel is boot'ed with 'kaslr-seed'
> > >     dts property and it is really wiped to 0:
> > > 
> > >     [root@qualcomm-amberwing]# dtc -I dtb -O dts /sys/firmware/fdt | grep -A 10 -i chosen
> > >          chosen {
> > >                  kaslr-seed = <0x0 0x0>;
> > >                  ...
> > >          }
> > > 
> > > 2. Now issue 'kexec_load' to load the secondary kernel (let's assume
> > >     that we are using the same kernel as the secondary kernel):
> > >     # kexec -l /boot/vmlinuz-`uname -r` --initrd=/boot/initramfs-`uname
> > >       -r`.img --reuse-cmdline -d
> > > 
> > > 3. Issue 'kexec -e' to warm boot to the secondary:
> > >     # kexec -e
> > > 
> > > 4. Now after the secondary boots, confirm that the load address of the
> > >     modules is randomized in every successive boot:
> > > 
> > >     [root@qualcomm-amberwing]# cat /proc/modules
> > >     sunrpc 524288 1 - Live 0xffff0307db190000
> > >     vfat 262144 1 - Live 0xffff0307db110000
> > >     fat 262144 1 vfat, Live 0xffff0307db090000
> > >     crc32_ce 262144 0 - Live 0xffff0307d8c70000
> > >     ...
> > > 
> > > Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
> > > ---
> > > Changes since v1:
> > >   - Addressed Akashi's comments regarding the goto label path.
> > >   - v1 can be viewed here: https://marc.info/?l=kexec&m=152373724406110&w=2
> > 
> > Ping. Any comments on this v2?

...

> Ping. Seems we don't have comments on this v2 (since the last month or so).
> 
> Having the capability to pass an appropriate 'kaslr-seed' value to the
> secondary kernel is important for booting KASLR enabled secondary kernels -
> which is an important use case for kexec for distributions like Fedora.
> 
> Kindly consider applying this patch which has been properly tested on a
> couple of arm64 machines both for KASLR and non-KASLR boot cases.
> 
> Right now, I am forced to keep this patch locally and refer KASLR users on
> arm64 to my public github tree which has this patch committed on top of the
> upstream kexec-tools. I would prefer to route them to the upstream
> kexec-tools itself (rather than my github tree), once this patch is applied
> there.

Thanks for the patch, applied.
My questions were covered in the review of v1.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec