[PATCH] LoongArch: Set defaults to exec stack 0.

[PATCH] LoongArch: Set defaults to exec stack 0.

[liuzhensong]

---
 bfd/elfnn-loongarch.c       | 1 +
 ld/testsuite/ld-elf/elf.exp | 1 +
 2 files changed, 2 insertions(+)

diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index 4efe3d9370c..43182ead8cd 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -132,6 +132,7 @@ struct loongarch_elf_link_hash_table
 
 #define elf_backend_want_dynrelro 1
 #define elf_backend_rela_normal 1
+#define elf_backend_default_execstack 0
 
 /* Generate a PLT header.  */
 
diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
index 10c635e9593..b8829b4e3ee 100644
--- a/ld/testsuite/ld-elf/elf.exp
+++ b/ld/testsuite/ld-elf/elf.exp
@@ -187,6 +187,7 @@ proc target_defaults_to_execstack {} {
 	|| [istarget "nios2*-*-*"]
 	|| [istarget "powerpc64*-*-*"]
 	|| [istarget "riscv*-*-*"]
+	|| [istarget "loongarch*-*-*"]
 	|| [istarget "tilegx*-*-*"]
 	|| [istarget "tilepro*-*-*"] } {
 	  return 0
-- 
2.31.1

Re: [PATCH] LoongArch: Set defaults to exec stack 0.

[WANG Xuerui]

On 2022/7/25 10:22, liuzhensong wrote:
> ---
>   bfd/elfnn-loongarch.c       | 1 +
>   ld/testsuite/ld-elf/elf.exp | 1 +
>   2 files changed, 2 insertions(+)
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index 4efe3d9370c..43182ead8cd 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -132,6 +132,7 @@ struct loongarch_elf_link_hash_table
>   
>   #define elf_backend_want_dynrelro 1
>   #define elf_backend_rela_normal 1
> +#define elf_backend_default_execstack 0
>   
>   /* Generate a PLT header.  */
>   
> diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
> index 10c635e9593..b8829b4e3ee 100644
> --- a/ld/testsuite/ld-elf/elf.exp
> +++ b/ld/testsuite/ld-elf/elf.exp
> @@ -187,6 +187,7 @@ proc target_defaults_to_execstack {} {
>   	|| [istarget "nios2*-*-*"]
>   	|| [istarget "powerpc64*-*-*"]
>   	|| [istarget "riscv*-*-*"]
> +	|| [istarget "loongarch*-*-*"]
>   	|| [istarget "tilegx*-*-*"]
>   	|| [istarget "tilepro*-*-*"] } {
>   	  return 0
Looks good to me. I don't know why this isn't the case from the 
beginning... I'll test on Gentoo shortly after.

Re: [PATCH] LoongArch: Set defaults to exec stack 0.

[Xi Ruoyao]

On Mon, 2022-07-25 at 10:24 +0800, WANG Xuerui wrote:
> On 2022/7/25 10:22, liuzhensong wrote:
> > ---
> >   bfd/elfnn-loongarch.c       | 1 +
> >   ld/testsuite/ld-elf/elf.exp | 1 +
> >   2 files changed, 2 insertions(+)
> > 
> > diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> > index 4efe3d9370c..43182ead8cd 100644
> > --- a/bfd/elfnn-loongarch.c
> > +++ b/bfd/elfnn-loongarch.c
> > @@ -132,6 +132,7 @@ struct loongarch_elf_link_hash_table
> >   
> >   #define elf_backend_want_dynrelro 1
> >   #define elf_backend_rela_normal 1
> > +#define elf_backend_default_execstack 0
> >   
> >   /* Generate a PLT header.  */
> >   
> > diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-
> > elf/elf.exp
> > index 10c635e9593..b8829b4e3ee 100644
> > --- a/ld/testsuite/ld-elf/elf.exp
> > +++ b/ld/testsuite/ld-elf/elf.exp
> > @@ -187,6 +187,7 @@ proc target_defaults_to_execstack {} {
> >         || [istarget "nios2*-*-*"]
> >         || [istarget "powerpc64*-*-*"]
> >         || [istarget "riscv*-*-*"]
> > +       || [istarget "loongarch*-*-*"]
> >         || [istarget "tilegx*-*-*"]
> >         || [istarget "tilepro*-*-*"] } {
> >           return 0
> Looks good to me. I don't know why this isn't the case from the 
> beginning... I'll test on Gentoo shortly after.

I guess it's just noticed recently because ld has started to emit
warning for assembly inputs w/o .note.GNU-stack section.  Before the
warning the only way to notice this problem is "process xxx is started
with executable stack" in the kernel log if someone forgot to add a
.note.GNU-stack section in the assembly source file.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] LoongArch: Set defaults to exec stack 0.

[Fangrui Song]

On 2022-07-25, WANG Xuerui wrote:
>On 2022/7/25 10:22, liuzhensong wrote:
>>---
>>  bfd/elfnn-loongarch.c       | 1 +
>>  ld/testsuite/ld-elf/elf.exp | 1 +
>>  2 files changed, 2 insertions(+)
>>
>>diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
>>index 4efe3d9370c..43182ead8cd 100644
>>--- a/bfd/elfnn-loongarch.c
>>+++ b/bfd/elfnn-loongarch.c
>>@@ -132,6 +132,7 @@ struct loongarch_elf_link_hash_table
>>  #define elf_backend_want_dynrelro 1
>>  #define elf_backend_rela_normal 1
>>+#define elf_backend_default_execstack 0
>>  /* Generate a PLT header.  */
>>diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
>>index 10c635e9593..b8829b4e3ee 100644
>>--- a/ld/testsuite/ld-elf/elf.exp
>>+++ b/ld/testsuite/ld-elf/elf.exp
>>@@ -187,6 +187,7 @@ proc target_defaults_to_execstack {} {
>>  	|| [istarget "nios2*-*-*"]
>>  	|| [istarget "powerpc64*-*-*"]
>>  	|| [istarget "riscv*-*-*"]
>>+	|| [istarget "loongarch*-*-*"]
>>  	|| [istarget "tilegx*-*-*"]
>>  	|| [istarget "tilepro*-*-*"] } {
>>  	  return 0
>Looks good to me. I don't know why this isn't the case from the 
>beginning... I'll test on Gentoo shortly after.

LGTM. Add loongarch before nios2 for an alphabetical order.

Re: [PATCH] LoongArch: Set defaults to exec stack 0.

[Andreas Schwab]

On Jul 25 2022, liuzhensong wrote:

> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index 4efe3d9370c..43182ead8cd 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -132,6 +132,7 @@ struct loongarch_elf_link_hash_table
>  
>  #define elf_backend_want_dynrelro 1
>  #define elf_backend_rela_normal 1
> +#define elf_backend_default_execstack 0

AFAICS, the loongarch kernel defaults to executable stack:

arch/loongarch/include/asm/elf.h:#define elf_read_implies_exec(ex, exec_stk) (exec_stk == EXSTACK_DEFAULT)

Which means this change is wrong.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] LoongArch: Set defaults to exec stack 0.

[Xi Ruoyao]

On Mon, 2022-07-25 at 10:08 +0200, Andreas Schwab via Binutils wrote:
> On Jul 25 2022, liuzhensong wrote:
> 
> > diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> > index 4efe3d9370c..43182ead8cd 100644
> > --- a/bfd/elfnn-loongarch.c
> > +++ b/bfd/elfnn-loongarch.c
> > @@ -132,6 +132,7 @@ struct loongarch_elf_link_hash_table
> >  
> >  #define elf_backend_want_dynrelro 1
> >  #define elf_backend_rela_normal 1
> > +#define elf_backend_default_execstack 0
> 
> AFAICS, the loongarch kernel defaults to executable stack:
> 
> arch/loongarch/include/asm/elf.h:#define elf_read_implies_exec(ex,
> exec_stk) (exec_stk == EXSTACK_DEFAULT)

+ Huacai.

> Which means this change is wrong.

The doc of elf_backend_default_execstack says:

  /* True if an object file lacking a .note.GNU-stack section
     should be assumed to be requesting exec stack.  At least one
     other file in the link needs to have a .note.GNU-stack section
     for a PT_GNU_STACK segment to be created.  */

So I think it only controls the default behavior of bfd library and is
not related to the kernel?
-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] LoongArch: Set defaults to exec stack 0.

