arm64: kaslr: Adjust the offset to avoid Image across alignment boundary

PATCH New Catalin Marinas 3 participants 8 messages Aug 18, 2017 arm64

Share & tools

Catalin Marinas

author maintainer

2017-08-18 15:04 UTC #0 lore raw

PATCH 1/1 arm64: kaslr: Adjust the offset to avoid Image across alignment boundary

With 16KB pages and a kernel Image larger than 16MB, the current kaslr_early_init() logic for avoiding mappings across swapper table boundaries fails since increasing the offset by kimg_sz just moves the problem to the next boundary.

This patch decreases the offset by the boundary overflow amount, with slight risk of reduced entropy as the kernel is more likely to be found at kimg_sz below a swapper table boundary.

Trying-to-fix: afd0e5a87670 ("arm64: kaslr: Fix up the kernel image alignment")

Cc Ard Biesheuvel <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Will Deacon <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Signed-off-by Catalin Marinas <[email protected]>

---

While preparing this email, I noticed that the kernel eventually failed to boot, though after a lot more reboot iterations. Mark Rutland also managed to make the KASLR kernel fail to boot with 64K pages which wouldn't be explained by this patch.

So, any suggestions are welcome. My testing method, qemu starting a guest in a loop with virtio-rng-pci.

Thanks.

 arch/arm64/kernel/kaslr.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

arch/arm64/kernel/kaslr.c +5 -4

index a9710efb8c01..e8cdc02f66ae 100644
@@ -131,13 +131,14 @@ u64 __init kaslr_early_init(u64 dt_phys, u64 modulo_offset)
 	/*
 	 * The kernel Image should not extend across a 1GB/32MB/512MB alignment
 	 * boundary (for 4KB/16KB/64KB granule kernels, respectively). If this
-	 * happens, increase the KASLR offset by the size of the kernel image
-	 * rounded up by SWAPPER_BLOCK_SIZE.
+	 * happens, decrease the KASLR offset by the boundary overflow rounded
+	 * up to SWAPPER_BLOCK_SIZE.
 	 */
 	if ((((u64)_text + offset + modulo_offset) >> SWAPPER_TABLE_SHIFT) !=
 	    (((u64)_end + offset + modulo_offset) >> SWAPPER_TABLE_SHIFT)) {
-		u64 kimg_sz = _end - _text;
-		offset = (offset + round_up(kimg_sz, SWAPPER_BLOCK_SIZE))
+		u64 adjust = ((u64)_end + offset + modulo_offset) &
+			((1 << SWAPPER_TABLE_SHIFT) - 1);
+		offset = (offset - round_up(adjust, SWAPPER_BLOCK_SIZE))
 				& mask;
 	}

Catalin Marinas

author maintainer

2017-08-18 15:19 UTC #1 lore raw

PATCH 1/1 arm64: kaslr: Adjust the offset to avoid Image across alignment boundary

Quoted 17 lines · Catalin Marinas

With 16KB pages and a kernel Image larger than 16MB, the current kaslr_early_init() logic for avoiding mappings across swapper table boundaries fails since increasing the offset by kimg_sz just moves the problem to the next boundary.
This patch decreases the offset by the boundary overflow amount, with slight risk of reduced entropy as the kernel is more likely to be found at kimg_sz below a swapper table boundary.
Trying-to-fix: afd0e5a87670 ("arm64: kaslr: Fix up the kernel image alignment")
Cc Ard Biesheuvel <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Will Deacon <[email protected]>
Cc Neeraj Upadhyay <[email protected]>
Signed-off-by Catalin Marinas <[email protected]>
---

Quoted 7 lines

While preparing this email, I noticed that the kernel eventually failed to boot, though after a lot more reboot iterations. Mark Rutland also managed to make the KASLR kernel fail to boot with 64K pages which wouldn't be explained by this patch.
So, any suggestions are welcome. My testing method, qemu starting a guest in a loop with virtio-rng-pci.

Apparently, the booting gets much more stable if I disable the physical relocation in arm64-stub.c (but keep the virtual one with the fix in this patch). So I guess we are chasing two different issues.

Signature · 1 line

Catalin

Ard Biesheuvel

2017-08-18 15:20 UTC #2 lore raw

PATCH 1/1 arm64: kaslr: Adjust the offset to avoid Image across alignment boundary

Quoted 23 lines · Catalin Marinas

