| Message ID | 20170818150435.35224-1-catalin.marinas@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Aug 18, 2017 at 04:04:34PM +0100, Catalin Marinas wrote: > 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 <ard.biesheuvel@linaro.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Neeraj Upadhyay <neeraju@codeaurora.org> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> > --- > > 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.
On 18 August 2017 at 16:19, Catalin Marinas <catalin.marinas@arm.com> wrote: > On Fri, Aug 18, 2017 at 04:04:34PM +0100, Catalin Marinas wrote: >> 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 <ard.biesheuvel@linaro.org> >> Cc: Mark Rutland <mark.rutland@arm.com> >> Cc: Will Deacon <will.deacon@arm.com> >> Cc: Neeraj Upadhyay <neeraju@codeaurora.org> >> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> >> --- >> >> 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. > So this is using QEMU with 16k pages support?
On Fri, Aug 18, 2017 at 04:20:16PM +0100, Ard Biesheuvel wrote: > On 18 August 2017 at 16:19, Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Fri, Aug 18, 2017 at 04:04:34PM +0100, Catalin Marinas wrote: > >> 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 <ard.biesheuvel@linaro.org> > >> Cc: Mark Rutland <mark.rutland@arm.com> > >> Cc: Will Deacon <will.deacon@arm.com> > >> Cc: Neeraj Upadhyay <neeraju@codeaurora.org> > >> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> > >> --- > >> > >> 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. > > So this is using QEMU with 16k pages support? Qemu running on a ThunderX, so native KVM support.
On 18 August 2017 at 16:22, Catalin Marinas <catalin.marinas@arm.com> wrote: > On Fri, Aug 18, 2017 at 04:20:16PM +0100, Ard Biesheuvel wrote: >> On 18 August 2017 at 16:19, Catalin Marinas <catalin.marinas@arm.com> wrote: >> > On Fri, Aug 18, 2017 at 04:04:34PM +0100, Catalin Marinas wrote: >> >> 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 <ard.biesheuvel@linaro.org> >> >> Cc: Mark Rutland <mark.rutland@arm.com> >> >> Cc: Will Deacon <will.deacon@arm.com> >> >> Cc: Neeraj Upadhyay <neeraju@codeaurora.org> >> >> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> >> >> --- >> >> >> >> 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. >> >> 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.
On 18 August 2017 at 16:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 18 August 2017 at 16:22, Catalin Marinas <catalin.marinas@arm.com> wrote: >> On Fri, Aug 18, 2017 at 04:20:16PM +0100, Ard Biesheuvel wrote: >>> On 18 August 2017 at 16:19, Catalin Marinas <catalin.marinas@arm.com> wrote: >>> > On Fri, Aug 18, 2017 at 04:04:34PM +0100, Catalin Marinas wrote: >>> >> 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 <ard.biesheuvel@linaro.org> >>> >> Cc: Mark Rutland <mark.rutland@arm.com> >>> >> Cc: Will Deacon <will.deacon@arm.com> >>> >> Cc: Neeraj Upadhyay <neeraju@codeaurora.org> >>> >> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> >>> >> --- >>> >> >>> >> 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. >>> >>> 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?
On Fri, Aug 18, 2017 at 04:29:44PM +0100, Ard Biesheuvel wrote: > On 18 August 2017 at 16:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On 18 August 2017 at 16:22, Catalin Marinas <catalin.marinas@arm.com> wrote: > >> On Fri, Aug 18, 2017 at 04:20:16PM +0100, Ard Biesheuvel wrote: > >>> On 18 August 2017 at 16:19, Catalin Marinas <catalin.marinas@arm.com> wrote: > >>> > On Fri, Aug 18, 2017 at 04:04:34PM +0100, Catalin Marinas wrote: > >>> >> 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 <ard.biesheuvel@linaro.org> > >>> >> Cc: Mark Rutland <mark.rutland@arm.com> > >>> >> Cc: Will Deacon <will.deacon@arm.com> > >>> >> Cc: Neeraj Upadhyay <neeraju@codeaurora.org> > >>> >> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> > >>> >> --- > >>> >> > >>> >> 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. > >>> > >>> 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. > 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"
diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c index a9710efb8c01..e8cdc02f66ae 100644 --- a/arch/arm64/kernel/kaslr.c +++ b/arch/arm64/kernel/kaslr.c @@ -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; }
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 <ard.biesheuvel@linaro.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Neeraj Upadhyay <neeraju@codeaurora.org> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> --- 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(-)