[Andreas Schwab]

On Jul 25 2022, Xi Ruoyao wrote:

> The doc of elf_backend_default_execstack says:
>
>   /* True if an object file lacking a .note.GNU-stack section
>      should be assumed to be requesting exec stack.  At least one
>      other file in the link needs to have a .note.GNU-stack section
>      for a PT_GNU_STACK segment to be created.  */
>
> So I think it only controls the default behavior of bfd library and is
> not related to the kernel?

The kernel always controls the ultimate decision.

It also means that if no input file has a GNU-stack note, no
PT_GNU_STACK segment is created for the output, in which case the kernel
default (which is PF_X for loongarch) applies.

The value of DEFAULT_STACK_PERMS in glibc must also agree with the
kernel.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] LoongArch: Set defaults to exec stack 0.

[WANG Xuerui]

On 2022/7/25 16:35, Andreas Schwab via Binutils wrote:
> On Jul 25 2022, Xi Ruoyao wrote:
>
>> The doc of elf_backend_default_execstack says:
>>
>>    /* True if an object file lacking a .note.GNU-stack section
>>       should be assumed to be requesting exec stack.  At least one
>>       other file in the link needs to have a .note.GNU-stack section
>>       for a PT_GNU_STACK segment to be created.  */
>>
>> So I think it only controls the default behavior of bfd library and is
>> not related to the kernel?
> The kernel always controls the ultimate decision.
>
> It also means that if no input file has a GNU-stack note, no
> PT_GNU_STACK segment is created for the output, in which case the kernel
> default (which is PF_X for loongarch) applies.
>
> The value of DEFAULT_STACK_PERMS in glibc must also agree with the
> kernel.
>
Well, actually the kernel behavior seems to be overlooked during the 
upstreaming process. No sane architecture in 2022 should have executable 
stacks in the beginning.

