lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOsKWHCCEAi-G=Ld9GJ2YUrbbV6dEzThXh5rOzYp6kWfUHfNHw@mail.gmail.com>
Date: Tue, 27 Aug 2024 12:57:41 +0800
From: Chunyan Zhang <zhangchunyan@...as.ac.cn>
To: Alexandre Ghiti <alexghiti@...osinc.com>
Cc: Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, 
	Albert Ou <aou@...s.berkeley.edu>, Guo Ren <guoren@...nel.org>, 
	Heiko Stuebner <heiko@...ech.de>, Philipp Tomsich <philipp.tomsich@...ll.eu>, 
	linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH -fixes] riscv: Fix RISCV_ALTERNATIVE_EARLY

Hi Alex,

On Mon, 26 Aug 2024 at 18:58, Alexandre Ghiti <alexghiti@...osinc.com> wrote:
>
> RISCV_ALTERNATIVE_EARLY will issue sbi_ecall() very early in the boot
> process, before the first memory mapping is setup so we can't have any
> instrumentation happening here.

I also found that when CONFIG_KASAN is enabled, and either
RISCV_ALTERNATIVE_EARLY or CONFIG_DT_IDLE_GENPD is set, the kernel
cannot boot.

This patch fixed the issue.

Tested-by: Chunyan Zhang <zhangchunyan@...as.ac.cn>

