* [PATCH 1/2] Revert "riscv: Fixup static_obj() fail" @ 2020-10-07 15:08 guoren 2020-10-07 15:08 ` [PATCH 2/2] riscv: Fixup static_obj() fail v2 guoren 2020-10-07 18:52 ` [PATCH 1/2] Revert "riscv: Fixup static_obj() fail" Atish Patra 0 siblings, 2 replies; 10+ messages in thread From: guoren @ 2020-10-07 15:08 UTC (permalink / raw) To: palmerdabbelt Cc: Guo Ren, linux-kernel, schwab, atishp, linux-riscv, aurelien From: Guo Ren <guoren@linux.alibaba.com> This reverts commit 6184358da0004c8fd940afda6c0a0fa4027dc911. It will cause bootup failure with HARDENED_USERCOPY. As Aurelien has reported: [ 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 ]--- Signed-off-by: Guo Ren <guoren@linux.alibaba.com> Reported-by: Aurelien Jarno <aurelien@aurel32.net> Cc: Palmer Dabbelt <palmerdabbelt@google.com> Cc: Atish Patra <atishp@atishpatra.org> Cc: Andreas Schwab <schwab@linux-m68k.org> --- 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 f3586e3..e6f8016 100644 --- a/arch/riscv/kernel/vmlinux.lds.S +++ b/arch/riscv/kernel/vmlinux.lds.S @@ -22,7 +22,6 @@ SECTIONS /* Beginning of code and text segment */ . = LOAD_OFFSET; _start = .; - _stext = .; HEAD_TEXT_SECTION . = ALIGN(PAGE_SIZE); @@ -55,6 +54,7 @@ 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] 10+ messages in thread
* [PATCH 2/2] riscv: Fixup static_obj() fail v2 2020-10-07 15:08 [PATCH 1/2] Revert "riscv: Fixup static_obj() fail" guoren @ 2020-10-07 15:08 ` guoren 2020-10-07 19:46 ` Atish Patra 2020-10-08 3:54 ` Palmer Dabbelt 2020-10-07 18:52 ` [PATCH 1/2] Revert "riscv: Fixup static_obj() fail" Atish Patra 1 sibling, 2 replies; 10+ messages in thread From: guoren @ 2020-10-07 15:08 UTC (permalink / raw) To: palmerdabbelt Cc: Guo Ren, linux-kernel, schwab, atishp, linux-riscv, aurelien From: Guo Ren <guoren@linux.alibaba.com> v1 is commit: 6184358da0004c8fd940afda6c0a0fa4027dc911 which has been reverted. When enable LOCKDEP, static_obj() will cause error: [ 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 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; if (arch_is_kernel_data(addr)) return 1; We could implement arch_is_kernel_data to fixup it. Link: https://lore.kernel.org/linux-riscv/1593266228-61125-1-git-send-email-guoren@kernel.org/T/#t Signed-off-by: Guo Ren <guoren@linux.alibaba.com> Reported-by: Aurelien Jarno <aurelien@aurel32.net> Cc: Palmer Dabbelt <palmerdabbelt@google.com> Cc: Atish Patra <atishp@atishpatra.org> Cc: Andreas Schwab <schwab@linux-m68k.org> Cc: Aurelien Jarno <aurelien@aurel32.net> --- arch/riscv/include/asm/sections.h | 20 ++++++++++++++++++++ arch/riscv/kernel/setup.c | 9 +++++++++ 2 files changed, 29 insertions(+) create mode 100644 arch/riscv/include/asm/sections.h 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; +} -- 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] 10+ messages in thread
* Re: [PATCH 2/2] riscv: Fixup static_obj() fail v2 2020-10-07 15:08 ` [PATCH 2/2] riscv: Fixup static_obj() fail v2 guoren @ 2020-10-07 19:46 ` Atish Patra 2020-10-08 2:30 ` Guo Ren 2020-10-08 3:54 ` Palmer Dabbelt 1 sibling, 1 reply; 10+ messages in thread From: Atish Patra @ 2020-10-07 19:46 UTC (permalink / raw) To: Guo Ren Cc: Guo Ren, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Andreas Schwab, linux-riscv, aurelien On Wed, Oct 7, 2020 at 8:09 AM <guoren@kernel.org> wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > v1 is commit: 6184358da0004c8fd940afda6c0a0fa4027dc911 which has > been reverted. > > When enable LOCKDEP, static_obj() will cause error: > > [ 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 > > 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; > > if (arch_is_kernel_data(addr)) > return 1; > > We could implement arch_is_kernel_data to fixup it. > > Link: https://lore.kernel.org/linux-riscv/1593266228-61125-1-git-send-email-guoren@kernel.org/T/#t > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > Reported-by: Aurelien Jarno <aurelien@aurel32.net> > Cc: Palmer Dabbelt <palmerdabbelt@google.com> > Cc: Atish Patra <atishp@atishpatra.org> > Cc: Andreas Schwab <schwab@linux-m68k.org> > Cc: Aurelien Jarno <aurelien@aurel32.net> > --- > arch/riscv/include/asm/sections.h | 20 ++++++++++++++++++++ > arch/riscv/kernel/setup.c | 9 +++++++++ > 2 files changed, 29 insertions(+) > create mode 100644 arch/riscv/include/asm/sections.h > > 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 You may want to rebase it on top of for-next as UEFI series also adds this file. . > @@ -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; > +} > -- > 2.7.4 > Looks good. Much cleaner than the first approach. FYI: I am still looking into separating __init text & data so that different permissions can be applied to them. With this patch, we can just separate it on top of _stext. 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] 10+ messages in thread
* Re: [PATCH 2/2] riscv: Fixup static_obj() fail v2 2020-10-07 19:46 ` Atish Patra @ 2020-10-08 2:30 ` Guo Ren 0 siblings, 0 replies; 10+ messages in thread From: Guo Ren @ 2020-10-08 2:30 UTC (permalink / raw) To: Atish Patra Cc: Guo Ren, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Andreas Schwab, linux-riscv, aurelien On Thu, Oct 8, 2020 at 3:46 AM Atish Patra <atishp@atishpatra.org> wrote: > > On Wed, Oct 7, 2020 at 8:09 AM <guoren@kernel.org> wrote: > > > > From: Guo Ren <guoren@linux.alibaba.com> > > > > v1 is commit: 6184358da0004c8fd940afda6c0a0fa4027dc911 which has > > been reverted. > > > > When enable LOCKDEP, static_obj() will cause error: > > > > [ 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 > > > > 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; > > > > if (arch_is_kernel_data(addr)) > > return 1; > > > > We could implement arch_is_kernel_data to fixup it. > > > > Link: https://lore.kernel.org/linux-riscv/1593266228-61125-1-git-send-email-guoren@kernel.org/T/#t > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > Reported-by: Aurelien Jarno <aurelien@aurel32.net> > > Cc: Palmer Dabbelt <palmerdabbelt@google.com> > > Cc: Atish Patra <atishp@atishpatra.org> > > Cc: Andreas Schwab <schwab@linux-m68k.org> > > Cc: Aurelien Jarno <aurelien@aurel32.net> > > --- > > arch/riscv/include/asm/sections.h | 20 ++++++++++++++++++++ > > arch/riscv/kernel/setup.c | 9 +++++++++ > > 2 files changed, 29 insertions(+) > > create mode 100644 arch/riscv/include/asm/sections.h > > > > 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 > > You may want to rebase it on top of for-next as UEFI series also adds this file. ok > . > > > @@ -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; > > +} > > -- > > 2.7.4 > > > > Looks good. Much cleaner than the first approach. > FYI: I am still looking into separating __init text & data so that > different permissions can be applied to them. > With this patch, we can just separate it on top of _stext. > > > Reviewed-by: Atish Patra <atish.patra@wdc.com> > > -- > 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] 10+ messages in thread
* Re: [PATCH 2/2] riscv: Fixup static_obj() fail v2 2020-10-07 15:08 ` [PATCH 2/2] riscv: Fixup static_obj() fail v2 guoren 2020-10-07 19:46 ` Atish Patra @ 2020-10-08 3:54 ` Palmer Dabbelt 2020-10-08 17:57 ` Atish Patra 2020-10-09 1:53 ` Guo Ren 1 sibling, 2 replies; 10+ messages in thread From: Palmer Dabbelt @ 2020-10-08 3:54 UTC (permalink / raw) To: guoren; +Cc: guoren, linux-kernel, schwab, atishp, linux-riscv, aurelien On Wed, 07 Oct 2020 08:08:33 PDT (-0700), guoren@kernel.org wrote: > From: Guo Ren <guoren@linux.alibaba.com> > > v1 is commit: 6184358da0004c8fd940afda6c0a0fa4027dc911 which has > been reverted. > > When enable LOCKDEP, static_obj() will cause error: > > [ 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 > > 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; > > if (arch_is_kernel_data(addr)) > return 1; > > We could implement arch_is_kernel_data to fixup it. > > Link: https://lore.kernel.org/linux-riscv/1593266228-61125-1-git-send-email-guoren@kernel.org/T/#t > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > Reported-by: Aurelien Jarno <aurelien@aurel32.net> > Cc: Palmer Dabbelt <palmerdabbelt@google.com> > Cc: Atish Patra <atishp@atishpatra.org> > Cc: Andreas Schwab <schwab@linux-m68k.org> > Cc: Aurelien Jarno <aurelien@aurel32.net> > --- > arch/riscv/include/asm/sections.h | 20 ++++++++++++++++++++ > arch/riscv/kernel/setup.c | 9 +++++++++ > 2 files changed, 29 insertions(+) > create mode 100644 arch/riscv/include/asm/sections.h > > 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; > +} I'm a bit confused as to what you're trying to do here. Yesterday I got another version of this patch set that moves init data around, today a different one. Yesterday's is tested and simpler, and given it's so late in the process I'm inclined to take that as I don't want to break anything. Right now I have 84814460eef9 ("riscv: Fixup bootup failure with HARDENED_USERCOPY") a78c6f5956a9 ("RISC-V: Make sure memblock reserves the memory containing DT") 549738f15da0 ("Linux 5.9-rc8") Unless there's some functional bug, that's what I'm going to send out for 5.9 -- I'm not all that worried about lacking the ability to free init data. The above seems like fine 5.10 material. Let me know if I'm missing something here. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] riscv: Fixup static_obj() fail v2 2020-10-08 3:54 ` Palmer Dabbelt @ 2020-10-08 17:57 ` Atish Patra 2020-10-09 1:53 ` Guo Ren 1 sibling, 0 replies; 10+ messages in thread From: Atish Patra @ 2020-10-08 17:57 UTC (permalink / raw) To: Palmer Dabbelt Cc: Guo Ren, linux-kernel@vger.kernel.org List, Andreas Schwab, Guo Ren, linux-riscv, aurelien On Wed, Oct 7, 2020 at 8:54 PM Palmer Dabbelt <palmerdabbelt@google.com> wrote: > > On Wed, 07 Oct 2020 08:08:33 PDT (-0700), guoren@kernel.org wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > > > v1 is commit: 6184358da0004c8fd940afda6c0a0fa4027dc911 which has > > been reverted. > > > > When enable LOCKDEP, static_obj() will cause error: > > > > [ 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 > > > > 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; > > > > if (arch_is_kernel_data(addr)) > > return 1; > > > > We could implement arch_is_kernel_data to fixup it. > > > > Link: https://lore.kernel.org/linux-riscv/1593266228-61125-1-git-send-email-guoren@kernel.org/T/#t > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > Reported-by: Aurelien Jarno <aurelien@aurel32.net> > > Cc: Palmer Dabbelt <palmerdabbelt@google.com> > > Cc: Atish Patra <atishp@atishpatra.org> > > Cc: Andreas Schwab <schwab@linux-m68k.org> > > Cc: Aurelien Jarno <aurelien@aurel32.net> > > --- > > arch/riscv/include/asm/sections.h | 20 ++++++++++++++++++++ > > arch/riscv/kernel/setup.c | 9 +++++++++ > > 2 files changed, 29 insertions(+) > > create mode 100644 arch/riscv/include/asm/sections.h > > > > 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; > > +} > > I'm a bit confused as to what you're trying to do here. Yesterday I got > another version of this patch set that moves init data around, today a > different one. Yesterday's is tested and simpler, and given it's so late in > the process I'm inclined to take that as I don't want to break anything. > I agree. This version will have some more conflicts with other patches and it's too late in cycle. It's best to merge it in 5.10. > Right now I have > > 84814460eef9 ("riscv: Fixup bootup failure with HARDENED_USERCOPY") > a78c6f5956a9 ("RISC-V: Make sure memblock reserves the memory containing DT") > 549738f15da0 ("Linux 5.9-rc8") > > Unless there's some functional bug, that's what I'm going to send out for 5.9 > -- I'm not all that worried about lacking the ability to free init data. The > above seems like fine 5.10 material. > > Let me know if I'm missing something here. -- Regards, Atish _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] riscv: Fixup static_obj() fail v2 2020-10-08 3:54 ` Palmer Dabbelt 2020-10-08 17:57 ` Atish Patra @ 2020-10-09 1:53 ` Guo Ren 2020-10-09 21:16 ` Atish Patra 1 sibling, 1 reply; 10+ messages in thread From: Guo Ren @ 2020-10-09 1:53 UTC (permalink / raw) To: Palmer Dabbelt Cc: Guo Ren, Linux Kernel Mailing List, Andreas Schwab, Atish Patra, linux-riscv, aurelien On Thu, Oct 8, 2020 at 11:54 AM Palmer Dabbelt <palmerdabbelt@google.com> wrote: > > On Wed, 07 Oct 2020 08:08:33 PDT (-0700), guoren@kernel.org wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > > > v1 is commit: 6184358da0004c8fd940afda6c0a0fa4027dc911 which has > > been reverted. > > > > When enable LOCKDEP, static_obj() will cause error: > > > > [ 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 > > > > 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; > > > > if (arch_is_kernel_data(addr)) > > return 1; > > > > We could implement arch_is_kernel_data to fixup it. > > > > Link: https://lore.kernel.org/linux-riscv/1593266228-61125-1-git-send-email-guoren@kernel.org/T/#t > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > Reported-by: Aurelien Jarno <aurelien@aurel32.net> > > Cc: Palmer Dabbelt <palmerdabbelt@google.com> > > Cc: Atish Patra <atishp@atishpatra.org> > > Cc: Andreas Schwab <schwab@linux-m68k.org> > > Cc: Aurelien Jarno <aurelien@aurel32.net> > > --- > > arch/riscv/include/asm/sections.h | 20 ++++++++++++++++++++ > > arch/riscv/kernel/setup.c | 9 +++++++++ > > 2 files changed, 29 insertions(+) > > create mode 100644 arch/riscv/include/asm/sections.h > > > > 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; > > +} > > I'm a bit confused as to what you're trying to do here. Yesterday I got > another version of this patch set that moves init data around, today a > different one. Yesterday's is tested and simpler, and given it's so late in > the process I'm inclined to take that as I don't want to break anything. > > Right now I have > > 84814460eef9 ("riscv: Fixup bootup failure with HARDENED_USERCOPY") > a78c6f5956a9 ("RISC-V: Make sure memblock reserves the memory containing DT") > 549738f15da0 ("Linux 5.9-rc8") > > Unless there's some functional bug, that's what I'm going to send out for 5.9 > -- I'm not all that worried about lacking the ability to free init data. The > above seems like fine 5.10 material. > > Let me know if I'm missing something here. 84814460eef9 could resolve the problem and Atish ask for any other idea? So It's another choice, I forgot RFC in prefix. 6184358da0004c8fd940afda6c0a0fa4027dc911("riscv: Fixup static_obj() fail") is a sloppy patch that introduces another problem. Sorry about that. -- 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] 10+ messages in thread
* Re: [PATCH 2/2] riscv: Fixup static_obj() fail v2 2020-10-09 1:53 ` Guo Ren @ 2020-10-09 21:16 ` Atish Patra 2020-10-09 21:23 ` Palmer Dabbelt 0 siblings, 1 reply; 10+ messages in thread From: Atish Patra @ 2020-10-09 21:16 UTC (permalink / raw) To: Guo Ren Cc: Guo Ren, Palmer Dabbelt, Linux Kernel Mailing List, Andreas Schwab, linux-riscv, aurelien On Thu, Oct 8, 2020 at 6:53 PM Guo Ren <guoren@kernel.org> wrote: > > On Thu, Oct 8, 2020 at 11:54 AM Palmer Dabbelt <palmerdabbelt@google.com> wrote: > > > > On Wed, 07 Oct 2020 08:08:33 PDT (-0700), guoren@kernel.org wrote: > > > From: Guo Ren <guoren@linux.alibaba.com> > > > > > > v1 is commit: 6184358da0004c8fd940afda6c0a0fa4027dc911 which has > > > been reverted. > > > > > > When enable LOCKDEP, static_obj() will cause error: > > > > > > [ 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 > > > > > > 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; > > > > > > if (arch_is_kernel_data(addr)) > > > return 1; > > > > > > We could implement arch_is_kernel_data to fixup it. > > > > > > Link: https://lore.kernel.org/linux-riscv/1593266228-61125-1-git-send-email-guoren@kernel.org/T/#t > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > > Reported-by: Aurelien Jarno <aurelien@aurel32.net> > > > Cc: Palmer Dabbelt <palmerdabbelt@google.com> > > > Cc: Atish Patra <atishp@atishpatra.org> > > > Cc: Andreas Schwab <schwab@linux-m68k.org> > > > Cc: Aurelien Jarno <aurelien@aurel32.net> > > > --- > > > arch/riscv/include/asm/sections.h | 20 ++++++++++++++++++++ > > > arch/riscv/kernel/setup.c | 9 +++++++++ > > > 2 files changed, 29 insertions(+) > > > create mode 100644 arch/riscv/include/asm/sections.h > > > > > > 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; > > > +} > > > > I'm a bit confused as to what you're trying to do here. Yesterday I got > > another version of this patch set that moves init data around, today a > > different one. Yesterday's is tested and simpler, and given it's so late in > > the process I'm inclined to take that as I don't want to break anything. > > > > Right now I have > > > > 84814460eef9 ("riscv: Fixup bootup failure with HARDENED_USERCOPY") > > a78c6f5956a9 ("RISC-V: Make sure memblock reserves the memory containing DT") > > 549738f15da0 ("Linux 5.9-rc8") > > > > Unless there's some functional bug, that's what I'm going to send out for 5.9 > > -- I'm not all that worried about lacking the ability to free init data. The > > above seems like fine 5.10 material. > > > > Let me know if I'm missing something here. > 84814460eef9 could resolve the problem and Atish ask for any other > idea? So It's another choice, I forgot RFC in prefix. > I prefer this fix as it is cleaner and doesn't waste memory. I have sent another series on top of this fix, that addresses the init section protections we talked about. All of these are definitely next merge window material. > 6184358da0004c8fd940afda6c0a0fa4027dc911("riscv: Fixup static_obj() > fail") is a sloppy patch that introduces another problem. Sorry about > that. > > -- > 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 [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] riscv: Fixup static_obj() fail v2 2020-10-09 21:16 ` Atish Patra @ 2020-10-09 21:23 ` Palmer Dabbelt 0 siblings, 0 replies; 10+ messages in thread From: Palmer Dabbelt @ 2020-10-09 21:23 UTC (permalink / raw) To: atishp; +Cc: guoren, linux-kernel, schwab, guoren, linux-riscv, aurelien On Fri, 09 Oct 2020 14:16:00 PDT (-0700), atishp@atishpatra.org wrote: > On Thu, Oct 8, 2020 at 6:53 PM Guo Ren <guoren@kernel.org> wrote: >> >> On Thu, Oct 8, 2020 at 11:54 AM Palmer Dabbelt <palmerdabbelt@google.com> wrote: >> > >> > On Wed, 07 Oct 2020 08:08:33 PDT (-0700), guoren@kernel.org wrote: >> > > From: Guo Ren <guoren@linux.alibaba.com> >> > > >> > > v1 is commit: 6184358da0004c8fd940afda6c0a0fa4027dc911 which has >> > > been reverted. >> > > >> > > When enable LOCKDEP, static_obj() will cause error: >> > > >> > > [ 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 >> > > >> > > 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; >> > > >> > > if (arch_is_kernel_data(addr)) >> > > return 1; >> > > >> > > We could implement arch_is_kernel_data to fixup it. >> > > >> > > Link: https://lore.kernel.org/linux-riscv/1593266228-61125-1-git-send-email-guoren@kernel.org/T/#t >> > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> >> > > Reported-by: Aurelien Jarno <aurelien@aurel32.net> >> > > Cc: Palmer Dabbelt <palmerdabbelt@google.com> >> > > Cc: Atish Patra <atishp@atishpatra.org> >> > > Cc: Andreas Schwab <schwab@linux-m68k.org> >> > > Cc: Aurelien Jarno <aurelien@aurel32.net> >> > > --- >> > > arch/riscv/include/asm/sections.h | 20 ++++++++++++++++++++ >> > > arch/riscv/kernel/setup.c | 9 +++++++++ >> > > 2 files changed, 29 insertions(+) >> > > create mode 100644 arch/riscv/include/asm/sections.h >> > > >> > > 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; >> > > +} >> > >> > I'm a bit confused as to what you're trying to do here. Yesterday I got >> > another version of this patch set that moves init data around, today a >> > different one. Yesterday's is tested and simpler, and given it's so late in >> > the process I'm inclined to take that as I don't want to break anything. >> > >> > Right now I have >> > >> > 84814460eef9 ("riscv: Fixup bootup failure with HARDENED_USERCOPY") >> > a78c6f5956a9 ("RISC-V: Make sure memblock reserves the memory containing DT") >> > 549738f15da0 ("Linux 5.9-rc8") >> > >> > Unless there's some functional bug, that's what I'm going to send out for 5.9 >> > -- I'm not all that worried about lacking the ability to free init data. The >> > above seems like fine 5.10 material. >> > >> > Let me know if I'm missing something here. >> 84814460eef9 could resolve the problem and Atish ask for any other >> idea? So It's another choice, I forgot RFC in prefix. >> > > I prefer this fix as it is cleaner and doesn't waste memory. I have > sent another series > on top of this fix, that addresses the init section protections we > talked about. All of these are definitely > next merge window material. Thanks, I'll take a look. > >> 6184358da0004c8fd940afda6c0a0fa4027dc911("riscv: Fixup static_obj() >> fail") is a sloppy patch that introduces another problem. Sorry about >> that. >> >> -- >> 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] 10+ messages in thread
* Re: [PATCH 1/2] Revert "riscv: Fixup static_obj() fail" 2020-10-07 15:08 [PATCH 1/2] Revert "riscv: Fixup static_obj() fail" guoren 2020-10-07 15:08 ` [PATCH 2/2] riscv: Fixup static_obj() fail v2 guoren @ 2020-10-07 18:52 ` Atish Patra 1 sibling, 0 replies; 10+ messages in thread From: Atish Patra @ 2020-10-07 18:52 UTC (permalink / raw) To: Guo Ren Cc: Guo Ren, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Andreas Schwab, linux-riscv, aurelien On Wed, Oct 7, 2020 at 8:09 AM <guoren@kernel.org> wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > This reverts commit 6184358da0004c8fd940afda6c0a0fa4027dc911. > > It will cause bootup failure with HARDENED_USERCOPY. > > As Aurelien has reported: > > [ 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 ]--- > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > Reported-by: Aurelien Jarno <aurelien@aurel32.net> > Cc: Palmer Dabbelt <palmerdabbelt@google.com> > Cc: Atish Patra <atishp@atishpatra.org> > Cc: Andreas Schwab <schwab@linux-m68k.org> > --- > 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 f3586e3..e6f8016 100644 > --- a/arch/riscv/kernel/vmlinux.lds.S > +++ b/arch/riscv/kernel/vmlinux.lds.S > @@ -22,7 +22,6 @@ SECTIONS > /* Beginning of code and text segment */ > . = LOAD_OFFSET; > _start = .; > - _stext = .; > HEAD_TEXT_SECTION > . = ALIGN(PAGE_SIZE); > > @@ -55,6 +54,7 @@ SECTIONS > . = ALIGN(SECTION_ALIGN); > .text : { > _text = .; > + _stext = .; > TEXT_TEXT > SCHED_TEXT > CPUIDLE_TEXT > -- > 2.7.4 > 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] 10+ messages in thread
end of thread, other threads:[~2020-10-09 21:24 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-07 15:08 [PATCH 1/2] Revert "riscv: Fixup static_obj() fail" guoren 2020-10-07 15:08 ` [PATCH 2/2] riscv: Fixup static_obj() fail v2 guoren 2020-10-07 19:46 ` Atish Patra 2020-10-08 2:30 ` Guo Ren 2020-10-08 3:54 ` Palmer Dabbelt 2020-10-08 17:57 ` Atish Patra 2020-10-09 1:53 ` Guo Ren 2020-10-09 21:16 ` Atish Patra 2020-10-09 21:23 ` Palmer Dabbelt 2020-10-07 18:52 ` [PATCH 1/2] Revert "riscv: Fixup static_obj() fail" Atish Patra
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).