Re: [PATCH v6 0/5] riscv: Introduce KASLR

[Song Shuai]

在 2023/8/17 21:10, Alexandre Ghiti 写道:
> Hi Song,
>
> On Tue, Aug 15, 2023 at 1:24 PM Song Shuai <songshuaishuai@xxxxxxxxxxx> wrote:
> > Hi, Alex:
> >
> > 在 2023/7/22 20:38, Alexandre Ghiti 写道:
> > > The following KASLR implementation allows to randomize the kernel mapping:
> > >
> > > - virtually: we expect the bootloader to provide a seed in the device-tree
> > > - physically: only implemented in the EFI stub, it relies on the firmware to
> > >     provide a seed using EFI_RNG_PROTOCOL. arm64 has a similar implementation
> > >     hence the patch 3 factorizes KASLR related functions for riscv to take
> > >     advantage.
> > >
> > > The new virtual kernel location is limited by the early page table that only
> > > has one PUD and with the PMD alignment constraint, the kernel can only take
> > > < 512 positions.
> >
> > I have gone through the code and tested this series with RiscVVirt edk2.
> > All seems good to me, you can add :
> >
> > Tested-by: Song Shuai <songshuaishuai@xxxxxxxxxxx>
> >
> > And a few questions about patch 2 ("riscv: Dump out kernel offset
> > information on panic"):
> >
> > 1. The dump_kernel_offset() function would output "Kernel Offset: 0x0
> > from 0xffffffff80000000" when booting with "nokaslr" option.
> >
> > How about disabling the registration of "kernel_offset_notifier" with
> > "nokaslr" option?
>
> I'd rather keep it as it shows the "nokaslr" flag was taken into account.
>
> > 2. Inspired by patch 2, I added the Crash KASLR support based on this
> > series [1].
> > So is it necessary to keep this patch if we have Crash KASLR support?
>
> I don't understand your question here

Crash can automatically calculate virt_offset by comparing the vmlinux 
and vmcore. If this patch is just intended to assist Crash in setting 
the "--kaslr offset," it might be deleted; if not just keep it. 	

> > [1]:
> > https://lore.kernel.org/linux-riscv/20230815104800.705753-1-songshuaishuai@xxxxxxxxxxx/T/#u
> >
> > > base-commit-tag: v6.5-rc1
> > >
> > > Changes in v6:
> > >     * Fix reintroduced build failures by compiling kaslr.c only for arm64
> > >       and riscv, as suggested by Ard
> > >
> > > Changes in v5:
> > >     * Renamed efi-stub-kaslr.c into kaslr.c and fix commit log of patch 3,
> > >       as suggested by Ard
> > >     * Removed stubs since the kaslr functions were moved to their own file
> > >       (and then does not trigger any build failure for architectures that do
> > >       not call those functions since they are in their own compilation unit)
> > >
> > > Changes in v4:
> > >     * Fix efi_get_kimg macro that returned nothing
> > >     * Moved new kaslr functions into their own files to avoid zboot link
> > >       failures, as suggested by Ard
> > >
> > > Changes in v3:
> > >     * Rebase on top of 6.4-rc2
> > >     * Make RANDOMIZE_BASE depend on 64bit
> > >     * Fix efi_icache_sync and efi_get_kimg_min_align which were undefined
> > >       in x86 (and certainly other archs)
> > >     * Add patch 4 to fix warning on rv32
> > >
> > > Changes in v2:
> > >     * Rebase on top of 6.3-rc1
> > >     * Add a riscv cache sync after memcpying the kernel
> > >     * Add kaslr_offset implementation for KCOV
> > >     * Add forward declaration to quiet LLVM
> > >
> > > Alexandre Ghiti (5):
> > >     riscv: Introduce virtual kernel mapping KASLR
> > >     riscv: Dump out kernel offset information on panic
> > >     arm64: libstub: Move KASLR handling functions to kaslr.c
> > >     libstub: Fix compilation warning for rv32
> > >     riscv: libstub: Implement KASLR by using generic functions
> > >
> > >    arch/arm64/include/asm/efi.h              |   2 +
> > >    arch/riscv/Kconfig                        |  19 +++
> > >    arch/riscv/include/asm/efi.h              |   2 +
> > >    arch/riscv/include/asm/page.h             |   3 +
> > >    arch/riscv/kernel/image-vars.h            |   1 +
> > >    arch/riscv/kernel/pi/Makefile             |   2 +-
> > >    arch/riscv/kernel/pi/cmdline_early.c      |  13 ++
> > >    arch/riscv/kernel/pi/fdt_early.c          |  30 ++++
> > >    arch/riscv/kernel/setup.c                 |  25 ++++
> > >    arch/riscv/mm/init.c                      |  36 ++++-
> > >    drivers/firmware/efi/libstub/Makefile     |   4 +-
> > >    drivers/firmware/efi/libstub/arm64-stub.c | 117 ++--------------
> > >    drivers/firmware/efi/libstub/efistub.h    |   8 ++
> > >    drivers/firmware/efi/libstub/kaslr.c      | 159 ++++++++++++++++++++++
> > >    drivers/firmware/efi/libstub/riscv-stub.c |  33 ++---
> > >    15 files changed, 328 insertions(+), 126 deletions(-)
> > >    create mode 100644 arch/riscv/kernel/pi/fdt_early.c
> > >    create mode 100644 drivers/firmware/efi/libstub/kaslr.c
> >
> > --
> > Thanks
> > Song Shuai
>
> Thanks for testing this and your suggestions!
>
> Alex

--
Thanks
Song Shuai