Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-10-07 Thread Guo Ren
On Wed, Oct 7, 2020 at 4:39 AM Atish Patra  wrote:
>
> On Tue, Oct 6, 2020 at 9:46 AM Guo Ren  wrote:
> >
> > On Tue, Oct 6, 2020 at 3:14 AM Atish Patra  wrote:
> > >
> > > On Thu, Sep 24, 2020 at 9:19 AM 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)
> > > > +
> > >
> > > 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 ..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 
+
+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 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -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)
+{
+ 

Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-10-06 Thread Atish Patra
On Tue, Oct 6, 2020 at 9:46 AM Guo Ren  wrote:
>
> On Tue, Oct 6, 2020 at 3:14 AM Atish Patra  wrote:
> >
> > On Thu, Sep 24, 2020 at 9:19 AM 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)
> > > +
> >
> > 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 

Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-10-06 Thread Guo Ren
On Tue, Oct 6, 2020 at 12:39 AM Palmer Dabbelt  wrote:
>
> On Mon, 05 Oct 2020 01:25:22 PDT (-0700), sch...@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-guo...@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/


Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-10-06 Thread Guo Ren
On Tue, Oct 6, 2020 at 3:14 AM Atish Patra  wrote:
>
> On Thu, Sep 24, 2020 at 9:19 AM 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)
> > +
>
> 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  
> > 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, sch...@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-ri...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
>
>
> --
> Regards,
> Atish



-- 
Best Regards
 Guo Ren

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


Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-10-05 Thread Palmer Dabbelt

On Mon, 05 Oct 2020 14:12:44 PDT (-0700), ati...@atishpatra.org wrote:

On Mon, Oct 5, 2020 at 12:46 PM Palmer Dabbelt  wrote:


On Mon, 05 Oct 2020 11:40:54 PDT (-0700), sch...@linux-m68k.org wrote:
> On Okt 05 2020, Palmer Dabbelt wrote:
>
>> On Mon, 05 Oct 2020 01:25:22 PDT (-0700), sch...@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: ffe00036140e ra : ffe00036140e sp : 
ffe004bc7d60
> [ 4561.011679]  gp : ffe00127ee60 tp : ffe1b05d t0 : 
ffe001297ca0
> [ 4561.018886]  t1 : ffe001297c30 t2 :  s0 : 
ffe004bc7d80
> [ 4561.026093]  s1 : ffe3afda a0 : 005b a1 : 
ffe1f7d67588
> [ 4561.033298]  a2 : ffe1f7d6c108 a3 :  a4 : 
ffe43e80
> [ 4561.040506]  a5 : ffe1f7d6be80 a6 : 0144 a7 : 

> [ 4561.047712]  s2 : 0010 s3 :  s4 : 
ffe3afea
> [ 4561.054918]  s5 : ffe1f7e00e80 s6 : 002af4a2c2e0 s7 : 
f000
> [ 4561.062124]  s8 : 003ff000 s9 : ffe19f985400 s10: 
0010
> [ 4561.069329]  s11: ffe1f7e00e80 t3 : 00038fa8 t4 : 
00038fa8
> [ 4561.076533]  t5 : 0001 t6 : ffe00128e062
> [ 4561.081832] status: 00020120 badaddr:  cause: 
0003
> [ 4561.089821] ---[ end trace a7c93e7595e9c2cc ]---
> [ 4561.095589] BUG: Bad rss-counter state mm:c54f4c29 
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-ri...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv


Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-10-05 Thread Atish Patra
On Mon, Oct 5, 2020 at 12:46 PM Palmer Dabbelt  wrote:
>
> On Mon, 05 Oct 2020 11:40:54 PDT (-0700), sch...@linux-m68k.org wrote:
> > On Okt 05 2020, Palmer Dabbelt wrote:
> >
> >> On Mon, 05 Oct 2020 01:25:22 PDT (-0700), sch...@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: ffe00036140e ra : ffe00036140e sp : 
> > ffe004bc7d60
> > [ 4561.011679]  gp : ffe00127ee60 tp : ffe1b05d t0 : 
> > ffe001297ca0
> > [ 4561.018886]  t1 : ffe001297c30 t2 :  s0 : 
> > ffe004bc7d80
> > [ 4561.026093]  s1 : ffe3afda a0 : 005b a1 : 
> > ffe1f7d67588
> > [ 4561.033298]  a2 : ffe1f7d6c108 a3 :  a4 : 
> > ffe43e80
> > [ 4561.040506]  a5 : ffe1f7d6be80 a6 : 0144 a7 : 
> > 
> > [ 4561.047712]  s2 : 0010 s3 :  s4 : 
> > ffe3afea
> > [ 4561.054918]  s5 : ffe1f7e00e80 s6 : 002af4a2c2e0 s7 : 
> > f000
> > [ 4561.062124]  s8 : 003ff000 s9 : ffe19f985400 s10: 
> > 0010
> > [ 4561.069329]  s11: ffe1f7e00e80 t3 : 00038fa8 t4 : 
> > 00038fa8
> > [ 4561.076533]  t5 : 0001 t6 : ffe00128e062
> > [ 4561.081832] status: 00020120 badaddr:  cause: 
> > 0003
> > [ 4561.089821] ---[ end trace a7c93e7595e9c2cc ]---
> > [ 4561.095589] BUG: Bad rss-counter state mm:c54f4c29 
> > 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-ri...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