We'll fix this before Linux 5.19 is released, fortunately there's the 
rc8 giving us the chance. Compatibility should not be a problem, as no 
modern program should rely on the incorrect behavior, and the new-world 
is still not widely adopted (read: <50 installations worldwide) so any 
breakage can be chased down and fixed.

Thanks very much for spotting this.

Re: [PATCH] LoongArch: Set defaults to exec stack 0.

[Huacai Chen]

Hi, all,

On Mon, Jul 25, 2022 at 5:17 PM WANG Xuerui <i.swmail@xen0n.name> wrote:
>
> On 2022/7/25 16:35, Andreas Schwab via Binutils wrote:
> > On Jul 25 2022, Xi Ruoyao wrote:
> >
> >> The doc of elf_backend_default_execstack says:
> >>
> >>    /* True if an object file lacking a .note.GNU-stack section
> >>       should be assumed to be requesting exec stack.  At least one
> >>       other file in the link needs to have a .note.GNU-stack section
> >>       for a PT_GNU_STACK segment to be created.  */
> >>
> >> So I think it only controls the default behavior of bfd library and is
> >> not related to the kernel?
> > The kernel always controls the ultimate decision.
> >
> > It also means that if no input file has a GNU-stack note, no
> > PT_GNU_STACK segment is created for the output, in which case the kernel
> > default (which is PF_X for loongarch) applies.
> >
> > The value of DEFAULT_STACK_PERMS in glibc must also agree with the
> > kernel.
> >
> Well, actually the kernel behavior seems to be overlooked during the
> upstreaming process. No sane architecture in 2022 should have executable
> stacks in the beginning.
>
> We'll fix this before Linux 5.19 is released, fortunately there's the
> rc8 giving us the chance. Compatibility should not be a problem, as no
> modern program should rely on the incorrect behavior, and the new-world
> is still not widely adopted (read: <50 installations worldwide) so any
> breakage can be chased down and fixed.
>
> Thanks very much for spotting this.
Thank you very much, I will fix this in kernel side.