Quoted 22 lines · Catalin Marinas
With 16KB pages and a kernel Image larger than 16MB, the current kaslr_early_init() logic for avoiding mappings across swapper table boundaries fails since increasing the offset by kimg_sz just moves the problem to the next boundary.
This patch decreases the offset by the boundary overflow amount, with slight risk of reduced entropy as the kernel is more likely to be found at kimg_sz below a swapper table boundary.
Trying-to-fix: afd0e5a87670 ("arm64: kaslr: Fix up the kernel image alignment")
Cc Ard Biesheuvel <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Will Deacon <[email protected]>
Cc Neeraj Upadhyay <[email protected]>
Signed-off-by Catalin Marinas <[email protected]>
---
While preparing this email, I noticed that the kernel eventually failed to boot, though after a lot more reboot iterations. Mark Rutland also managed to make the KASLR kernel fail to boot with 64K pages which wouldn't be explained by this patch.

Quoted 7 lines

Quoted 2 lines
So, any suggestions are welcome. My testing method, qemu starting a guest in a loop with virtio-rng-pci.
Apparently, the booting gets much more stable if I disable the physical relocation in arm64-stub.c (but keep the virtual one with the fix in this patch). So I guess we are chasing two different issues.

So this is using QEMU with 16k pages support?

Catalin Marinas

author maintainer

2017-08-18 15:22 UTC #3 lore raw

PATCH 1/1 arm64: kaslr: Adjust the offset to avoid Image across alignment boundary

Quoted 24 lines · Ard Biesheuvel

Quoted 23 lines · Catalin Marinas
Quoted 22 lines · Catalin Marinas
With 16KB pages and a kernel Image larger than 16MB, the current kaslr_early_init() logic for avoiding mappings across swapper table boundaries fails since increasing the offset by kimg_sz just moves the problem to the next boundary.
This patch decreases the offset by the boundary overflow amount, with slight risk of reduced entropy as the kernel is more likely to be found at kimg_sz below a swapper table boundary.
Trying-to-fix: afd0e5a87670 ("arm64: kaslr: Fix up the kernel image alignment")
Cc Ard Biesheuvel <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Will Deacon <[email protected]>
Cc Neeraj Upadhyay <[email protected]>
Signed-off-by Catalin Marinas <[email protected]>
---
While preparing this email, I noticed that the kernel eventually failed to boot, though after a lot more reboot iterations. Mark Rutland also managed to make the KASLR kernel fail to boot with 64K pages which wouldn't be explained by this patch.

Quoted 8 lines

Quoted 6 lines
Quoted 2 lines
So, any suggestions are welcome. My testing method, qemu starting a guest in a loop with virtio-rng-pci.
Apparently, the booting gets much more stable if I disable the physical relocation in arm64-stub.c (but keep the virtual one with the fix in this patch). So I guess we are chasing two different issues.
So this is using QEMU with 16k pages support?

Qemu running on a ThunderX, so native KVM support.

Signature · 1 line

Catalin

Ard Biesheuvel

2017-08-18 15:24 UTC #4 lore raw

PATCH 1/1 arm64: kaslr: Adjust the offset to avoid Image across alignment boundary

Quoted 28 lines · Catalin Marinas

Quoted 27 lines · Ard Biesheuvel
Quoted 26 lines · Catalin Marinas
Quoted 24 lines · Catalin Marinas
With 16KB pages and a kernel Image larger than 16MB, the current kaslr_early_init() logic for avoiding mappings across swapper table boundaries fails since increasing the offset by kimg_sz just moves the problem to the next boundary.
This patch decreases the offset by the boundary overflow amount, with slight risk of reduced entropy as the kernel is more likely to be found at kimg_sz below a swapper table boundary.
Trying-to-fix: afd0e5a87670 ("arm64: kaslr: Fix up the kernel image alignment")
Cc Ard Biesheuvel <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Will Deacon <[email protected]>
Cc Neeraj Upadhyay <[email protected]>
Signed-off-by Catalin Marinas <[email protected]>
---
While preparing this email, I noticed that the kernel eventually failed to boot, though after a lot more reboot iterations. Mark Rutland also managed to make the KASLR kernel fail to boot with 64K pages which wouldn't be explained by this patch.
So, any suggestions are welcome. My testing method, qemu starting a guest in a loop with virtio-rng-pci.

Quoted 8 lines