--
Regards,
Atish


Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-10-05 Thread Palmer Dabbelt

On Mon, 05 Oct 2020 11:40:54 PDT (-0700), sch...@linux-m68k.org wrote:

On Okt 05 2020, Palmer Dabbelt wrote:


On Mon, 05 Oct 2020 01:25:22 PDT (-0700), sch...@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: ffe00036140e ra : ffe00036140e sp : ffe004bc7d60
[ 4561.011679]  gp : ffe00127ee60 tp : ffe1b05d t0 : 
ffe001297ca0
[ 4561.018886]  t1 : ffe001297c30 t2 :  s0 : 
ffe004bc7d80
[ 4561.026093]  s1 : ffe3afda a0 : 005b a1 : 
ffe1f7d67588
[ 4561.033298]  a2 : ffe1f7d6c108 a3 :  a4 : 
ffe43e80
[ 4561.040506]  a5 : ffe1f7d6be80 a6 : 0144 a7 : 

[ 4561.047712]  s2 : 0010 s3 :  s4 : 
ffe3afea
[ 4561.054918]  s5 : ffe1f7e00e80 s6 : 002af4a2c2e0 s7 : 
f000
[ 4561.062124]  s8 : 003ff000 s9 : ffe19f985400 s10: 
0010
[ 4561.069329]  s11: ffe1f7e00e80 t3 : 00038fa8 t4 : 
00038fa8
[ 4561.076533]  t5 : 0001 t6 : ffe00128e062
[ 4561.081832] status: 00020120 badaddr:  cause: 
0003
[ 4561.089821] ---[ end trace a7c93e7595e9c2cc ]---
[ 4561.095589] BUG: Bad rss-counter state mm:c54f4c29 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.


Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-10-05 Thread Atish Patra
On Thu, Sep 24, 2020 at 9:19 AM 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)
> +

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  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, sch...@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-ri...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Regards,
Atish


Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-10-05 Thread Andreas Schwab
On Okt 05 2020, Palmer Dabbelt wrote:

> On Mon, 05 Oct 2020 01:25:22 PDT (-0700), sch...@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: ffe00036140e ra : ffe00036140e sp : ffe004bc7d60
[ 4561.011679]  gp : ffe00127ee60 tp : ffe1b05d t0 : 
ffe001297ca0
[ 4561.018886]  t1 : ffe001297c30 t2 :  s0 : 
ffe004bc7d80
[ 4561.026093]  s1 : ffe3afda a0 : 005b a1 : 
ffe1f7d67588
[ 4561.033298]  a2 : ffe1f7d6c108 a3 :  a4 : 
ffe43e80
[ 4561.040506]  a5 : ffe1f7d6be80 a6 : 0144 a7 : 

[ 4561.047712]  s2 : 0010 s3 :  s4 : 
ffe3afea
[ 4561.054918]  s5 : ffe1f7e00e80 s6 : 002af4a2c2e0 s7 : 
f000
[ 4561.062124]  s8 : 003ff000 s9 : ffe19f985400 s10: 
0010
[ 4561.069329]  s11: ffe1f7e00e80 t3 : 00038fa8 t4 : 
00038fa8
[ 4561.076533]  t5 : 0001 t6 : ffe00128e062
[ 4561.081832] status: 00020120 badaddr:  cause: 
0003
[ 4561.089821] ---[ end trace a7c93e7595e9c2cc ]---
[ 4561.095589] BUG: Bad rss-counter state mm:c54f4c29 type:MM_ANONPAGES 
val:1

Andreas.

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


Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-10-05 Thread Palmer Dabbelt

On Mon, 05 Oct 2020 01:25:22 PDT (-0700), sch...@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.


Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-10-05 Thread Andreas Schwab
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, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."


Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-09-29 Thread Aurelien Jarno
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 