Huacai
>

Re: [PATCH] LoongArch: Set defaults to exec stack 0.

[Xi Ruoyao]

On Mon, 2022-07-25 at 17:18 +0800, Huacai Chen wrote:
> Hi, all,
> 
> On Mon, Jul 25, 2022 at 5:17 PM WANG Xuerui <i.swmail@xen0n.name> wrote:
> > 
> > On 2022/7/25 16:35, Andreas Schwab via Binutils wrote:
> > > On Jul 25 2022, Xi Ruoyao wrote:
> > > 
> > > > The doc of elf_backend_default_execstack says:
> > > > 
> > > >    /* True if an object file lacking a .note.GNU-stack section
> > > >       should be assumed to be requesting exec stack.  At least one
> > > >       other file in the link needs to have a .note.GNU-stack section
> > > >       for a PT_GNU_STACK segment to be created.  */
> > > > 
> > > > So I think it only controls the default behavior of bfd library and is
> > > > not related to the kernel?
> > > The kernel always controls the ultimate decision.
> > > 
> > > It also means that if no input file has a GNU-stack note, no
> > > PT_GNU_STACK segment is created for the output, in which case the kernel
> > > default (which is PF_X for loongarch) applies.
> > > 
> > > The value of DEFAULT_STACK_PERMS in glibc must also agree with the
> > > kernel.
> > > 
> > Well, actually the kernel behavior seems to be overlooked during the
> > upstreaming process. No sane architecture in 2022 should have executable
> > stacks in the beginning.

Hmm..  But how about LA32?

"Non-eXecutable bit (NX), 1 bit. This bit is set when a fetch operation
is not allowed on the address space where this page table entry is
located. This control bit is only exist in LA64."
> 

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] LoongArch: Set defaults to exec stack 0.

[WANG Xuerui]

On 2022/7/25 17:22, Xi Ruoyao wrote:
> On Mon, 2022-07-25 at 17:18 +0800, Huacai Chen wrote:
>> Hi, all,
>>
>> On Mon, Jul 25, 2022 at 5:17 PM WANG Xuerui <i.swmail@xen0n.name> wrote:
>>> On 2022/7/25 16:35, Andreas Schwab via Binutils wrote:
>>>> On Jul 25 2022, Xi Ruoyao wrote:
>>>>
>>>>> The doc of elf_backend_default_execstack says:
>>>>>
>>>>>     /* True if an object file lacking a .note.GNU-stack section
>>>>>        should be assumed to be requesting exec stack.  At least one
>>>>>        other file in the link needs to have a .note.GNU-stack section
>>>>>        for a PT_GNU_STACK segment to be created.  */
>>>>>
>>>>> So I think it only controls the default behavior of bfd library and is
>>>>> not related to the kernel?
>>>> The kernel always controls the ultimate decision.
>>>>
>>>> It also means that if no input file has a GNU-stack note, no
>>>> PT_GNU_STACK segment is created for the output, in which case the kernel
>>>> default (which is PF_X for loongarch) applies.
>>>>
>>>> The value of DEFAULT_STACK_PERMS in glibc must also agree with the
>>>> kernel.
>>>>
>>> Well, actually the kernel behavior seems to be overlooked during the
>>> upstreaming process. No sane architecture in 2022 should have executable
>>> stacks in the beginning.
> Hmm..  But how about LA32?
>
> "Non-eXecutable bit (NX), 1 bit. This bit is set when a fetch operation
> is not allowed on the address space where this page table entry is
> located. This control bit is only exist in LA64."

