linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 0/3] riscv: Enable LOCKDEP
@ 2020-06-27 13:57 guoren
  2020-06-27 13:57 ` [PATCH V2 1/3] riscv: Fixup static_obj() fail guoren
                   ` (3 more replies)
  0 siblings, 4 replies; 24+ messages in thread
From: guoren @ 2020-06-27 13:57 UTC (permalink / raw)
  To: palmerdabbelt, paul.walmsley, anup, greentime.hu, zong.li, aou,
	tglx, tycho, nickhu
  Cc: linux-riscv, Guo Ren, linux-kernel, linux-csky

From: Guo Ren <guoren@linux.alibaba.com>

Lockdep is needed by proving the spinlocks and rwlocks. To support it,
we need to add TRACE_IRQFLAGS codes in kernel/entry.S. These patches
follow Documentation/irqflags-tracing.txt.

Fixup 2 bugs that block the lockdep implementation.

---
Changes in v2
 - Remove sX regs recovery codes which are unnecessary, because
   callee will handle them. Thx Greentime :)

 - Move "restore a0 - a7" to handle_syscall, but if _TIF_SYSCALL_WORK
   is set, "restore a1 - a7" is still duplicated. I prefer a C wrapper
   for syscall.

Guo Ren (2):
  riscv: Fixup static_obj() fail
  riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT

Zong Li (1):
  riscv: Fixup lockdep_assert_held with wrong param cpu_running

 arch/riscv/Kconfig              |  3 +++
 arch/riscv/kernel/entry.S       | 33 ++++++++++++++++++++++++++++++++-
 arch/riscv/kernel/smpboot.c     |  1 -
 arch/riscv/kernel/vmlinux.lds.S |  2 +-
 4 files changed, 36 insertions(+), 3 deletions(-)

-- 
2.7.4


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-06-27 13:57 [PATCH V2 0/3] riscv: Enable LOCKDEP guoren
@ 2020-06-27 13:57 ` guoren
  2020-09-11 20:45   ` Aurelien Jarno
  2020-06-27 13:57 ` [PATCH V2 2/3] riscv: Fixup lockdep_assert_held with wrong param cpu_running guoren
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 24+ messages in thread
From: guoren @ 2020-06-27 13:57 UTC (permalink / raw)
  To: palmerdabbelt, paul.walmsley, anup, greentime.hu, zong.li, aou,
	tglx, tycho, nickhu
  Cc: linux-riscv, Guo Ren, linux-kernel, linux-csky

From: Guo Ren <guoren@linux.alibaba.com>

When enable LOCKDEP, static_obj() will cause error. Because some
__initdata static variables is before _stext:

static int static_obj(const void *obj)
{
        unsigned long start = (unsigned long) &_stext,
                      end   = (unsigned long) &_end,
                      addr  = (unsigned long) obj;

        /*
         * static variable?
         */
        if ((addr >= start) && (addr < end))
                return 1;

[    0.067192] INFO: trying to register non-static key.
[    0.067325] the code is fine but needs lockdep annotation.
[    0.067449] turning off the locking correctness validator.
[    0.067718] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.7.0-rc7-dirty #44
[    0.067945] Call Trace:
[    0.068369] [<ffffffe00020323c>] walk_stackframe+0x0/0xa4
[    0.068506] [<ffffffe000203422>] show_stack+0x2a/0x34
[    0.068631] [<ffffffe000521e4e>] dump_stack+0x94/0xca
[    0.068757] [<ffffffe000255a4e>] register_lock_class+0x5b8/0x5bc
[    0.068969] [<ffffffe000255abe>] __lock_acquire+0x6c/0x1d5c
[    0.069101] [<ffffffe0002550fe>] lock_acquire+0xae/0x312
[    0.069228] [<ffffffe000989a8e>] _raw_spin_lock_irqsave+0x40/0x5a
[    0.069357] [<ffffffe000247c64>] complete+0x1e/0x50
[    0.069479] [<ffffffe000984c38>] rest_init+0x1b0/0x28a
[    0.069660] [<ffffffe0000016a2>] 0xffffffe0000016a2
[    0.069779] [<ffffffe000001b84>] 0xffffffe000001b84
[    0.069953] [<ffffffe000001092>] 0xffffffe000001092

static __initdata DECLARE_COMPLETION(kthreadd_done);

noinline void __ref rest_init(void)
{
	...
	complete(&kthreadd_done);

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
---
 arch/riscv/kernel/vmlinux.lds.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
index e6f8016..f3586e3 100644
--- a/arch/riscv/kernel/vmlinux.lds.S
+++ b/arch/riscv/kernel/vmlinux.lds.S
@@ -22,6 +22,7 @@ SECTIONS
 	/* Beginning of code and text segment */
 	. = LOAD_OFFSET;
 	_start = .;
+	_stext = .;
 	HEAD_TEXT_SECTION
 	. = ALIGN(PAGE_SIZE);
 
@@ -54,7 +55,6 @@ SECTIONS
 	. = ALIGN(SECTION_ALIGN);
 	.text : {
 		_text = .;
-		_stext = .;
 		TEXT_TEXT
 		SCHED_TEXT
 		CPUIDLE_TEXT
-- 
2.7.4


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH V2 2/3] riscv: Fixup lockdep_assert_held with wrong param cpu_running
  2020-06-27 13:57 [PATCH V2 0/3] riscv: Enable LOCKDEP guoren
  2020-06-27 13:57 ` [PATCH V2 1/3] riscv: Fixup static_obj() fail guoren
@ 2020-06-27 13:57 ` guoren
  2020-09-29 22:12   ` Atish Patra
  2020-06-27 13:57 ` [PATCH V2 3/3] riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT guoren
  2020-07-09 22:06 ` [PATCH V2 0/3] riscv: Enable LOCKDEP Palmer Dabbelt
  3 siblings, 1 reply; 24+ messages in thread
From: guoren @ 2020-06-27 13:57 UTC (permalink / raw)
  To: palmerdabbelt, paul.walmsley, anup, greentime.hu, zong.li, aou,
	tglx, tycho, nickhu
  Cc: linux-riscv, Guo Ren, linux-kernel, linux-csky

From: Zong Li <zong.li@sifive.com>

The cpu_running is not a lock-class, it lacks the dep_map member in
completion. It causes the error as follow:

arch/riscv/kernel/smpboot.c: In function '__cpu_up':
./include/linux/lockdep.h:364:52: error: 'struct completion' has no member named 'dep_map'
  364 | #define lockdep_is_held(lock)  lock_is_held(&(lock)->dep_map)
      |                                                    ^~
./include/asm-generic/bug.h:113:25: note: in definition of macro 'WARN_ON'
  113 |  int __ret_warn_on = !!(condition);    \
      |                         ^~~~~~~~~
./include/linux/lockdep.h:390:27: note: in expansion of macro 'lockdep_is_held'
  390 |   WARN_ON(debug_locks && !lockdep_is_held(l)); \
      |                           ^~~~~~~~~~~~~~~
arch/riscv/kernel/smpboot.c:118:2: note: in expansion of macro 'lockdep_assert_held'
  118 |  lockdep_assert_held(&cpu_running);

There are a lot of archs which use cpu_running in smpboot.c (arm,
arm64, openrisc, xtensa, s390, x86, mips), but none of them try
lockdep_assert_held(&cpu_running.wait.lock). So Just remove it.

Signed-off-by: Zong Li <zong.li@sifive.com>
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
---
 arch/riscv/kernel/smpboot.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 4e99227..defc4e1 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -121,7 +121,6 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle)
 
 	ret = start_secondary_cpu(cpu, tidle);
 	if (!ret) {
-		lockdep_assert_held(&cpu_running);
 		wait_for_completion_timeout(&cpu_running,
 					    msecs_to_jiffies(1000));
 
-- 
2.7.4


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH V2 3/3] riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT
  2020-06-27 13:57 [PATCH V2 0/3] riscv: Enable LOCKDEP guoren
  2020-06-27 13:57 ` [PATCH V2 1/3] riscv: Fixup static_obj() fail guoren
  2020-06-27 13:57 ` [PATCH V2 2/3] riscv: Fixup lockdep_assert_held with wrong param cpu_running guoren
@ 2020-06-27 13:57 ` guoren
  2020-07-09 22:06 ` [PATCH V2 0/3] riscv: Enable LOCKDEP Palmer Dabbelt
  3 siblings, 0 replies; 24+ messages in thread
From: guoren @ 2020-06-27 13:57 UTC (permalink / raw)
  To: palmerdabbelt, paul.walmsley, anup, greentime.hu, zong.li, aou,
	tglx, tycho, nickhu
  Cc: linux-riscv, Guo Ren, linux-kernel, linux-csky

From: Guo Ren <guoren@linux.alibaba.com>

Lockdep is needed by proving the spinlocks and rwlocks. To suupport
it, we need fixup TRACE_IRQFLAGS_SUPPORT in kernel/entry.S. This
patch follow Documentation/irqflags-tracing.txt.

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Greentime Hu <greentime.hu@sifive.com>
---
 arch/riscv/Kconfig        |  3 +++
 arch/riscv/kernel/entry.S | 34 +++++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 58d6f66..d5d5100 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -179,6 +179,9 @@ config PGTABLE_LEVELS
 	default 3 if 64BIT
 	default 2
 
+config LOCKDEP_SUPPORT
+	def_bool y
+
 source "arch/riscv/Kconfig.socs"
 
 menu "Platform type"
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index cae7e6d..45c81e4 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -97,19 +97,26 @@ _save_context:
 	la gp, __global_pointer$
 .option pop
 
-	la ra, ret_from_exception
+#ifdef CONFIG_TRACE_IRQFLAGS
+	call trace_hardirqs_off
+#endif
 	/*
 	 * MSB of cause differentiates between
 	 * interrupts and exceptions
 	 */
 	bge s4, zero, 1f
 
+	la ra, ret_from_exception
+
 	/* Handle interrupts */
 	move a0, sp /* pt_regs */
 	la a1, handle_arch_irq
 	REG_L a1, (a1)
 	jr a1
 1:
+#ifdef CONFIG_TRACE_IRQFLAGS
+	call trace_hardirqs_on
+#endif
 	/*
 	 * Exceptions run with interrupts enabled or disabled depending on the
 	 * state of SR_PIE in m/sstatus.
@@ -119,6 +126,7 @@ _save_context:
 	csrs CSR_STATUS, SR_IE
 
 1:
+	la ra, ret_from_exception
 	/* Handle syscalls */
 	li t0, EXC_SYSCALL
 	beq s4, t0, handle_syscall
@@ -137,6 +145,17 @@ _save_context:
 	tail do_trap_unknown
 
 handle_syscall:
+#ifdef CONFIG_TRACE_IRQFLAGS
+	/* Recover a0 - a7 for system calls */
+	REG_L a0, PT_A0(sp)
+	REG_L a1, PT_A1(sp)
+	REG_L a2, PT_A2(sp)
+	REG_L a3, PT_A3(sp)
+	REG_L a4, PT_A4(sp)
+	REG_L a5, PT_A5(sp)
+	REG_L a6, PT_A6(sp)
+	REG_L a7, PT_A7(sp)
+#endif
 	 /* save the initial A0 value (needed in signal handlers) */
 	REG_S a0, PT_ORIG_A0(sp)
 	/*
@@ -190,6 +209,9 @@ ret_from_syscall_rejected:
 ret_from_exception:
 	REG_L s0, PT_STATUS(sp)
 	csrc CSR_STATUS, SR_IE
+#ifdef CONFIG_TRACE_IRQFLAGS
+	call trace_hardirqs_off
+#endif
 #ifdef CONFIG_RISCV_M_MODE
 	/* the MPP value is too large to be used as an immediate arg for addi */
 	li t0, SR_MPP
@@ -216,6 +238,16 @@ resume_userspace:
 	csrw CSR_SCRATCH, tp
 
 restore_all:
+#ifdef CONFIG_TRACE_IRQFLAGS
+	REG_L s1, PT_STATUS(sp)
+	andi t0, s1, SR_PIE
+	beqz t0, 1f
+	call trace_hardirqs_on
+	j 2f
+1:
+	call trace_hardirqs_off
+2:
+#endif
 	REG_L a0, PT_STATUS(sp)
 	/*
 	 * The current load reservation is effectively part of the processor's
-- 
2.7.4


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 0/3] riscv: Enable LOCKDEP
  2020-06-27 13:57 [PATCH V2 0/3] riscv: Enable LOCKDEP guoren
                   ` (2 preceding siblings ...)
  2020-06-27 13:57 ` [PATCH V2 3/3] riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT guoren
@ 2020-07-09 22:06 ` Palmer Dabbelt
  2020-07-09 23:15   ` Guo Ren
  3 siblings, 1 reply; 24+ messages in thread
From: Palmer Dabbelt @ 2020-07-09 22:06 UTC (permalink / raw)
  To: guoren
  Cc: tycho, aou, nickhu, anup, linux-kernel, linux-csky, guoren,
	zong.li, Paul Walmsley, greentime.hu, tglx, linux-riscv

On Sat, 27 Jun 2020 06:57:05 PDT (-0700), guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Lockdep is needed by proving the spinlocks and rwlocks. To support it,
> we need to add TRACE_IRQFLAGS codes in kernel/entry.S. These patches
> follow Documentation/irqflags-tracing.txt.
>
> Fixup 2 bugs that block the lockdep implementation.
>
> ---
> Changes in v2
>  - Remove sX regs recovery codes which are unnecessary, because
>    callee will handle them. Thx Greentime :)
>
>  - Move "restore a0 - a7" to handle_syscall, but if _TIF_SYSCALL_WORK
>    is set, "restore a1 - a7" is still duplicated. I prefer a C wrapper
>    for syscall.
>
> Guo Ren (2):
>   riscv: Fixup static_obj() fail
>   riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT
>
> Zong Li (1):
>   riscv: Fixup lockdep_assert_held with wrong param cpu_running
>
>  arch/riscv/Kconfig              |  3 +++
>  arch/riscv/kernel/entry.S       | 33 ++++++++++++++++++++++++++++++++-
>  arch/riscv/kernel/smpboot.c     |  1 -
>  arch/riscv/kernel/vmlinux.lds.S |  2 +-
>  4 files changed, 36 insertions(+), 3 deletions(-)

These are on for-next.  As far as I can tell lockdep is working, but I'm just
doing some simple boot tests.

Thanks!

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 0/3] riscv: Enable LOCKDEP
  2020-07-09 22:06 ` [PATCH V2 0/3] riscv: Enable LOCKDEP Palmer Dabbelt
@ 2020-07-09 23:15   ` Guo Ren
  0 siblings, 0 replies; 24+ messages in thread
From: Guo Ren @ 2020-07-09 23:15 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel,
	Linux Kernel Mailing List, linux-csky, Guo Ren, Zong Li,
	Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv

Thank you, Palmer

On Fri, Jul 10, 2020 at 6:06 AM Palmer Dabbelt <palmerdabbelt@google.com> wrote:
>
> On Sat, 27 Jun 2020 06:57:05 PDT (-0700), guoren@kernel.org wrote:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Lockdep is needed by proving the spinlocks and rwlocks. To support it,
> > we need to add TRACE_IRQFLAGS codes in kernel/entry.S. These patches
> > follow Documentation/irqflags-tracing.txt.
> >
> > Fixup 2 bugs that block the lockdep implementation.
> >
> > ---
> > Changes in v2
> >  - Remove sX regs recovery codes which are unnecessary, because
> >    callee will handle them. Thx Greentime :)
> >
> >  - Move "restore a0 - a7" to handle_syscall, but if _TIF_SYSCALL_WORK
> >    is set, "restore a1 - a7" is still duplicated. I prefer a C wrapper
> >    for syscall.
> >
> > Guo Ren (2):
> >   riscv: Fixup static_obj() fail
> >   riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT
> >
> > Zong Li (1):
> >   riscv: Fixup lockdep_assert_held with wrong param cpu_running
> >
> >  arch/riscv/Kconfig              |  3 +++
> >  arch/riscv/kernel/entry.S       | 33 ++++++++++++++++++++++++++++++++-
> >  arch/riscv/kernel/smpboot.c     |  1 -
> >  arch/riscv/kernel/vmlinux.lds.S |  2 +-
> >  4 files changed, 36 insertions(+), 3 deletions(-)
>
> These are on for-next.  As far as I can tell lockdep is working, but I'm just
> doing some simple boot tests.
>
> Thanks!



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-06-27 13:57 ` [PATCH V2 1/3] riscv: Fixup static_obj() fail guoren
@ 2020-09-11 20:45   ` Aurelien Jarno
  2020-09-12  2:39     ` Guo Ren
  0 siblings, 1 reply; 24+ messages in thread
From: Aurelien Jarno @ 2020-09-11 20:45 UTC (permalink / raw)
  To: guoren
  Cc: tycho, aou, nickhu, anup, palmerdabbelt, linux-kernel,
	linux-csky, Guo Ren, zong.li, paul.walmsley, greentime.hu, tglx,
	linux-riscv

Hi,

On 2020-06-27 13:57, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
> 
> When enable LOCKDEP, static_obj() will cause error. Because some
> __initdata static variables is before _stext:
> 
> static int static_obj(const void *obj)
> {
>         unsigned long start = (unsigned long) &_stext,
>                       end   = (unsigned long) &_end,
>                       addr  = (unsigned long) obj;
> 
>         /*
>          * static variable?
>          */
>         if ((addr >= start) && (addr < end))
>                 return 1;
> 
> [    0.067192] INFO: trying to register non-static key.
> [    0.067325] the code is fine but needs lockdep annotation.
> [    0.067449] turning off the locking correctness validator.
> [    0.067718] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.7.0-rc7-dirty #44
> [    0.067945] Call Trace:
> [    0.068369] [<ffffffe00020323c>] walk_stackframe+0x0/0xa4
> [    0.068506] [<ffffffe000203422>] show_stack+0x2a/0x34
> [    0.068631] [<ffffffe000521e4e>] dump_stack+0x94/0xca
> [    0.068757] [<ffffffe000255a4e>] register_lock_class+0x5b8/0x5bc
> [    0.068969] [<ffffffe000255abe>] __lock_acquire+0x6c/0x1d5c
> [    0.069101] [<ffffffe0002550fe>] lock_acquire+0xae/0x312
> [    0.069228] [<ffffffe000989a8e>] _raw_spin_lock_irqsave+0x40/0x5a
> [    0.069357] [<ffffffe000247c64>] complete+0x1e/0x50
> [    0.069479] [<ffffffe000984c38>] rest_init+0x1b0/0x28a
> [    0.069660] [<ffffffe0000016a2>] 0xffffffe0000016a2
> [    0.069779] [<ffffffe000001b84>] 0xffffffe000001b84
> [    0.069953] [<ffffffe000001092>] 0xffffffe000001092
> 
> static __initdata DECLARE_COMPLETION(kthreadd_done);
> 
> noinline void __ref rest_init(void)
> {
> 	...
> 	complete(&kthreadd_done);
> 
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> ---
>  arch/riscv/kernel/vmlinux.lds.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> index e6f8016..f3586e3 100644
> --- a/arch/riscv/kernel/vmlinux.lds.S
> +++ b/arch/riscv/kernel/vmlinux.lds.S
> @@ -22,6 +22,7 @@ SECTIONS
>  	/* Beginning of code and text segment */
>  	. = LOAD_OFFSET;
>  	_start = .;
> +	_stext = .;
>  	HEAD_TEXT_SECTION
>  	. = ALIGN(PAGE_SIZE);
>  
> @@ -54,7 +55,6 @@ SECTIONS
>  	. = ALIGN(SECTION_ALIGN);
>  	.text : {
>  		_text = .;
> -		_stext = .;
>  		TEXT_TEXT
>  		SCHED_TEXT
>  		CPUIDLE_TEXT


This patch has been backported to kernel 5.8.4. This causes the kernel
to crash when trying to execute the init process:

[    3.484586] AppArmor: AppArmor sha1 policy hashing enabled
[    4.749835] Freeing unused kernel memory: 492K
[    4.752017] Run /init as init process
[    4.753571] usercopy: Kernel memory overwrite attempt detected to kernel text (offset 507879, size 11)!
[    4.754838] ------------[ cut here ]------------
[    4.755651] kernel BUG at mm/usercopy.c:99!
[    4.756445] Kernel BUG [#1]
[    4.756815] Modules linked in:
[    4.757542] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.8.0-1-riscv64 #1 Debian 5.8.7-1
[    4.758372] epc: ffffffe0003b5120 ra : ffffffe0003b5120 sp : ffffffe07f783ca0
[    4.758960]  gp : ffffffe000cc7230 tp : ffffffe07f77cec0 t0 : ffffffe000cdafc0
[    4.759772]  t1 : 0000000000000064 t2 : 0000000000000000 s0 : ffffffe07f783cf0
[    4.760534]  s1 : ffffffe00095d780 a0 : 000000000000005b a1 : 0000000000000020
[    4.761309]  a2 : 0000000000000005 a3 : 0000000000000000 a4 : ffffffe000c1f340
[    4.761848]  a5 : ffffffe000c1f340 a6 : 0000000000000000 a7 : 0000000000000087
[    4.762684]  s2 : ffffffe000941848 s3 : 000000000007bfe7 s4 : 000000000000000b
[    4.763500]  s5 : 0000000000000000 s6 : ffffffe00091cc00 s7 : fffffffffffff000
[    4.764376]  s8 : 0000003ffffff000 s9 : ffffffe0769f3200 s10: 000000000000000b
[    4.765208]  s11: ffffffe07d548c40 t3 : 0000000000000000 t4 : 000000000001dcd0
[    4.766059]  t5 : ffffffe000cc8510 t6 : ffffffe000cd64aa
[    4.766712] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003
[    4.768308] ---[ end trace 1f8e733e834d4c3e ]---
[    4.769129] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[    4.770070] SMP: stopping secondary CPUs
[    4.771110] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---

Note that this is with CONFIG_HARDENED_USERCOPY=y

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-09-11 20:45   ` Aurelien Jarno
@ 2020-09-12  2:39     ` Guo Ren
  2020-09-14 10:38       ` Aurelien Jarno
  0 siblings, 1 reply; 24+ messages in thread
From: Guo Ren @ 2020-09-12  2:39 UTC (permalink / raw)
  To: Guo Ren, Palmer Dabbelt, Paul Walmsley, Anup Patel, Greentime Hu,
	Zong Li, Albert Ou, Thomas Gleixner, Tycho Andersen, Nick Hu,
	linux-riscv, Guo Ren, Linux Kernel Mailing List, linux-csky

It's come from mm/usercopy.c
/* Is this address range in the kernel text area? */
static inline void check_kernel_text_object(const unsigned long ptr,
                                            unsigned long n, bool to_user)
{
        unsigned long textlow = (unsigned long)_stext;
        unsigned long texthigh = (unsigned long)_etext;
        unsigned long textlow_linear, texthigh_linear;

        if (overlaps(ptr, n, textlow, texthigh))
                usercopy_abort("kernel text", NULL, to_user, ptr - textlow, n);

The __init_text/data areas will be freed after bootup, so I think it should be:
-        unsigned long textlow = (unsigned long)_stext;
+        unsigned long textlow = (unsigned long)_text;

That means _stext should include init_text/data and _text is only for freeable.


On Sat, Sep 12, 2020 at 5:01 AM Aurelien Jarno <aurelien@aurel32.net> wrote:
>
> Hi,
>
> On 2020-06-27 13:57, guoren@kernel.org wrote:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > When enable LOCKDEP, static_obj() will cause error. Because some
> > __initdata static variables is before _stext:
> >
> > static int static_obj(const void *obj)
> > {
> >         unsigned long start = (unsigned long) &_stext,
> >                       end   = (unsigned long) &_end,
> >                       addr  = (unsigned long) obj;
> >
> >         /*
> >          * static variable?
> >          */
> >         if ((addr >= start) && (addr < end))
> >                 return 1;
> >
> > [    0.067192] INFO: trying to register non-static key.
> > [    0.067325] the code is fine but needs lockdep annotation.
> > [    0.067449] turning off the locking correctness validator.
> > [    0.067718] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.7.0-rc7-dirty #44
> > [    0.067945] Call Trace:
> > [    0.068369] [<ffffffe00020323c>] walk_stackframe+0x0/0xa4
> > [    0.068506] [<ffffffe000203422>] show_stack+0x2a/0x34
> > [    0.068631] [<ffffffe000521e4e>] dump_stack+0x94/0xca
> > [    0.068757] [<ffffffe000255a4e>] register_lock_class+0x5b8/0x5bc
> > [    0.068969] [<ffffffe000255abe>] __lock_acquire+0x6c/0x1d5c
> > [    0.069101] [<ffffffe0002550fe>] lock_acquire+0xae/0x312
> > [    0.069228] [<ffffffe000989a8e>] _raw_spin_lock_irqsave+0x40/0x5a
> > [    0.069357] [<ffffffe000247c64>] complete+0x1e/0x50
> > [    0.069479] [<ffffffe000984c38>] rest_init+0x1b0/0x28a
> > [    0.069660] [<ffffffe0000016a2>] 0xffffffe0000016a2
> > [    0.069779] [<ffffffe000001b84>] 0xffffffe000001b84
> > [    0.069953] [<ffffffe000001092>] 0xffffffe000001092
> >
> > static __initdata DECLARE_COMPLETION(kthreadd_done);
> >
> > noinline void __ref rest_init(void)
> > {
> >       ...
> >       complete(&kthreadd_done);
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > ---
> >  arch/riscv/kernel/vmlinux.lds.S | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > index e6f8016..f3586e3 100644
> > --- a/arch/riscv/kernel/vmlinux.lds.S
> > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > @@ -22,6 +22,7 @@ SECTIONS
> >       /* Beginning of code and text segment */
> >       . = LOAD_OFFSET;
> >       _start = .;
> > +     _stext = .;
> >       HEAD_TEXT_SECTION
> >       . = ALIGN(PAGE_SIZE);
> >
> > @@ -54,7 +55,6 @@ SECTIONS
> >       . = ALIGN(SECTION_ALIGN);
> >       .text : {
> >               _text = .;
> > -             _stext = .;
> >               TEXT_TEXT
> >               SCHED_TEXT
> >               CPUIDLE_TEXT
>
>
> This patch has been backported to kernel 5.8.4. This causes the kernel
> to crash when trying to execute the init process:
>
> [    3.484586] AppArmor: AppArmor sha1 policy hashing enabled
> [    4.749835] Freeing unused kernel memory: 492K
> [    4.752017] Run /init as init process
> [    4.753571] usercopy: Kernel memory overwrite attempt detected to kernel text (offset 507879, size 11)!
> [    4.754838] ------------[ cut here ]------------
> [    4.755651] kernel BUG at mm/usercopy.c:99!
> [    4.756445] Kernel BUG [#1]
> [    4.756815] Modules linked in:
> [    4.757542] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.8.0-1-riscv64 #1 Debian 5.8.7-1
> [    4.758372] epc: ffffffe0003b5120 ra : ffffffe0003b5120 sp : ffffffe07f783ca0
> [    4.758960]  gp : ffffffe000cc7230 tp : ffffffe07f77cec0 t0 : ffffffe000cdafc0
> [    4.759772]  t1 : 0000000000000064 t2 : 0000000000000000 s0 : ffffffe07f783cf0
> [    4.760534]  s1 : ffffffe00095d780 a0 : 000000000000005b a1 : 0000000000000020
> [    4.761309]  a2 : 0000000000000005 a3 : 0000000000000000 a4 : ffffffe000c1f340
> [    4.761848]  a5 : ffffffe000c1f340 a6 : 0000000000000000 a7 : 0000000000000087
> [    4.762684]  s2 : ffffffe000941848 s3 : 000000000007bfe7 s4 : 000000000000000b
> [    4.763500]  s5 : 0000000000000000 s6 : ffffffe00091cc00 s7 : fffffffffffff000
> [    4.764376]  s8 : 0000003ffffff000 s9 : ffffffe0769f3200 s10: 000000000000000b
> [    4.765208]  s11: ffffffe07d548c40 t3 : 0000000000000000 t4 : 000000000001dcd0
> [    4.766059]  t5 : ffffffe000cc8510 t6 : ffffffe000cd64aa
> [    4.766712] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003
> [    4.768308] ---[ end trace 1f8e733e834d4c3e ]---
> [    4.769129] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> [    4.770070] SMP: stopping secondary CPUs
> [    4.771110] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
>
> Note that this is with CONFIG_HARDENED_USERCOPY=y
>
> Aurelien
>
> --
> Aurelien Jarno                          GPG: 4096R/1DDD8C9B
> aurelien@aurel32.net                 http://www.aurel32.net



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-09-12  2:39     ` Guo Ren
@ 2020-09-14 10:38       ` Aurelien Jarno
  2020-09-24  7:36         ` Andreas Schwab
  2020-10-05  8:25         ` Andreas Schwab
  0 siblings, 2 replies; 24+ messages in thread
From: Aurelien Jarno @ 2020-09-14 10:38 UTC (permalink / raw)
  To: Guo Ren
  Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt,
	Linux Kernel Mailing List, linux-csky, Guo Ren, Zong Li,
	Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv

On 2020-09-12 10:39, Guo Ren wrote:
> It's come from mm/usercopy.c
> /* Is this address range in the kernel text area? */
> static inline void check_kernel_text_object(const unsigned long ptr,
>                                             unsigned long n, bool to_user)
> {
>         unsigned long textlow = (unsigned long)_stext;
>         unsigned long texthigh = (unsigned long)_etext;
>         unsigned long textlow_linear, texthigh_linear;
> 
>         if (overlaps(ptr, n, textlow, texthigh))
>                 usercopy_abort("kernel text", NULL, to_user, ptr - textlow, n);
> 
> The __init_text/data areas will be freed after bootup, so I think it should be:
> -        unsigned long textlow = (unsigned long)_stext;
> +        unsigned long textlow = (unsigned long)_text;
> 
> That means _stext should include init_text/data and _text is only for freeable.

I have no idea if it is the right thing to do or not, but I can confirm
this fixes the issue.

How should we proceed to get that fixed in time for 5.9? For the older
branches where it has been backported (so far 5.7 and 5.8), should we
just get that commit reverted instead?

Thanks,
Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-09-14 10:38       ` Aurelien Jarno
@ 2020-09-24  7:36         ` Andreas Schwab
  2020-09-24 16:19           ` Guo Ren
  2020-10-05  8:25         ` Andreas Schwab
  1 sibling, 1 reply; 24+ messages in thread
From: Andreas Schwab @ 2020-09-24  7:36 UTC (permalink / raw)
  To: Guo Ren
  Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt,
	Linux Kernel Mailing List, linux-csky, Guo Ren, Zong Li,
	Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv

On Sep 14 2020, Aurelien Jarno wrote:

> How should we proceed to get that fixed in time for 5.9? For the older
> branches where it has been backported (so far 5.7 and 5.8), should we
> just get that commit reverted instead?

Can this please be resolved ASAP?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-09-24  7:36         ` Andreas Schwab
@ 2020-09-24 16:19           ` Guo Ren
  2020-09-29 18:51             ` Aurelien Jarno
  2020-10-05 19:14             ` Atish Patra
  0 siblings, 2 replies; 24+ messages in thread
From: Guo Ren @ 2020-09-24 16:19 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt,
	Linux Kernel Mailing List, linux-csky, Guo Ren, Zong Li,
	Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv

How about this, revert the commit and don't free INIT_DATA_SECTION. I
think the solution is safe enough, but wast a little memory.

diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
index f3586e3..34d00d9 100644
--- a/arch/riscv/kernel/vmlinux.lds.S
+++ b/arch/riscv/kernel/vmlinux.lds.S
@@ -22,13 +22,11 @@ SECTIONS
        /* Beginning of code and text segment */
        . = LOAD_OFFSET;
        _start = .;
-       _stext = .;
        HEAD_TEXT_SECTION
        . = ALIGN(PAGE_SIZE);

        __init_begin = .;
        INIT_TEXT_SECTION(PAGE_SIZE)
-       INIT_DATA_SECTION(16)
        . = ALIGN(8);
        __soc_early_init_table : {
                __soc_early_init_table_start = .;
@@ -55,6 +53,7 @@ SECTIONS
        . = ALIGN(SECTION_ALIGN);
        .text : {
                _text = .;
+               _stext = .;
                TEXT_TEXT
                SCHED_TEXT
                CPUIDLE_TEXT
@@ -67,6 +66,8 @@ SECTIONS
                _etext = .;
        }

+       INIT_DATA_SECTION(16)
+
        /* Start of data section */
        _sdata = .;
        RO_DATA(SECTION_ALIGN)

On Thu, Sep 24, 2020 at 3:36 PM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Sep 14 2020, Aurelien Jarno wrote:
>
> > How should we proceed to get that fixed in time for 5.9? For the older
> > branches where it has been backported (so far 5.7 and 5.8), should we
> > just get that commit reverted instead?
>
> Can this please be resolved ASAP?
>
> Andreas.
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> "And now for something completely different."



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-09-24 16:19           ` Guo Ren
@ 2020-09-29 18:51             ` Aurelien Jarno
  2020-10-05 19:14             ` Atish Patra
  1 sibling, 0 replies; 24+ messages in thread
From: Aurelien Jarno @ 2020-09-29 18:51 UTC (permalink / raw)
  To: Guo Ren
  Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt,
	Linux Kernel Mailing List, linux-csky, Guo Ren, Andreas Schwab,
	Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner,
	linux-riscv

Hi,

On 2020-09-25 00:19, Guo Ren wrote:
> How about this, revert the commit and don't free INIT_DATA_SECTION. I
> think the solution is safe enough, but wast a little memory.
> 
> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> index f3586e3..34d00d9 100644
> --- a/arch/riscv/kernel/vmlinux.lds.S
> +++ b/arch/riscv/kernel/vmlinux.lds.S
> @@ -22,13 +22,11 @@ SECTIONS
>         /* Beginning of code and text segment */
>         . = LOAD_OFFSET;
>         _start = .;
> -       _stext = .;
>         HEAD_TEXT_SECTION
>         . = ALIGN(PAGE_SIZE);
> 
>         __init_begin = .;
>         INIT_TEXT_SECTION(PAGE_SIZE)
> -       INIT_DATA_SECTION(16)
>         . = ALIGN(8);
>         __soc_early_init_table : {
>                 __soc_early_init_table_start = .;
> @@ -55,6 +53,7 @@ SECTIONS
>         . = ALIGN(SECTION_ALIGN);
>         .text : {
>                 _text = .;
> +               _stext = .;
>                 TEXT_TEXT
>                 SCHED_TEXT
>                 CPUIDLE_TEXT
> @@ -67,6 +66,8 @@ SECTIONS
>                 _etext = .;
>         }
> 
> +       INIT_DATA_SECTION(16)
> +
>         /* Start of data section */
>         _sdata = .;
>         RO_DATA(SECTION_ALIGN)
> 

This patch doesn't apply, as tabs have been converted to space
somewhere. After fixing that, the patch applies and I confirm that it
fixes the problem.

Tested-by: Aurelien Jarno <aurelien@aurel32.net>

Thanks,
Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 2/3] riscv: Fixup lockdep_assert_held with wrong param cpu_running
  2020-06-27 13:57 ` [PATCH V2 2/3] riscv: Fixup lockdep_assert_held with wrong param cpu_running guoren
@ 2020-09-29 22:12   ` Atish Patra
  0 siblings, 0 replies; 24+ messages in thread
From: Atish Patra @ 2020-09-29 22:12 UTC (permalink / raw)
  To: Guo Ren
  Cc: tycho, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt,
	linux-kernel@vger.kernel.org List, linux-csky, Guo Ren, Zong Li,
	Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv

On Sat, Jun 27, 2020 at 6:58 AM <guoren@kernel.org> wrote:
>
> From: Zong Li <zong.li@sifive.com>
>
> The cpu_running is not a lock-class, it lacks the dep_map member in
> completion. It causes the error as follow:
>
> arch/riscv/kernel/smpboot.c: In function '__cpu_up':
> ./include/linux/lockdep.h:364:52: error: 'struct completion' has no member named 'dep_map'
>   364 | #define lockdep_is_held(lock)  lock_is_held(&(lock)->dep_map)
>       |                                                    ^~
> ./include/asm-generic/bug.h:113:25: note: in definition of macro 'WARN_ON'
>   113 |  int __ret_warn_on = !!(condition);    \
>       |                         ^~~~~~~~~
> ./include/linux/lockdep.h:390:27: note: in expansion of macro 'lockdep_is_held'
>   390 |   WARN_ON(debug_locks && !lockdep_is_held(l)); \
>       |                           ^~~~~~~~~~~~~~~
> arch/riscv/kernel/smpboot.c:118:2: note: in expansion of macro 'lockdep_assert_held'
>   118 |  lockdep_assert_held(&cpu_running);
>
> There are a lot of archs which use cpu_running in smpboot.c (arm,
> arm64, openrisc, xtensa, s390, x86, mips), but none of them try
> lockdep_assert_held(&cpu_running.wait.lock). So Just remove it.
>
> Signed-off-by: Zong Li <zong.li@sifive.com>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> ---
>  arch/riscv/kernel/smpboot.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 4e99227..defc4e1 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -121,7 +121,6 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle)
>
>         ret = start_secondary_cpu(cpu, tidle);
>         if (!ret) {
> -               lockdep_assert_held(&cpu_running);
>                 wait_for_completion_timeout(&cpu_running,
>                                             msecs_to_jiffies(1000));
>
> --
> 2.7.4
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Thanks for catching this.

Reviewed-by: Atish Patra <atish.patra@wdc.com>

-- 
Regards,
Atish

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-09-14 10:38       ` Aurelien Jarno
  2020-09-24  7:36         ` Andreas Schwab
@ 2020-10-05  8:25         ` Andreas Schwab
  2020-10-05 16:39           ` Palmer Dabbelt
  1 sibling, 1 reply; 24+ messages in thread
From: Andreas Schwab @ 2020-10-05  8:25 UTC (permalink / raw)
  To: Guo Ren
  Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt,
	Linux Kernel Mailing List, linux-csky, Guo Ren, Zong Li,
	Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv

On Sep 14 2020, Aurelien Jarno wrote:

> How should we proceed to get that fixed in time for 5.9? For the older
> branches where it has been backported (so far 5.7 and 5.8), should we
> just get that commit reverted instead?

Why is this still broken?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-10-05  8:25         ` Andreas Schwab
@ 2020-10-05 16:39           ` Palmer Dabbelt
  2020-10-05 18:40             ` Andreas Schwab
  2020-10-06 16:55             ` Guo Ren
  0 siblings, 2 replies; 24+ messages in thread
From: Palmer Dabbelt @ 2020-10-05 16:39 UTC (permalink / raw)
  To: schwab
  Cc: tycho, aou, nickhu, anup, linux-kernel, linux-csky, guoren,
	guoren, zong.li, Paul Walmsley, greentime.hu, tglx, linux-riscv

On Mon, 05 Oct 2020 01:25:22 PDT (-0700), schwab@linux-m68k.org wrote:
> On Sep 14 2020, Aurelien Jarno wrote:
>
>> How should we proceed to get that fixed in time for 5.9? For the older
>> branches where it has been backported (so far 5.7 and 5.8), should we
>> just get that commit reverted instead?
>
> Why is this still broken?

Sorry, I hadn't seen this.  I'm not seeing a boot failure on 5.9-rc8 with just
CONFIG_HARDENED_USERCPOY=y in addition to defconfig (on QEMU, though I doubt
that's relevant here).  It looks like the fix is to essentially revert this,
which I'm fine with, but I'd prefer to have a failing test to make sure this
doesn't break again.

Guo: I don't see an actual patch (signed off and such) posted for this, do you
mind posting one?  Otherwise I'll take a crack at constructing the revert
myself.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-10-05 16:39           ` Palmer Dabbelt
@ 2020-10-05 18:40             ` Andreas Schwab
  2020-10-05 19:45               ` Palmer Dabbelt
  2020-10-06 16:55             ` Guo Ren
  1 sibling, 1 reply; 24+ messages in thread
From: Andreas Schwab @ 2020-10-05 18:40 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: tycho, aou, nickhu, anup, linux-kernel, linux-csky, guoren,
	guoren, zong.li, Paul Walmsley, greentime.hu, tglx, linux-riscv

On Okt 05 2020, Palmer Dabbelt wrote:

> On Mon, 05 Oct 2020 01:25:22 PDT (-0700), schwab@linux-m68k.org wrote:
>> On Sep 14 2020, Aurelien Jarno wrote:
>>
>>> How should we proceed to get that fixed in time for 5.9? For the older
>>> branches where it has been backported (so far 5.7 and 5.8), should we
>>> just get that commit reverted instead?
>>
>> Why is this still broken?
>
> Sorry, I hadn't seen this.  I'm not seeing a boot failure on 5.9-rc8 with just
> CONFIG_HARDENED_USERCPOY=y in addition to defconfig (on QEMU, though I doubt
> that's relevant here).

I don't see a boot failure either, but eventually you will get crashes
like this, and resources are not properly released:

[ 4560.936645] usercopy: Kernel memory overwrite attempt detected to kernel text (offset 241626, size 16)!
[ 4560.945324] ------------[ cut here ]------------
[ 4560.949954] kernel BUG at mm/usercopy.c:99!
[ 4560.954030] Kernel BUG [#1]
[ 4560.956805] Modules linked in: nfsv3 nfs_acl rfkill mmc_block sf_pdma i2c_ocores virt_dma spi_sifive uio_pdrv_genirq uio loop drm drm_panel_orientation_quirks rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache af_packet mscc macsec macb ptp pps_core phylink of_mdio fixed_phy libphy pwm_sifive mmc_spi crc_itu_t crc7 of_mmc_spi mmc_core spi_bitbang sunrpc sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
[ 4560.995103] CPU: 2 PID: 23806 Comm: nis Not tainted 5.8.10-1-default #1 openSUSE Tumbleweed (unreleased)
[ 4561.004563] epc: ffffffe00036140e ra : ffffffe00036140e sp : ffffffe004bc7d60
[ 4561.011679]  gp : ffffffe00127ee60 tp : ffffffe1b05d0000 t0 : ffffffe001297ca0
[ 4561.018886]  t1 : ffffffe001297c30 t2 : 0000000000000000 s0 : ffffffe004bc7d80
[ 4561.026093]  s1 : ffffffe00003afda a0 : 000000000000005b a1 : ffffffe1f7d67588
[ 4561.033298]  a2 : ffffffe1f7d6c108 a3 : 0000000000000000 a4 : ffffffe000043e80
[ 4561.040506]  a5 : ffffffe1f7d6be80 a6 : 0000000000000144 a7 : 0000000000000000
[ 4561.047712]  s2 : 0000000000000010 s3 : 0000000000000000 s4 : ffffffe00003afea
[ 4561.054918]  s5 : ffffffe1f7e00e80 s6 : 0000002af4a2c2e0 s7 : fffffffffffff000
[ 4561.062124]  s8 : 0000003ffffff000 s9 : ffffffe19f985400 s10: 0000000000000010
[ 4561.069329]  s11: ffffffe1f7e00e80 t3 : 0000000000038fa8 t4 : 0000000000038fa8
[ 4561.076533]  t5 : 0000000000000001 t6 : ffffffe00128e062
[ 4561.081832] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
[ 4561.089821] ---[ end trace a7c93e7595e9c2cc ]---
[ 4561.095589] BUG: Bad rss-counter state mm:00000000c54f4c29 type:MM_ANONPAGES val:1

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-09-24 16:19           ` Guo Ren
  2020-09-29 18:51             ` Aurelien Jarno
@ 2020-10-05 19:14             ` Atish Patra
  2020-10-06 16:46               ` Guo Ren
  1 sibling, 1 reply; 24+ messages in thread
From: Atish Patra @ 2020-10-05 19:14 UTC (permalink / raw)
  To: Guo Ren
  Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt,
	Linux Kernel Mailing List, linux-csky, Guo Ren, Andreas Schwab,
	Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner,
	linux-riscv

On Thu, Sep 24, 2020 at 9:19 AM Guo Ren <guoren@kernel.org> wrote:
>
> How about this, revert the commit and don't free INIT_DATA_SECTION. I
> think the solution is safe enough, but wast a little memory.
>
> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> index f3586e3..34d00d9 100644
> --- a/arch/riscv/kernel/vmlinux.lds.S
> +++ b/arch/riscv/kernel/vmlinux.lds.S
> @@ -22,13 +22,11 @@ SECTIONS
>         /* Beginning of code and text segment */
>         . = LOAD_OFFSET;
>         _start = .;
> -       _stext = .;
>         HEAD_TEXT_SECTION
>         . = ALIGN(PAGE_SIZE);
>
>         __init_begin = .;
>         INIT_TEXT_SECTION(PAGE_SIZE)
> -       INIT_DATA_SECTION(16)
>         . = ALIGN(8);
>         __soc_early_init_table : {
>                 __soc_early_init_table_start = .;
> @@ -55,6 +53,7 @@ SECTIONS
>         . = ALIGN(SECTION_ALIGN);
>         .text : {
>                 _text = .;
> +               _stext = .;
>                 TEXT_TEXT
>                 SCHED_TEXT
>                 CPUIDLE_TEXT
> @@ -67,6 +66,8 @@ SECTIONS
>                 _etext = .;
>         }
>
> +       INIT_DATA_SECTION(16)
> +

I think you need to move EXIT_DATA as well. Currently, we have init
data & text in one section.
In general it is better idea to separate those similar to ARM64.
Additionally, ARM64 applies different mapping for init data & text
as the init data section is marked as non-executable[1]

However, we don't modify any permission for any init sections. Should
we do that as well ?

[1] https://patchwork.kernel.org/patch/9572869/

>         /* Start of data section */
>         _sdata = .;
>         RO_DATA(SECTION_ALIGN)
>
> On Thu, Sep 24, 2020 at 3:36 PM Andreas Schwab <schwab@linux-m68k.org> wrote:
> >
> > On Sep 14 2020, Aurelien Jarno wrote:
> >
> > > How should we proceed to get that fixed in time for 5.9? For the older
> > > branches where it has been backported (so far 5.7 and 5.8), should we
> > > just get that commit reverted instead?
> >
> > Can this please be resolved ASAP?
> >
> > Andreas.
> >
> > --
> > Andreas Schwab, schwab@linux-m68k.org
> > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> > "And now for something completely different."
>
>
>
> --
> Best Regards
>  Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Regards,
Atish

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-10-05 18:40             ` Andreas Schwab
@ 2020-10-05 19:45               ` Palmer Dabbelt
  2020-10-05 21:12                 ` Atish Patra
  0 siblings, 1 reply; 24+ messages in thread
From: Palmer Dabbelt @ 2020-10-05 19:45 UTC (permalink / raw)
  To: schwab
  Cc: tycho, aou, nickhu, anup, linux-kernel, linux-csky, guoren,
	guoren, zong.li, Paul Walmsley, greentime.hu, tglx, linux-riscv

On Mon, 05 Oct 2020 11:40:54 PDT (-0700), schwab@linux-m68k.org wrote:
> On Okt 05 2020, Palmer Dabbelt wrote:
>
>> On Mon, 05 Oct 2020 01:25:22 PDT (-0700), schwab@linux-m68k.org wrote:
>>> On Sep 14 2020, Aurelien Jarno wrote:
>>>
>>>> How should we proceed to get that fixed in time for 5.9? For the older
>>>> branches where it has been backported (so far 5.7 and 5.8), should we
>>>> just get that commit reverted instead?
>>>
>>> Why is this still broken?
>>
>> Sorry, I hadn't seen this.  I'm not seeing a boot failure on 5.9-rc8 with just
>> CONFIG_HARDENED_USERCPOY=y in addition to defconfig (on QEMU, though I doubt
>> that's relevant here).
>
> I don't see a boot failure either, but eventually you will get crashes
> like this, and resources are not properly released:
>
> [ 4560.936645] usercopy: Kernel memory overwrite attempt detected to kernel text (offset 241626, size 16)!
> [ 4560.945324] ------------[ cut here ]------------
> [ 4560.949954] kernel BUG at mm/usercopy.c:99!
> [ 4560.954030] Kernel BUG [#1]
> [ 4560.956805] Modules linked in: nfsv3 nfs_acl rfkill mmc_block sf_pdma i2c_ocores virt_dma spi_sifive uio_pdrv_genirq uio loop drm drm_panel_orientation_quirks rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache af_packet mscc macsec macb ptp pps_core phylink of_mdio fixed_phy libphy pwm_sifive mmc_spi crc_itu_t crc7 of_mmc_spi mmc_core spi_bitbang sunrpc sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
> [ 4560.995103] CPU: 2 PID: 23806 Comm: nis Not tainted 5.8.10-1-default #1 openSUSE Tumbleweed (unreleased)
> [ 4561.004563] epc: ffffffe00036140e ra : ffffffe00036140e sp : ffffffe004bc7d60
> [ 4561.011679]  gp : ffffffe00127ee60 tp : ffffffe1b05d0000 t0 : ffffffe001297ca0
> [ 4561.018886]  t1 : ffffffe001297c30 t2 : 0000000000000000 s0 : ffffffe004bc7d80
> [ 4561.026093]  s1 : ffffffe00003afda a0 : 000000000000005b a1 : ffffffe1f7d67588
> [ 4561.033298]  a2 : ffffffe1f7d6c108 a3 : 0000000000000000 a4 : ffffffe000043e80
> [ 4561.040506]  a5 : ffffffe1f7d6be80 a6 : 0000000000000144 a7 : 0000000000000000
> [ 4561.047712]  s2 : 0000000000000010 s3 : 0000000000000000 s4 : ffffffe00003afea
> [ 4561.054918]  s5 : ffffffe1f7e00e80 s6 : 0000002af4a2c2e0 s7 : fffffffffffff000
> [ 4561.062124]  s8 : 0000003ffffff000 s9 : ffffffe19f985400 s10: 0000000000000010
> [ 4561.069329]  s11: ffffffe1f7e00e80 t3 : 0000000000038fa8 t4 : 0000000000038fa8
> [ 4561.076533]  t5 : 0000000000000001 t6 : ffffffe00128e062
> [ 4561.081832] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
> [ 4561.089821] ---[ end trace a7c93e7595e9c2cc ]---
> [ 4561.095589] BUG: Bad rss-counter state mm:00000000c54f4c29 type:MM_ANONPAGES val:1

Ah, I must have misunderstood.  I guess I just assumed "init crashes" meant on
boot, not just at some time later.  I just sent out a patch reverting this, LMK
if it fixes the issue.  I have some work stuff to do, but I'll try to find some
time tonight to look into fixing both of the bugs -- otherwise I'll just take
the revert (assuming it does actually fix the issue for you and passes the
tests).

I saw Atish post after I started writing this: I agree we need to sort of the
kernel's memory map, I just think it's too late for 5.9.

> Andreas.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-10-05 19:45               ` Palmer Dabbelt
@ 2020-10-05 21:12                 ` Atish Patra
  2020-10-05 21:17                   ` Palmer Dabbelt
  0 siblings, 1 reply; 24+ messages in thread
From: Atish Patra @ 2020-10-05 21:12 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: linux-riscv, Tycho Andersen, Albert Ou, Nick Hu, Anup Patel,
	linux-kernel@vger.kernel.org List, linux-csky, Guo Ren,
	Andreas Schwab, Zong Li, Paul Walmsley, Guo Ren, Thomas Gleixner,
	Greentime Hu

On Mon, Oct 5, 2020 at 12:46 PM Palmer Dabbelt <palmerdabbelt@google.com> wrote:
>
> On Mon, 05 Oct 2020 11:40:54 PDT (-0700), schwab@linux-m68k.org wrote:
> > On Okt 05 2020, Palmer Dabbelt wrote:
> >
> >> On Mon, 05 Oct 2020 01:25:22 PDT (-0700), schwab@linux-m68k.org wrote:
> >>> On Sep 14 2020, Aurelien Jarno wrote:
> >>>
> >>>> How should we proceed to get that fixed in time for 5.9? For the older
> >>>> branches where it has been backported (so far 5.7 and 5.8), should we
> >>>> just get that commit reverted instead?
> >>>
> >>> Why is this still broken?
> >>
> >> Sorry, I hadn't seen this.  I'm not seeing a boot failure on 5.9-rc8 with just
> >> CONFIG_HARDENED_USERCPOY=y in addition to defconfig (on QEMU, though I doubt
> >> that's relevant here).
> >
> > I don't see a boot failure either, but eventually you will get crashes
> > like this, and resources are not properly released:
> >
> > [ 4560.936645] usercopy: Kernel memory overwrite attempt detected to kernel text (offset 241626, size 16)!
> > [ 4560.945324] ------------[ cut here ]------------
> > [ 4560.949954] kernel BUG at mm/usercopy.c:99!
> > [ 4560.954030] Kernel BUG [#1]
> > [ 4560.956805] Modules linked in: nfsv3 nfs_acl rfkill mmc_block sf_pdma i2c_ocores virt_dma spi_sifive uio_pdrv_genirq uio loop drm drm_panel_orientation_quirks rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache af_packet mscc macsec macb ptp pps_core phylink of_mdio fixed_phy libphy pwm_sifive mmc_spi crc_itu_t crc7 of_mmc_spi mmc_core spi_bitbang sunrpc sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
> > [ 4560.995103] CPU: 2 PID: 23806 Comm: nis Not tainted 5.8.10-1-default #1 openSUSE Tumbleweed (unreleased)
> > [ 4561.004563] epc: ffffffe00036140e ra : ffffffe00036140e sp : ffffffe004bc7d60
> > [ 4561.011679]  gp : ffffffe00127ee60 tp : ffffffe1b05d0000 t0 : ffffffe001297ca0
> > [ 4561.018886]  t1 : ffffffe001297c30 t2 : 0000000000000000 s0 : ffffffe004bc7d80
> > [ 4561.026093]  s1 : ffffffe00003afda a0 : 000000000000005b a1 : ffffffe1f7d67588
> > [ 4561.033298]  a2 : ffffffe1f7d6c108 a3 : 0000000000000000 a4 : ffffffe000043e80
> > [ 4561.040506]  a5 : ffffffe1f7d6be80 a6 : 0000000000000144 a7 : 0000000000000000
> > [ 4561.047712]  s2 : 0000000000000010 s3 : 0000000000000000 s4 : ffffffe00003afea
> > [ 4561.054918]  s5 : ffffffe1f7e00e80 s6 : 0000002af4a2c2e0 s7 : fffffffffffff000
> > [ 4561.062124]  s8 : 0000003ffffff000 s9 : ffffffe19f985400 s10: 0000000000000010
> > [ 4561.069329]  s11: ffffffe1f7e00e80 t3 : 0000000000038fa8 t4 : 0000000000038fa8
> > [ 4561.076533]  t5 : 0000000000000001 t6 : ffffffe00128e062
> > [ 4561.081832] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
> > [ 4561.089821] ---[ end trace a7c93e7595e9c2cc ]---
> > [ 4561.095589] BUG: Bad rss-counter state mm:00000000c54f4c29 type:MM_ANONPAGES val:1
>
> Ah, I must have misunderstood.  I guess I just assumed "init crashes" meant on
> boot, not just at some time later.  I just sent out a patch reverting this, LMK
> if it fixes the issue.  I have some work stuff to do, but I'll try to find some
> time tonight to look into fixing both of the bugs -- otherwise I'll just take
> the revert (assuming it does actually fix the issue for you and passes the
> tests).
>
> I saw Atish post after I started writing this: I agree we need to sort of the
> kernel's memory map, I just think it's too late for 5.9.
>

Yes. It is definitely a for-next material. I will try to take a stab
at this if nobody else has an objection.

> > Andreas.
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



--
Regards,
Atish

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-10-05 21:12                 ` Atish Patra
@ 2020-10-05 21:17                   ` Palmer Dabbelt
  0 siblings, 0 replies; 24+ messages in thread
From: Palmer Dabbelt @ 2020-10-05 21:17 UTC (permalink / raw)
  To: atishp
  Cc: linux-riscv, tycho, aou, nickhu, anup, linux-kernel, linux-csky,
	guoren, schwab, zong.li, Paul Walmsley, guoren, tglx,
	greentime.hu

On Mon, 05 Oct 2020 14:12:44 PDT (-0700), atishp@atishpatra.org wrote:
> On Mon, Oct 5, 2020 at 12:46 PM Palmer Dabbelt <palmerdabbelt@google.com> wrote:
>>
>> On Mon, 05 Oct 2020 11:40:54 PDT (-0700), schwab@linux-m68k.org wrote:
>> > On Okt 05 2020, Palmer Dabbelt wrote:
>> >
>> >> On Mon, 05 Oct 2020 01:25:22 PDT (-0700), schwab@linux-m68k.org wrote:
>> >>> On Sep 14 2020, Aurelien Jarno wrote:
>> >>>
>> >>>> How should we proceed to get that fixed in time for 5.9? For the older
>> >>>> branches where it has been backported (so far 5.7 and 5.8), should we
>> >>>> just get that commit reverted instead?
>> >>>
>> >>> Why is this still broken?
>> >>
>> >> Sorry, I hadn't seen this.  I'm not seeing a boot failure on 5.9-rc8 with just
>> >> CONFIG_HARDENED_USERCPOY=y in addition to defconfig (on QEMU, though I doubt
>> >> that's relevant here).
>> >
>> > I don't see a boot failure either, but eventually you will get crashes
>> > like this, and resources are not properly released:
>> >
>> > [ 4560.936645] usercopy: Kernel memory overwrite attempt detected to kernel text (offset 241626, size 16)!
>> > [ 4560.945324] ------------[ cut here ]------------
>> > [ 4560.949954] kernel BUG at mm/usercopy.c:99!
>> > [ 4560.954030] Kernel BUG [#1]
>> > [ 4560.956805] Modules linked in: nfsv3 nfs_acl rfkill mmc_block sf_pdma i2c_ocores virt_dma spi_sifive uio_pdrv_genirq uio loop drm drm_panel_orientation_quirks rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache af_packet mscc macsec macb ptp pps_core phylink of_mdio fixed_phy libphy pwm_sifive mmc_spi crc_itu_t crc7 of_mmc_spi mmc_core spi_bitbang sunrpc sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
>> > [ 4560.995103] CPU: 2 PID: 23806 Comm: nis Not tainted 5.8.10-1-default #1 openSUSE Tumbleweed (unreleased)
>> > [ 4561.004563] epc: ffffffe00036140e ra : ffffffe00036140e sp : ffffffe004bc7d60
>> > [ 4561.011679]  gp : ffffffe00127ee60 tp : ffffffe1b05d0000 t0 : ffffffe001297ca0
>> > [ 4561.018886]  t1 : ffffffe001297c30 t2 : 0000000000000000 s0 : ffffffe004bc7d80
>> > [ 4561.026093]  s1 : ffffffe00003afda a0 : 000000000000005b a1 : ffffffe1f7d67588
>> > [ 4561.033298]  a2 : ffffffe1f7d6c108 a3 : 0000000000000000 a4 : ffffffe000043e80
>> > [ 4561.040506]  a5 : ffffffe1f7d6be80 a6 : 0000000000000144 a7 : 0000000000000000
>> > [ 4561.047712]  s2 : 0000000000000010 s3 : 0000000000000000 s4 : ffffffe00003afea
>> > [ 4561.054918]  s5 : ffffffe1f7e00e80 s6 : 0000002af4a2c2e0 s7 : fffffffffffff000
>> > [ 4561.062124]  s8 : 0000003ffffff000 s9 : ffffffe19f985400 s10: 0000000000000010
>> > [ 4561.069329]  s11: ffffffe1f7e00e80 t3 : 0000000000038fa8 t4 : 0000000000038fa8
>> > [ 4561.076533]  t5 : 0000000000000001 t6 : ffffffe00128e062
>> > [ 4561.081832] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
>> > [ 4561.089821] ---[ end trace a7c93e7595e9c2cc ]---
>> > [ 4561.095589] BUG: Bad rss-counter state mm:00000000c54f4c29 type:MM_ANONPAGES val:1
>>
>> Ah, I must have misunderstood.  I guess I just assumed "init crashes" meant on
>> boot, not just at some time later.  I just sent out a patch reverting this, LMK
>> if it fixes the issue.  I have some work stuff to do, but I'll try to find some
>> time tonight to look into fixing both of the bugs -- otherwise I'll just take
>> the revert (assuming it does actually fix the issue for you and passes the
>> tests).
>>
>> I saw Atish post after I started writing this: I agree we need to sort of the
>> kernel's memory map, I just think it's too late for 5.9.
>>
>
> Yes. It is definitely a for-next material. I will try to take a stab
> at this if nobody else has an objection.

WFM.  Thanks!

>
>> > Andreas.
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-10-05 19:14             ` Atish Patra
@ 2020-10-06 16:46               ` Guo Ren
  2020-10-06 20:38                 ` Atish Patra
  0 siblings, 1 reply; 24+ messages in thread
From: Guo Ren @ 2020-10-06 16:46 UTC (permalink / raw)
  To: Atish Patra
  Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt,
	Linux Kernel Mailing List, linux-csky, Guo Ren, Andreas Schwab,
	Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner,
	linux-riscv

On Tue, Oct 6, 2020 at 3:14 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Thu, Sep 24, 2020 at 9:19 AM Guo Ren <guoren@kernel.org> wrote:
> >
> > How about this, revert the commit and don't free INIT_DATA_SECTION. I
> > think the solution is safe enough, but wast a little memory.
> >
> > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > index f3586e3..34d00d9 100644
> > --- a/arch/riscv/kernel/vmlinux.lds.S
> > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > @@ -22,13 +22,11 @@ SECTIONS
> >         /* Beginning of code and text segment */
> >         . = LOAD_OFFSET;
> >         _start = .;
> > -       _stext = .;
> >         HEAD_TEXT_SECTION
> >         . = ALIGN(PAGE_SIZE);
> >
> >         __init_begin = .;
> >         INIT_TEXT_SECTION(PAGE_SIZE)
> > -       INIT_DATA_SECTION(16)
> >         . = ALIGN(8);
> >         __soc_early_init_table : {
> >                 __soc_early_init_table_start = .;
> > @@ -55,6 +53,7 @@ SECTIONS
> >         . = ALIGN(SECTION_ALIGN);
> >         .text : {
> >                 _text = .;
> > +               _stext = .;
> >                 TEXT_TEXT
> >                 SCHED_TEXT
> >                 CPUIDLE_TEXT
> > @@ -67,6 +66,8 @@ SECTIONS
> >                 _etext = .;
> >         }
> >
> > +       INIT_DATA_SECTION(16)
> > +
>
> I think you need to move EXIT_DATA as well. Currently, we have init
> data & text in one section.
It's not related to this issue. There is two check code problem:
 1.     static int static_obj(const void *obj)
    {
            unsigned long start = (unsigned long) &_stext,
                          end   = (unsigned long) &_end,
                          addr  = (unsigned long) obj;

            /*
             * static variable?
             */
            if ((addr >= start) && (addr < end))
                    return 1;

 2.     /* Is this address range in the kernel text area? */
    static inline void check_kernel_text_object(const unsigned long ptr,
                                                unsigned long n, bool to_user)
    {
            unsigned long textlow = (unsigned long)_stext;
            unsigned long texthigh = (unsigned long)_etext;
            unsigned long textlow_linear, texthigh_linear;

            if (overlaps(ptr, n, textlow, texthigh))
                    usercopy_abort("kernel text", NULL, to_user, ptr -
textlow, n);

The patch of commit: a0fa4027dc911 (riscv: Fixup static_obj() fail) broke 2th.

> In general it is better idea to separate those similar to ARM64.
> Additionally, ARM64 applies different mapping for init data & text
> as the init data section is marked as non-executable[1]
Yes, it's safer to protect init text & init data, but it's should be
another patch.

>
> However, we don't modify any permission for any init sections. Should
> we do that as well ?
Agree, we should do that.

>
> [1] https://patchwork.kernel.org/patch/9572869/
>
> >         /* Start of data section */
> >         _sdata = .;
> >         RO_DATA(SECTION_ALIGN)
> >
> > On Thu, Sep 24, 2020 at 3:36 PM Andreas Schwab <schwab@linux-m68k.org> wrote:
> > >
> > > On Sep 14 2020, Aurelien Jarno wrote:
> > >
> > > > How should we proceed to get that fixed in time for 5.9? For the older
> > > > branches where it has been backported (so far 5.7 and 5.8), should we
> > > > just get that commit reverted instead?
> > >
> > > Can this please be resolved ASAP?
> > >
> > > Andreas.
> > >
> > > --
> > > Andreas Schwab, schwab@linux-m68k.org
> > > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> > > "And now for something completely different."
> >
> >
> >
> > --
> > Best Regards
> >  Guo Ren
> >
> > ML: https://lore.kernel.org/linux-csky/
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
>
>
> --
> Regards,
> Atish



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-10-05 16:39           ` Palmer Dabbelt
  2020-10-05 18:40             ` Andreas Schwab
@ 2020-10-06 16:55             ` Guo Ren
  1 sibling, 0 replies; 24+ messages in thread
From: Guo Ren @ 2020-10-06 16:55 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel,
	Linux Kernel Mailing List, linux-csky, Guo Ren, Andreas Schwab,
	Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner,
	linux-riscv

On Tue, Oct 6, 2020 at 12:39 AM Palmer Dabbelt <palmerdabbelt@google.com> wrote:
>
> On Mon, 05 Oct 2020 01:25:22 PDT (-0700), schwab@linux-m68k.org wrote:
> > On Sep 14 2020, Aurelien Jarno wrote:
> >
> >> How should we proceed to get that fixed in time for 5.9? For the older
> >> branches where it has been backported (so far 5.7 and 5.8), should we
> >> just get that commit reverted instead?
> >
> > Why is this still broken?
>
> Sorry, I hadn't seen this.  I'm not seeing a boot failure on 5.9-rc8 with just
> CONFIG_HARDENED_USERCPOY=y in addition to defconfig (on QEMU, though I doubt
> that's relevant here).  It looks like the fix is to essentially revert this,
> which I'm fine with, but I'd prefer to have a failing test to make sure this
> doesn't break again.
>
> Guo: I don't see an actual patch (signed off and such) posted for this, do you
> mind posting one?  Otherwise I'll take a crack at constructing the revert
> myself.

Please have a look:
https://lore.kernel.org/linux-riscv/1602002973-92934-1-git-send-email-guoren@kernel.org/T/#u

The only revert couldn't solve the static_obj problem.

-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-10-06 16:46               ` Guo Ren
@ 2020-10-06 20:38                 ` Atish Patra
  2020-10-07 14:45                   ` Guo Ren
  0 siblings, 1 reply; 24+ messages in thread
From: Atish Patra @ 2020-10-06 20:38 UTC (permalink / raw)
  To: Guo Ren
  Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt,
	Linux Kernel Mailing List, linux-csky, Guo Ren, Andreas Schwab,
	Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner,
	linux-riscv

On Tue, Oct 6, 2020 at 9:46 AM Guo Ren <guoren@kernel.org> wrote:
>
> On Tue, Oct 6, 2020 at 3:14 AM Atish Patra <atishp@atishpatra.org> wrote:
> >
> > On Thu, Sep 24, 2020 at 9:19 AM Guo Ren <guoren@kernel.org> wrote:
> > >
> > > How about this, revert the commit and don't free INIT_DATA_SECTION. I
> > > think the solution is safe enough, but wast a little memory.
> > >
> > > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > > index f3586e3..34d00d9 100644
> > > --- a/arch/riscv/kernel/vmlinux.lds.S
> > > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > > @@ -22,13 +22,11 @@ SECTIONS
> > >         /* Beginning of code and text segment */
> > >         . = LOAD_OFFSET;
> > >         _start = .;
> > > -       _stext = .;
> > >         HEAD_TEXT_SECTION
> > >         . = ALIGN(PAGE_SIZE);
> > >
> > >         __init_begin = .;
> > >         INIT_TEXT_SECTION(PAGE_SIZE)
> > > -       INIT_DATA_SECTION(16)
> > >         . = ALIGN(8);
> > >         __soc_early_init_table : {
> > >                 __soc_early_init_table_start = .;
> > > @@ -55,6 +53,7 @@ SECTIONS
> > >         . = ALIGN(SECTION_ALIGN);
> > >         .text : {
> > >                 _text = .;
> > > +               _stext = .;
> > >                 TEXT_TEXT
> > >                 SCHED_TEXT
> > >                 CPUIDLE_TEXT
> > > @@ -67,6 +66,8 @@ SECTIONS
> > >                 _etext = .;
> > >         }
> > >
> > > +       INIT_DATA_SECTION(16)
> > > +
> >
> > I think you need to move EXIT_DATA as well. Currently, we have init
> > data & text in one section.
> It's not related to this issue. There is two check code problem:

Yes. But we shouldn't move only INIT_DATA_SECTION out of __init section
while leaving percpu & exit data in the __init section. Here is what I
have in mind.

diff --git a/arch/riscv/kernel/vmlinux.lds.S
b/arch/riscv/kernel/vmlinux.lds.S
index 9795359cb9da..4432cef8184e 100644
--- a/arch/riscv/kernel/vmlinux.lds.S
+++ b/arch/riscv/kernel/vmlinux.lds.S
@@ -26,13 +26,11 @@ SECTIONS
        /* Beginning of code and text segment */
        . = LOAD_OFFSET;
        _start = .;
        _start = .;
-       _stext = .;
        HEAD_TEXT_SECTION
        . = ALIGN(PAGE_SIZE);

        __init_begin = .;
        INIT_TEXT_SECTION(PAGE_SIZE)
-       INIT_DATA_SECTION(16)
        . = ALIGN(8);
        __soc_early_init_table : {
                __soc_early_init_table_start = .;
@@ -49,16 +47,13 @@ SECTIONS
        {
                EXIT_TEXT
        }
-       .exit.data :
-       {
-               EXIT_DATA
-       }
-       PERCPU_SECTION(L1_CACHE_BYTES)
+
        __init_end = .;

        . = ALIGN(SECTION_ALIGN);
        .text : {
                _text = .;
+               _stext = .;
                TEXT_TEXT
                SCHED_TEXT
                CPUIDLE_TEXT
@@ -77,6 +72,17 @@ SECTIONS
 #endif

        /* Start of data section */
+       __init_data_begin = .;
+       INIT_DATA_SECTION(16)
+       .exit.data :
+       {
+               EXIT_DATA
+       }
+
+       PERCPU_SECTION(L1_CACHE_BYTES)
+
+       __init_data_end = .;
+

As you correctly pointed out, this wastes around ~200K init memory
that is wasted.
That is not an ideal solution.

The other alternative is to move __init_text section after _text as
well similar to other architectures. But that won't work
for RISC-V as we jump from _start to __start_kernel(in __init section)
in head.S.  A JAL instruction can't be fit because
__start_kernel is now too far. We can't replace JAL with a JALR
because that would require an additional
instruction and violates image header format.

Any other ideas to solve this problem without wasting memory ?

>  1.     static int static_obj(const void *obj)
>     {
>             unsigned long start = (unsigned long) &_stext,
>                           end   = (unsigned long) &_end,
>                           addr  = (unsigned long) obj;
>
>             /*
>              * static variable?
>              */
>             if ((addr >= start) && (addr < end))
>                     return 1;
>
>  2.     /* Is this address range in the kernel text area? */
>     static inline void check_kernel_text_object(const unsigned long ptr,
>                                                 unsigned long n, bool to_user)
>     {
>             unsigned long textlow = (unsigned long)_stext;
>             unsigned long texthigh = (unsigned long)_etext;
>             unsigned long textlow_linear, texthigh_linear;
>
>             if (overlaps(ptr, n, textlow, texthigh))
>                     usercopy_abort("kernel text", NULL, to_user, ptr -
> textlow, n);
>
> The patch of commit: a0fa4027dc911 (riscv: Fixup static_obj() fail) broke 2th.
>
> > In general it is better idea to separate those similar to ARM64.
> > Additionally, ARM64 applies different mapping for init data & text
> > as the init data section is marked as non-executable[1]
> Yes, it's safer to protect init text & init data, but it's should be
> another patch.
>

Yes. I will send the patch based on this fix.

> >
> > However, we don't modify any permission for any init sections. Should
> > we do that as well ?
> Agree, we should do that.
>
> >
> > [1] https://patchwork.kernel.org/patch/9572869/
> >
> > >         /* Start of data section */
> > >         _sdata = .;
> > >         RO_DATA(SECTION_ALIGN)
> > >
> > > On Thu, Sep 24, 2020 at 3:36 PM Andreas Schwab <schwab@linux-m68k.org> wrote:
> > > >
> > > > On Sep 14 2020, Aurelien Jarno wrote:
> > > >
> > > > > How should we proceed to get that fixed in time for 5.9? For the older
> > > > > branches where it has been backported (so far 5.7 and 5.8), should we
> > > > > just get that commit reverted instead?
> > > >
> > > > Can this please be resolved ASAP?
> > > >
> > > > Andreas.
> > > >
> > > > --
> > > > Andreas Schwab, schwab@linux-m68k.org
> > > > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> > > > "And now for something completely different."
> > >
> > >
> > >
> > > --
> > > Best Regards
> > >  Guo Ren
> > >
> > > ML: https://lore.kernel.org/linux-csky/
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> >
> >
> >
> > --
> > Regards,
> > Atish
>
>
>
> --
> Best Regards
>  Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/



-- 
Regards,
Atish

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail
  2020-10-06 20:38                 ` Atish Patra
@ 2020-10-07 14:45                   ` Guo Ren
  0 siblings, 0 replies; 24+ messages in thread
From: Guo Ren @ 2020-10-07 14:45 UTC (permalink / raw)
  To: Atish Patra
  Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt,
	Linux Kernel Mailing List, linux-csky, Guo Ren, Andreas Schwab,
	Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner,
	linux-riscv

On Wed, Oct 7, 2020 at 4:39 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Tue, Oct 6, 2020 at 9:46 AM Guo Ren <guoren@kernel.org> wrote:
> >
> > On Tue, Oct 6, 2020 at 3:14 AM Atish Patra <atishp@atishpatra.org> wrote:
> > >
> > > On Thu, Sep 24, 2020 at 9:19 AM Guo Ren <guoren@kernel.org> wrote:
> > > >
> > > > How about this, revert the commit and don't free INIT_DATA_SECTION. I
> > > > think the solution is safe enough, but wast a little memory.
> > > >
> > > > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > > > index f3586e3..34d00d9 100644
> > > > --- a/arch/riscv/kernel/vmlinux.lds.S
> > > > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > > > @@ -22,13 +22,11 @@ SECTIONS
> > > >         /* Beginning of code and text segment */
> > > >         . = LOAD_OFFSET;
> > > >         _start = .;
> > > > -       _stext = .;
> > > >         HEAD_TEXT_SECTION
> > > >         . = ALIGN(PAGE_SIZE);
> > > >
> > > >         __init_begin = .;
> > > >         INIT_TEXT_SECTION(PAGE_SIZE)
> > > > -       INIT_DATA_SECTION(16)
> > > >         . = ALIGN(8);
> > > >         __soc_early_init_table : {
> > > >                 __soc_early_init_table_start = .;
> > > > @@ -55,6 +53,7 @@ SECTIONS
> > > >         . = ALIGN(SECTION_ALIGN);
> > > >         .text : {
> > > >                 _text = .;
> > > > +               _stext = .;
> > > >                 TEXT_TEXT
> > > >                 SCHED_TEXT
> > > >                 CPUIDLE_TEXT
> > > > @@ -67,6 +66,8 @@ SECTIONS
> > > >                 _etext = .;
> > > >         }
> > > >
> > > > +       INIT_DATA_SECTION(16)
> > > > +
> > >
> > > I think you need to move EXIT_DATA as well. Currently, we have init
> > > data & text in one section.
> > It's not related to this issue. There is two check code problem:
>
> Yes. But we shouldn't move only INIT_DATA_SECTION out of __init section
> while leaving percpu & exit data in the __init section. Here is what I
> have in mind.
>
> diff --git a/arch/riscv/kernel/vmlinux.lds.S
> b/arch/riscv/kernel/vmlinux.lds.S
> index 9795359cb9da..4432cef8184e 100644
> --- a/arch/riscv/kernel/vmlinux.lds.S
> +++ b/arch/riscv/kernel/vmlinux.lds.S
> @@ -26,13 +26,11 @@ SECTIONS
>         /* Beginning of code and text segment */
>         . = LOAD_OFFSET;
>         _start = .;
>         _start = .;
> -       _stext = .;
>         HEAD_TEXT_SECTION
>         . = ALIGN(PAGE_SIZE);
>
>         __init_begin = .;
>         INIT_TEXT_SECTION(PAGE_SIZE)
> -       INIT_DATA_SECTION(16)
>         . = ALIGN(8);
>         __soc_early_init_table : {
>                 __soc_early_init_table_start = .;
> @@ -49,16 +47,13 @@ SECTIONS
>         {
>                 EXIT_TEXT
>         }
> -       .exit.data :
> -       {
> -               EXIT_DATA
> -       }
> -       PERCPU_SECTION(L1_CACHE_BYTES)
> +
>         __init_end = .;
>
>         . = ALIGN(SECTION_ALIGN);
>         .text : {
>                 _text = .;
> +               _stext = .;
>                 TEXT_TEXT
>                 SCHED_TEXT
>                 CPUIDLE_TEXT
> @@ -77,6 +72,17 @@ SECTIONS
>  #endif
>
>         /* Start of data section */
> +       __init_data_begin = .;
> +       INIT_DATA_SECTION(16)
> +       .exit.data :
> +       {
> +               EXIT_DATA
> +       }
> +
> +       PERCPU_SECTION(L1_CACHE_BYTES)
> +
> +       __init_data_end = .;
> +
>
> As you correctly pointed out, this wastes around ~200K init memory
> that is wasted.
> That is not an ideal solution.
>
> The other alternative is to move __init_text section after _text as
> well similar to other architectures. But that won't work
> for RISC-V as we jump from _start to __start_kernel(in __init section)
> in head.S.  A JAL instruction can't be fit because
> __start_kernel is now too far. We can't replace JAL with a JALR
> because that would require an additional
> instruction and violates image header format.
>
> Any other ideas to solve this problem without wasting memory ?
How about:

diff --git a/arch/riscv/include/asm/sections.h
b/arch/riscv/include/asm/sections.h
new file mode 100644
index 00000000..2317b9e
--- /dev/null
+++ b/arch/riscv/include/asm/sections.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _ASM_RISCV_SECTIONS_H
+#define _ASM_RISCV_SECTIONS_H
+
+#define arch_is_kernel_data arch_is_kernel_data
+
+#include <asm-generic/sections.h>
+
+extern bool init_mem_is_free;
+
+static inline int arch_is_kernel_data(unsigned long addr)
+{
+       if (init_mem_is_free)
+               return 0;
+
+       return addr >= (unsigned long)__init_begin &&
+               addr < (unsigned long)__init_end;
+}
+#endif /* _ASM_RISCV_SECTIONS_H */
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 2c6dd32..9ebd5eb4 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -17,6 +17,7 @@
 #include <linux/sched/task.h>
 #include <linux/swiotlb.h>
 #include <linux/smp.h>
+#include <linux/poison.h>

 #include <asm/cpu_ops.h>
 #include <asm/setup.h>
@@ -112,3 +113,11 @@ static int __init topology_init(void)
        return 0;
 }
 subsys_initcall(topology_init);
+
+bool init_mem_is_free = false;
+
+void free_initmem(void)
+{
+       free_initmem_default(POISON_FREE_INITMEM);
+       init_mem_is_free = true;
+}

>
> >  1.     static int static_obj(const void *obj)
> >     {
> >             unsigned long start = (unsigned long) &_stext,
> >                           end   = (unsigned long) &_end,
> >                           addr  = (unsigned long) obj;
> >
> >             /*
> >              * static variable?
> >              */
> >             if ((addr >= start) && (addr < end))
> >                     return 1;
> >
> >  2.     /* Is this address range in the kernel text area? */
> >     static inline void check_kernel_text_object(const unsigned long ptr,
> >                                                 unsigned long n, bool to_user)
> >     {
> >             unsigned long textlow = (unsigned long)_stext;
> >             unsigned long texthigh = (unsigned long)_etext;
> >             unsigned long textlow_linear, texthigh_linear;
> >
> >             if (overlaps(ptr, n, textlow, texthigh))
> >                     usercopy_abort("kernel text", NULL, to_user, ptr -
> > textlow, n);
> >
> > The patch of commit: a0fa4027dc911 (riscv: Fixup static_obj() fail) broke 2th.
> >
> > > In general it is better idea to separate those similar to ARM64.
> > > Additionally, ARM64 applies different mapping for init data & text
> > > as the init data section is marked as non-executable[1]
> > Yes, it's safer to protect init text & init data, but it's should be
> > another patch.
> >
>
> Yes. I will send the patch based on this fix.
>
> > >
> > > However, we don't modify any permission for any init sections. Should
> > > we do that as well ?
> > Agree, we should do that.
> >
> > >
> > > [1] https://patchwork.kernel.org/patch/9572869/
> > >
> > > >         /* Start of data section */
> > > >         _sdata = .;
> > > >         RO_DATA(SECTION_ALIGN)
> > > >
> > > > On Thu, Sep 24, 2020 at 3:36 PM Andreas Schwab <schwab@linux-m68k.org> wrote:
> > > > >
> > > > > On Sep 14 2020, Aurelien Jarno wrote:
> > > > >
> > > > > > How should we proceed to get that fixed in time for 5.9? For the older
> > > > > > branches where it has been backported (so far 5.7 and 5.8), should we
> > > > > > just get that commit reverted instead?
> > > > >
> > > > > Can this please be resolved ASAP?
> > > > >
> > > > > Andreas.
> > > > >
> > > > > --
> > > > > Andreas Schwab, schwab@linux-m68k.org
> > > > > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> > > > > "And now for something completely different."
> > > >
> > > >
> > > >
> > > > --
> > > > Best Regards
> > > >  Guo Ren
> > > >
> > > > ML: https://lore.kernel.org/linux-csky/
> > > >
> > > > _______________________________________________
> > > > linux-riscv mailing list
> > > > linux-riscv@lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > >
> > >
> > >
> > > --
> > > Regards,
> > > Atish
> >
> >
> >
> > --
> > Best Regards
> >  Guo Ren
> >
> > ML: https://lore.kernel.org/linux-csky/
>
>
>
> --
> Regards,
> Atish



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2020-10-07 14:45 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-27 13:57 [PATCH V2 0/3] riscv: Enable LOCKDEP guoren
2020-06-27 13:57 ` [PATCH V2 1/3] riscv: Fixup static_obj() fail guoren
2020-09-11 20:45   ` Aurelien Jarno
2020-09-12  2:39     ` Guo Ren
2020-09-14 10:38       ` Aurelien Jarno
2020-09-24  7:36         ` Andreas Schwab
2020-09-24 16:19           ` Guo Ren
2020-09-29 18:51             ` Aurelien Jarno
2020-10-05 19:14             ` Atish Patra
2020-10-06 16:46               ` Guo Ren
2020-10-06 20:38                 ` Atish Patra
2020-10-07 14:45                   ` Guo Ren
2020-10-05  8:25         ` Andreas Schwab
2020-10-05 16:39           ` Palmer Dabbelt
2020-10-05 18:40             ` Andreas Schwab
2020-10-05 19:45               ` Palmer Dabbelt
2020-10-05 21:12                 ` Atish Patra
2020-10-05 21:17                   ` Palmer Dabbelt
2020-10-06 16:55             ` Guo Ren
2020-06-27 13:57 ` [PATCH V2 2/3] riscv: Fixup lockdep_assert_held with wrong param cpu_running guoren
2020-09-29 22:12   ` Atish Patra
2020-06-27 13:57 ` [PATCH V2 3/3] riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT guoren
2020-07-09 22:06 ` [PATCH V2 0/3] riscv: Enable LOCKDEP Palmer Dabbelt
2020-07-09 23:15   ` Guo Ren

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).