Thanks,
Aurelien

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


Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-09-24 Thread Guo Ren
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  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, sch...@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/


Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-09-24 Thread Andreas Schwab
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, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."


Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-09-14 Thread Aurelien Jarno
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
aurel...@aurel32.net http://www.aurel32.net


Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-09-11 Thread Guo Ren
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  wrote:
>
> Hi,
>
> On 2020-06-27 13:57, guo...@kernel.org wrote:
> > From: Guo Ren 
> >
> > 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] [] walk_stackframe+0x0/0xa4
> > [0.068506] [] show_stack+0x2a/0x34
> > [0.068631] [] dump_stack+0x94/0xca
> > [0.068757] [] register_lock_class+0x5b8/0x5bc
> > [0.068969] [] __lock_acquire+0x6c/0x1d5c
> > [0.069101] [] lock_acquire+0xae/0x312
> > [0.069228] [] _raw_spin_lock_irqsave+0x40/0x5a
> > [0.069357] [] complete+0x1e/0x50
> > [0.069479] [] rest_init+0x1b0/0x28a
> > [0.069660] [] 0xffe016a2
> > [0.069779] [] 0xffe01b84
> > [0.069953] [] 0xffe01092
> >
> > static __initdata DECLARE_COMPLETION(kthreadd_done);
> >
> > noinline void __ref rest_init(void)
> > {
> >   ...
> >   complete(_done);
> >
> > Signed-off-by: Guo Ren 
> > ---
> >  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: ffe0003b5120 ra : ffe0003b5120 sp : 
> ffe07f783ca0
> [4.758960]  gp : ffe000cc7230 tp : ffe07f77cec0 t0 : 
> ffe000cdafc0
> [4.759772]  t1 : 0064 t2 :  s0 : 
> ffe07f783cf0
> [4.760534]  s1 : ffe00095d780 a0 : 005b a1 : 
> 0020
> [4.761309]  a2 : 0005 a3 :  a4 : 
> ffe000c1f340
> [4.761848]  a5 : ffe000c1f340 a6 :  a7 : 
> 0087
> [4.762684]  s2 : ffe000941848 s3 : 0007bfe7 s4 : 
> 000b
> [4.763500]  s5 :  s6 : ffe00091cc00 s7 : 
> f000
> [4.764376]  s8 : 003ff000 s9 : ffe0769f3200 s10: 
> 000b
> [4.765208]  s11: ffe07d548c40 t3 :  t4 : 
> 0001dcd0
> [4.766059]  t5 : ffe000cc8510 t6 : ffe000cd64aa
> [4.766712] status: 0120 badaddr:  cause: 
> 0003
> [4.768308] ---[ end trace 1f8e733e834d4c3e ]---
> [4.769129] Kernel 

Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-09-11 Thread Aurelien Jarno
Hi,

On 2020-06-27 13:57, guo...@kernel.org wrote:
> From: Guo Ren 
> 
> 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] [] walk_stackframe+0x0/0xa4
> [0.068506] [] show_stack+0x2a/0x34
> [0.068631] [] dump_stack+0x94/0xca
> [0.068757] [] register_lock_class+0x5b8/0x5bc
> [0.068969] [] __lock_acquire+0x6c/0x1d5c
> [0.069101] [] lock_acquire+0xae/0x312
> [0.069228] [] _raw_spin_lock_irqsave+0x40/0x5a
> [0.069357] [] complete+0x1e/0x50
> [0.069479] [] rest_init+0x1b0/0x28a
> [0.069660] [] 0xffe016a2
> [0.069779] [] 0xffe01b84
> [0.069953] [] 0xffe01092
> 
> static __initdata DECLARE_COMPLETION(kthreadd_done);
> 
> noinline void __ref rest_init(void)
> {
>   ...
>   complete(_done);
> 
> Signed-off-by: Guo Ren 
> ---
>  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: ffe0003b5120 ra : ffe0003b5120 sp : ffe07f783ca0
[4.758960]  gp : ffe000cc7230 tp : ffe07f77cec0 t0 : 
ffe000cdafc0
[4.759772]  t1 : 0064 t2 :  s0 : 
ffe07f783cf0
[4.760534]  s1 : ffe00095d780 a0 : 005b a1 : 
0020
[4.761309]  a2 : 0005 a3 :  a4 : 
ffe000c1f340
[4.761848]  a5 : ffe000c1f340 a6 :  a7 : 
0087
[4.762684]  s2 : ffe000941848 s3 : 0007bfe7 s4 : 
000b
[4.763500]  s5 :  s6 : ffe00091cc00 s7 : 
f000
[4.764376]  s8 : 003ff000 s9 : ffe0769f3200 s10: 
000b
[4.765208]  s11: ffe07d548c40 t3 :  t4 : 
0001dcd0
[4.766059]  t5 : ffe000cc8510 t6 : ffe000cd64aa
[4.766712] status: 0120 badaddr:  cause: 
0003
[4.768308] ---[ end trace 1f8e733e834d4c3e ]---
[4.769129] Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x000b
[4.770070] SMP: stopping secondary CPUs
[4.771110] ---[ end Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x000b ]---

Note that this is with CONFIG_HARDENED_USERCOPY=y

Aurelien

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


[PATCH V2 1/3] riscv: Fixup static_obj() fail

2020-06-27 Thread guoren
From: Guo Ren 

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] [] walk_stackframe+0x0/0xa4
[0.068506] [] show_stack+0x2a/0x34
[0.068631] [] dump_stack+0x94/0xca
[0.068757] [] register_lock_class+0x5b8/0x5bc
[0.068969] [] __lock_acquire+0x6c/0x1d5c
[0.069101] [] lock_acquire+0xae/0x312
[0.069228] [] _raw_spin_lock_irqsave+0x40/0x5a
[0.069357] [] complete+0x1e/0x50
[0.069479] [] rest_init+0x1b0/0x28a
[0.069660] [] 0xffe016a2
[0.069779] [] 0xffe01b84
[0.069953] [] 0xffe01092

static __initdata DECLARE_COMPLETION(kthreadd_done);

noinline void __ref rest_init(void)
{
...
complete(_done);

Signed-off-by: Guo Ren 
---
 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