Good catch... In which case the predicate could look like "!cpu_has_nx". 
But the kernel is LA64-only at present, the ILP32 ABIs are not fully 
defined yet, and no LA32 cores are produced in large quantities, so just 
unconditionally disabling the predicate should cause no harm either. I 
think we'd go with the former approach then.

Re: [PATCH] LoongArch: Set defaults to exec stack 0.

[WANG Xuerui]

On 7/25/22 10:22, liuzhensong wrote:
> ---
>   bfd/elfnn-loongarch.c       | 1 +
>   ld/testsuite/ld-elf/elf.exp | 1 +
>   2 files changed, 2 insertions(+)
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index 4efe3d9370c..43182ead8cd 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -132,6 +132,7 @@ struct loongarch_elf_link_hash_table
>   
>   #define elf_backend_want_dynrelro 1
>   #define elf_backend_rela_normal 1
> +#define elf_backend_default_execstack 0
>   
>   /* Generate a PLT header.  */
>   
> diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
> index 10c635e9593..b8829b4e3ee 100644
> --- a/ld/testsuite/ld-elf/elf.exp
> +++ b/ld/testsuite/ld-elf/elf.exp
> @@ -187,6 +187,7 @@ proc target_defaults_to_execstack {} {
>   	|| [istarget "nios2*-*-*"]
>   	|| [istarget "powerpc64*-*-*"]
>   	|| [istarget "riscv*-*-*"]
> +	|| [istarget "loongarch*-*-*"]
>   	|| [istarget "tilegx*-*-*"]
>   	|| [istarget "tilepro*-*-*"] } {
>   	  return 0

Heads-up: this patch can now land, the Linux fix [1] is already upstream 
and will be included in 5.19.

[1]: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/loongarch?id=317980e6b4d03884429f2cdaf51efd28f01b71b0

Re: [PATCH] LoongArch: Set defaults to exec stack 0.

[liuzhensong]

在 2022/7/31 上午3:20, WANG Xuerui 写道:
> On 7/25/22 10:22, liuzhensong wrote:
>> ---
>>   bfd/elfnn-loongarch.c       | 1 +
>>   ld/testsuite/ld-elf/elf.exp | 1 +
>>   2 files changed, 2 insertions(+)
>>
>> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
>> index 4efe3d9370c..43182ead8cd 100644
>> --- a/bfd/elfnn-loongarch.c
>> +++ b/bfd/elfnn-loongarch.c
>> @@ -132,6 +132,7 @@ struct loongarch_elf_link_hash_table
>>     #define elf_backend_want_dynrelro 1
>>   #define elf_backend_rela_normal 1
>> +#define elf_backend_default_execstack 0
>>     /* Generate a PLT header.  */
>>   diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
>> index 10c635e9593..b8829b4e3ee 100644
>> --- a/ld/testsuite/ld-elf/elf.exp
>> +++ b/ld/testsuite/ld-elf/elf.exp
>> @@ -187,6 +187,7 @@ proc target_defaults_to_execstack {} {
>>       || [istarget "nios2*-*-*"]
>>       || [istarget "powerpc64*-*-*"]
>>       || [istarget "riscv*-*-*"]
>> +    || [istarget "loongarch*-*-*"]
>>       || [istarget "tilegx*-*-*"]
>>       || [istarget "tilepro*-*-*"] } {
>>         return 0
>
> Heads-up: this patch can now land, the Linux fix [1] is already 
> upstream and will be included in 5.19.
>
> [1]: 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/loongarch?id=317980e6b4d03884429f2cdaf51efd28f01b71b0

Done.

https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=48ca1b26b99c8fd5bc21f5c69b6b666d9b6b9726