Quoted 5 lines
Quoted 3 lines
Apparently, the booting gets much more stable if I disable the physical relocation in arm64-stub.c (but keep the virtual one with the fix in this patch). So I guess we are chasing two different issues.
So this is using QEMU with 16k pages support?
Qemu running on a ThunderX, so native KVM support.

Ah ok. I did not realize QEMU supports 16 KB pages in that case. Nice!

However, that makes it rather difficult to reproduce on my side.

Ard Biesheuvel

2017-08-18 15:29 UTC #5 lore raw

PATCH 1/1 arm64: kaslr: Adjust the offset to avoid Image across alignment boundary

Quoted 33 lines · Ard Biesheuvel

Quoted 32 lines · Catalin Marinas
Quoted 31 lines · Ard Biesheuvel
Quoted 29 lines · Catalin Marinas
Quoted 17 lines · Catalin Marinas
With 16KB pages and a kernel Image larger than 16MB, the current kaslr_early_init() logic for avoiding mappings across swapper table boundaries fails since increasing the offset by kimg_sz just moves the problem to the next boundary.
This patch decreases the offset by the boundary overflow amount, with slight risk of reduced entropy as the kernel is more likely to be found at kimg_sz below a swapper table boundary.
Trying-to-fix: afd0e5a87670 ("arm64: kaslr: Fix up the kernel image alignment")
Cc Ard Biesheuvel <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Will Deacon <[email protected]>
Cc Neeraj Upadhyay <[email protected]>
Signed-off-by Catalin Marinas <[email protected]>
---
Quoted 7 lines
While preparing this email, I noticed that the kernel eventually failed to boot, though after a lot more reboot iterations. Mark Rutland also managed to make the KASLR kernel fail to boot with 64K pages which wouldn't be explained by this patch.
So, any suggestions are welcome. My testing method, qemu starting a guest in a loop with virtio-rng-pci.
Apparently, the booting gets much more stable if I disable the physical relocation in arm64-stub.c (but keep the virtual one with the fix in this patch). So I guess we are chasing two different issues.

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
So this is using QEMU with 16k pages support?
Qemu running on a ThunderX, so native KVM support.
Ah ok. I did not realize QEMU supports 16 KB pages in that case. Nice!
However, that makes it rather difficult to reproduce on my side.

Are you booting with an initrd?

Catalin Marinas

author maintainer

2017-08-18 15:33 UTC #6 lore raw

PATCH 1/1 arm64: kaslr: Adjust the offset to avoid Image across alignment boundary

Quoted 34 lines · Ard Biesheuvel

Quoted 33 lines · Ard Biesheuvel
Quoted 32 lines · Catalin Marinas
Quoted 31 lines · Ard Biesheuvel
Quoted 29 lines · Catalin Marinas
Quoted 17 lines · Catalin Marinas
With 16KB pages and a kernel Image larger than 16MB, the current kaslr_early_init() logic for avoiding mappings across swapper table boundaries fails since increasing the offset by kimg_sz just moves the problem to the next boundary.
This patch decreases the offset by the boundary overflow amount, with slight risk of reduced entropy as the kernel is more likely to be found at kimg_sz below a swapper table boundary.
Trying-to-fix: afd0e5a87670 ("arm64: kaslr: Fix up the kernel image alignment")
Cc Ard Biesheuvel <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Will Deacon <[email protected]>
Cc Neeraj Upadhyay <[email protected]>
Signed-off-by Catalin Marinas <[email protected]>
---
Quoted 7 lines
While preparing this email, I noticed that the kernel eventually failed to boot, though after a lot more reboot iterations. Mark Rutland also managed to make the KASLR kernel fail to boot with 64K pages which wouldn't be explained by this patch.
So, any suggestions are welcome. My testing method, qemu starting a guest in a loop with virtio-rng-pci.
Apparently, the booting gets much more stable if I disable the physical relocation in arm64-stub.c (but keep the virtual one with the fix in this patch). So I guess we are chasing two different issues.

Quoted 7 lines

Quoted 7 lines
Quoted 3 lines
Quoted 1 line
So this is using QEMU with 16k pages support?
Qemu running on a ThunderX, so native KVM support.
Ah ok. I did not realize QEMU supports 16 KB pages in that case. Nice!
However, that makes it rather difficult to reproduce on my side.

As I said, we triggered it with 64K pages. Mark R is following up shortly.

Quoted 1 line

Are you booting with an initrd?

No, just:

qemu-system-aarch64 \
	-s \
	-machine virt,accel=kvm,gic_version=3 \
	-smp 1 -m 8192 \
	-cpu $CPU \
	-kernel $IMAGE \
	-semihosting \
	-nographic -serial mon:stdio \
	-monitor tcp:0.0.0.0:4000,server,nowait \
	-netdev user,id=net0 -device virtio-net-device,netdev=net0 \
	-bios /usr/share/qemu-efi/QEMU_EFI.fd \
	-device virtio-rng-pci \
	-append "$CONSOLE $ROOT init=/sbin/poweroff -f"

Signature · 1 line

Catalin

Mark Rutland

reviewer

2017-08-18 15:35 UTC #7 lore raw

PATCH 1/1 arm64: kaslr: Adjust the offset to avoid Image across alignment boundary

In reply to Ard Biesheuvel

Quoted 33 lines · Ard Biesheuvel

Quoted 32 lines · Catalin Marinas
Quoted 31 lines · Ard Biesheuvel
Quoted 29 lines · Catalin Marinas
Quoted 17 lines · Catalin Marinas
With 16KB pages and a kernel Image larger than 16MB, the current kaslr_early_init() logic for avoiding mappings across swapper table boundaries fails since increasing the offset by kimg_sz just moves the problem to the next boundary.
This patch decreases the offset by the boundary overflow amount, with slight risk of reduced entropy as the kernel is more likely to be found at kimg_sz below a swapper table boundary.
Trying-to-fix: afd0e5a87670 ("arm64: kaslr: Fix up the kernel image alignment")
Cc Ard Biesheuvel <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Will Deacon <[email protected]>
Cc Neeraj Upadhyay <[email protected]>
Signed-off-by Catalin Marinas <[email protected]>
---
Quoted 7 lines
While preparing this email, I noticed that the kernel eventually failed to boot, though after a lot more reboot iterations. Mark Rutland also managed to make the KASLR kernel fail to boot with 64K pages which wouldn't be explained by this patch.
So, any suggestions are welcome. My testing method, qemu starting a guest in a loop with virtio-rng-pci.
Apparently, the booting gets much more stable if I disable the physical relocation in arm64-stub.c (but keep the virtual one with the fix in this patch). So I guess we are chasing two different issues.

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
So this is using QEMU with 16k pages support?
Qemu running on a ThunderX, so native KVM support.
Ah ok. I did not realize QEMU supports 16 KB pages in that case. Nice!
However, that makes it rather difficult to reproduce on my side.

FWIW, I was testing with 64K pages, under QEMU+KVM on a SoftIrorn OverDrive 1000 (i.e. a 4-core A57 system).

I'd hacked early_kaslr_init() so that I could override the seed on the command line. I accidentally blatted that hack, but hopefully the below is equivalent.

I had a script that iterated the seed in 2M increments, launch a VM for each seed. The fileststem was configured to power down immediately once it reached userspace.

I found that it hung with seed value: 0x0000000016c00000, which would generate an offset of 0x17e00000. I tried nearby seeds, which worked:

  seed                  offset
  0x0000000016a00000    0x16a00000	worked
  0x0000000016c00000    0x17e00000	failed
  0x0000000016e00000    0x18000000	worked

... but I assume that failing values are dependent on the kernel Image layout.

Thanks,
Mark.

---->8----
t a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c index a9710ef..df50442 100644

arch/arm64/kernel/kaslr.c +8 -2

@@ -102,8 +102,8 @@ u64 __init kaslr_early_init(u64 dt_phys, u64 modulo_offset)
         * Retrieve (and wipe) the seed from the FDT
         */
        seed = get_kaslr_seed(fdt);
-       if (!seed)
-               return 0;
+       //if (!seed)
+       //      return 0;
 
        /*
         * Check if 'nokaslr' appears on the command line, and

@@ -114,6 +114,12 @@ u64 __init kaslr_early_init(u64 dt_phys, u64 modulo_offset)
        if (str == cmdline || (str > cmdline && *(str - 1) == ' '))
                return 0;
 
+       str = strstr(cmdline, "kaslr_seed=")
+       if (str) {
+               str += strlen("kaslr_seed=");
+               seed = simple_strtoull(str, NULL, 16);
+       }
+
        /*
         * OK, so we are proceeding with KASLR enabled. Calculate a suitable
         * kernel image offset from the seed. Let's place the kernel in the