>
> In addition, when the kernel is relocatable, we must also not issue any
> relocation this early since they would have been patched virtually only.
>
> So, instead of disabling instrumentation for the whole kernel/sbi.c file
> and compiling it with -fno-pie, simply move __sbi_ecall() and
> __sbi_base_ecall() into their own file where this is fixed.
>
> Fixes: 1745cfafebdf ("riscv: don't use global static vars to store alternative data")
> Signed-off-by: Alexandre Ghiti <alexghiti@...osinc.com>
> ---
>  arch/riscv/include/asm/sbi.h  |  2 ++
>  arch/riscv/kernel/Makefile    |  6 ++++-
>  arch/riscv/kernel/sbi.c       | 44 --------------------------------
>  arch/riscv/kernel/sbi_ecall.c | 48 +++++++++++++++++++++++++++++++++++
>  4 files changed, 55 insertions(+), 45 deletions(-)
>  create mode 100644 arch/riscv/kernel/sbi_ecall.c
>
> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> index 7cffd4ffecd0..5843a10b380e 100644
> --- a/arch/riscv/include/asm/sbi.h
> +++ b/arch/riscv/include/asm/sbi.h
> @@ -9,6 +9,7 @@
>
>  #include <linux/types.h>
>  #include <linux/cpumask.h>
> +#include <linux/jump_label.h>
>
>  #ifdef CONFIG_RISCV_SBI
>  enum sbi_ext_id {
> @@ -304,6 +305,7 @@ struct sbiret {
>  };
>
>  void sbi_init(void);
> +long __sbi_base_ecall(int fid);
>  struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1,
>                           unsigned long arg2, unsigned long arg3,
>                           unsigned long arg4, unsigned long arg5,
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index 06d407f1b30b..7f88cc4931f5 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -20,17 +20,21 @@ endif
>  ifdef CONFIG_RISCV_ALTERNATIVE_EARLY
>  CFLAGS_alternative.o := -mcmodel=medany
>  CFLAGS_cpufeature.o := -mcmodel=medany
> +CFLAGS_sbi_ecall.o := -mcmodel=medany
>  ifdef CONFIG_FTRACE
>  CFLAGS_REMOVE_alternative.o = $(CC_FLAGS_FTRACE)
>  CFLAGS_REMOVE_cpufeature.o = $(CC_FLAGS_FTRACE)
> +CFLAGS_REMOVE_sbi_ecall.o = $(CC_FLAGS_FTRACE)
>  endif
>  ifdef CONFIG_RELOCATABLE
>  CFLAGS_alternative.o += -fno-pie
>  CFLAGS_cpufeature.o += -fno-pie
> +CFLAGS_sbi_ecall.o += -fno-pie
>  endif
>  ifdef CONFIG_KASAN
>  KASAN_SANITIZE_alternative.o := n
>  KASAN_SANITIZE_cpufeature.o := n
> +KASAN_SANITIZE_sbi_ecall.o := n
>  endif
>  endif
>
> @@ -88,7 +92,7 @@ obj-$(CONFIG_DYNAMIC_FTRACE)  += mcount-dyn.o
>
>  obj-$(CONFIG_PERF_EVENTS)      += perf_callchain.o
>  obj-$(CONFIG_HAVE_PERF_REGS)   += perf_regs.o
> -obj-$(CONFIG_RISCV_SBI)                += sbi.o
> +obj-$(CONFIG_RISCV_SBI)                += sbi.o sbi_ecall.o
>  ifeq ($(CONFIG_RISCV_SBI), y)
>  obj-$(CONFIG_SMP)              += sbi-ipi.o
>  obj-$(CONFIG_SMP) += cpu_ops_sbi.o
> diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c
> index 837bdab2601b..ace9e2f59c41 100644
> --- a/arch/riscv/kernel/sbi.c
> +++ b/arch/riscv/kernel/sbi.c
> @@ -14,9 +14,6 @@
>  #include <asm/smp.h>
>  #include <asm/tlbflush.h>
>
> -#define CREATE_TRACE_POINTS
> -#include <asm/trace.h>
> -
>  /* default SBI version is 0.1 */
>  unsigned long sbi_spec_version __ro_after_init = SBI_SPEC_VERSION_DEFAULT;
>  EXPORT_SYMBOL(sbi_spec_version);
> @@ -27,36 +24,6 @@ static int (*__sbi_rfence)(int fid, const struct cpumask *cpu_mask,
>                            unsigned long start, unsigned long size,
>                            unsigned long arg4, unsigned long arg5) __ro_after_init;
>
> -struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1,
> -                         unsigned long arg2, unsigned long arg3,
> -                         unsigned long arg4, unsigned long arg5,
> -                         int fid, int ext)
> -{
> -       struct sbiret ret;
> -
> -       trace_sbi_call(ext, fid);
> -
> -       register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0);
> -       register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1);
> -       register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2);
> -       register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3);
> -       register uintptr_t a4 asm ("a4") = (uintptr_t)(arg4);
> -       register uintptr_t a5 asm ("a5") = (uintptr_t)(arg5);
> -       register uintptr_t a6 asm ("a6") = (uintptr_t)(fid);
> -       register uintptr_t a7 asm ("a7") = (uintptr_t)(ext);
> -       asm volatile ("ecall"
> -                     : "+r" (a0), "+r" (a1)
> -                     : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7)
> -                     : "memory");
> -       ret.error = a0;
> -       ret.value = a1;
> -
> -       trace_sbi_return(ext, ret.error, ret.value);
> -
> -       return ret;
> -}
> -EXPORT_SYMBOL(__sbi_ecall);
> -
>  int sbi_err_map_linux_errno(int err)
>  {
>         switch (err) {
> @@ -535,17 +502,6 @@ long sbi_probe_extension(int extid)
>  }
>  EXPORT_SYMBOL(sbi_probe_extension);
>
> -static long __sbi_base_ecall(int fid)
> -{
> -       struct sbiret ret;
> -
> -       ret = sbi_ecall(SBI_EXT_BASE, fid, 0, 0, 0, 0, 0, 0);
> -       if (!ret.error)
> -               return ret.value;
> -       else
> -               return sbi_err_map_linux_errno(ret.error);
> -}
> -
>  static inline long sbi_get_spec_version(void)
>  {
>         return __sbi_base_ecall(SBI_EXT_BASE_GET_SPEC_VERSION);
> diff --git a/arch/riscv/kernel/sbi_ecall.c b/arch/riscv/kernel/sbi_ecall.c
> new file mode 100644
> index 000000000000..24aabb4fbde3
> --- /dev/null
> +++ b/arch/riscv/kernel/sbi_ecall.c
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2024 Rivos Inc. */
> +
> +#include <asm/sbi.h>
> +#define CREATE_TRACE_POINTS
> +#include <asm/trace.h>
> +
> +long __sbi_base_ecall(int fid)
> +{
> +       struct sbiret ret;
> +
> +       ret = sbi_ecall(SBI_EXT_BASE, fid, 0, 0, 0, 0, 0, 0);
> +       if (!ret.error)
> +               return ret.value;
> +       else
> +               return sbi_err_map_linux_errno(ret.error);
> +}
> +EXPORT_SYMBOL(__sbi_base_ecall);
> +
> +struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1,
> +                         unsigned long arg2, unsigned long arg3,
> +                         unsigned long arg4, unsigned long arg5,
> +                         int fid, int ext)
> +{
> +       struct sbiret ret;
> +
> +       trace_sbi_call(ext, fid);
> +
> +       register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0);
> +       register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1);
> +       register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2);
> +       register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3);
> +       register uintptr_t a4 asm ("a4") = (uintptr_t)(arg4);
> +       register uintptr_t a5 asm ("a5") = (uintptr_t)(arg5);
> +       register uintptr_t a6 asm ("a6") = (uintptr_t)(fid);
> +       register uintptr_t a7 asm ("a7") = (uintptr_t)(ext);
> +       asm volatile ("ecall"
> +                      : "+r" (a0), "+r" (a1)
> +                      : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7)
> +                      : "memory");
> +       ret.error = a0;
> +       ret.value = a1;
> +
> +       trace_sbi_return(ext, ret.error, ret.value);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL(__sbi_ecall);
> --
> 2.39.2
>
>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