issues w/init

issues w/init

[Kumar Gala]

Ben,

The following patch is causing me issues w/init SEGV on boot.  This is  
a pretty old version of init and I'm wondering what the commit you had  
related to old ABI breakage:

commit 8d30c14cab30d405a05f2aaceda1e9ad57800f36
Author: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date:   Tue Feb 10 16:02:37 2009 +0000

     powerpc/mm: Rework I$/D$ coherency (v3)

     This patch reworks the way we do I and D cache coherency on  
PowerPC.


---
                 /*
                  * Allow execution from readable areas if the MMU  
does not
                  * provide separate controls over reading and  
executing.
+                *
+                * Note: That code used to not be enabled for 4xx/BookE.
+                * It is now as I/D cache coherency for these is done at
+                * set_pte_at() time and I see no reason why the test
+                * below wouldn't be valid on those processors. This - 
may-
+                * break programs compiled with a really old ABI though.
                  */

- k

Re: issues w/init

[Kumar Gala]

On Apr 16, 2009, at 1:21 PM, Kumar Gala wrote:

> Ben,
>
> The following patch is causing me issues w/init SEGV on boot.  This  
> is a pretty old version of init and I'm wondering what the commit  
> you had related to old ABI breakage:
>
> commit 8d30c14cab30d405a05f2aaceda1e9ad57800f36
> Author: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date:   Tue Feb 10 16:02:37 2009 +0000
>
>    powerpc/mm: Rework I$/D$ coherency (v3)
>
>    This patch reworks the way we do I and D cache coherency on  
> PowerPC.
>
>
> ---
>                /*
>                 * Allow execution from readable areas if the MMU  
> does not
>                 * provide separate controls over reading and  
> executing.
> +                *
> +                * Note: That code used to not be enabled for 4xx/ 
> BookE.
> +                * It is now as I/D cache coherency for these is  
> done at
> +                * set_pte_at() time and I see no reason why the test
> +                * below wouldn't be valid on those processors. This  
> -may-
> +                * break programs compiled with a really old ABI  
> though.
>                 */
>
> - k

A bit more debug info that might be helpful, I'm hitting this bad_area  
fault :

                 if (!(vma->vm_flags & VM_EXEC) &&
                     (cpu_has_feature(CPU_FTR_NOEXECUTE) ||
                      !(vma->vm_flags & (VM_READ | VM_WRITE))))
                         goto bad_area;

bad_area 7 = 48024bf4 vm_flags:0810 0873
SEGV 3 address:48024bf4 trap:400 error_code:0

[root:~] cat /proc/1/maps
00100000-00103000 r-xp 00100000 00:00 0          [vdso]
0feab000-0ffbe000 r-xp 00000000 00:0d 7127086    /lib/libc-2.2.5.so
0ffbe000-0ffcb000 ---p 00113000 00:0d 7127086    /lib/libc-2.2.5.so
0ffcb000-0ffeb000 rw-p 00110000 00:0d 7127086    /lib/libc-2.2.5.so
0ffeb000-0fff0000 rw-p 0ffeb000 00:00 0
10000000-10008000 r-xp 00000000 00:0d 9093222    /sbin/init
10017000-10018000 rw-p 00007000 00:0d 9093222    /sbin/init
10018000-1001c000 rwxp 10018000 00:00 0          [heap]
48000000-48013000 r-xp 00000000 00:0d 7127082    /lib/ld-2.2.5.so
48022000-48026000 rw-p 00012000 00:0d 7127082    /lib/ld-2.2.5.so
bfd0e000-bfd23000 rwxp bffeb000 00:00 0          [stack]

Re: issues w/init

[Scott Wood]

Kumar Gala wrote:
> [root:~] cat /proc/1/maps
> 00100000-00103000 r-xp 00100000 00:00 0          [vdso]
> 0feab000-0ffbe000 r-xp 00000000 00:0d 7127086    /lib/libc-2.2.5.so
> 0ffbe000-0ffcb000 ---p 00113000 00:0d 7127086    /lib/libc-2.2.5.so
> 0ffcb000-0ffeb000 rw-p 00110000 00:0d 7127086    /lib/libc-2.2.5.so

IIRC, this libc is old enough to execute out of non-exec memory in some 
cases.

-Scott

Re: issues w/init

[Kumar Gala]

On Apr 16, 2009, at 2:25 PM, Scott Wood wrote:

> Kumar Gala wrote:
>> [root:~] cat /proc/1/maps
>> 00100000-00103000 r-xp 00100000 00:00 0          [vdso]
>> 0feab000-0ffbe000 r-xp 00000000 00:0d 7127086    /lib/libc-2.2.5.so
>> 0ffbe000-0ffcb000 ---p 00113000 00:0d 7127086    /lib/libc-2.2.5.so
>> 0ffcb000-0ffeb000 rw-p 00110000 00:0d 7127086    /lib/libc-2.2.5.so
>
> IIRC, this libc is old enough to execute out of non-exec memory in  
> some cases.

Yeah, I'm going to that realization.  The question is are we ok with  
breaking older versions of libc like this.

- k

Re: issues w/init

[Benjamin Herrenschmidt]

On Thu, 2009-04-16 at 13:21 -0500, Kumar Gala wrote:
> Ben,
> 
> The following patch is causing me issues w/init SEGV on boot.  This is  
> a pretty old version of init and I'm wondering what the commit you had  
> related to old ABI breakage:

Can you test if the binary is trying to execute something that is
in a program section that isn't marked executable ? It could be
that it's build with a very old and broken toolchains.

(Note that when we finally add proper per-page exec permission support,
this will -also- break there even if we avoid that test below).

Maybe we can make a .config option for supporting obsolete crap that
tries to execute out of non executable sections ?

Cheers,
Ben.

> commit 8d30c14cab30d405a05f2aaceda1e9ad57800f36
> Author: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date:   Tue Feb 10 16:02:37 2009 +0000
> 
>      powerpc/mm: Rework I$/D$ coherency (v3)
> 
>      This patch reworks the way we do I and D cache coherency on  
> PowerPC.
> 
> 
> ---
>                  /*
>                   * Allow execution from readable areas if the MMU  
> does not
>                   * provide separate controls over reading and  
> executing.
> +                *
> +                * Note: That code used to not be enabled for 4xx/BookE.
> +                * It is now as I/D cache coherency for these is done at
> +                * set_pte_at() time and I see no reason why the test
> +                * below wouldn't be valid on those processors. This - 
> may-
> +                * break programs compiled with a really old ABI though.
>                   */
> 
> - k

Re: issues w/init

[Benjamin Herrenschmidt]

On Thu, 2009-04-16 at 13:53 -0500, Kumar Gala wrote:

> A bit more debug info that might be helpful, I'm hitting this bad_area  
> fault :
> 
>                  if (!(vma->vm_flags & VM_EXEC) &&
>                      (cpu_has_feature(CPU_FTR_NOEXECUTE) ||
>                       !(vma->vm_flags & (VM_READ | VM_WRITE))))
>                          goto bad_area;
> 
> bad_area 7 = 48024bf4 vm_flags:0810 0873
> SEGV 3 address:48024bf4 trap:400 error_code:0

Right, it's trying to execute off the data section (brobably just a blrl
instruction, that's what it used to do). You can see the VMA at 48022000
it's trying to execute from doesn't have the x bit set.

Toolchains were fixed, afaik, a while ago to properly mark the bit where
it does that executable, though the whole 32-bit ABI also got somewhat
overhauled to remove the need for that junk in the first place. I don't
remember the details off hand.

If we really want to support that old crap, then we probably need a
config option of some sort to force 32-bit to allow execution from
readable pages as I don't think we can identify such broken
binaries at runtime. Note that I'd be surprised if those binaries worked
under a 64-bit kernel, do you have a G5 you can try on ?

> [root:~] cat /proc/1/maps
> 00100000-00103000 r-xp 00100000 00:00 0          [vdso]
> 0feab000-0ffbe000 r-xp 00000000 00:0d 7127086    /lib/libc-2.2.5.so
> 0ffbe000-0ffcb000 ---p 00113000 00:0d 7127086    /lib/libc-2.2.5.so
> 0ffcb000-0ffeb000 rw-p 00110000 00:0d 7127086    /lib/libc-2.2.5.so
> 0ffeb000-0fff0000 rw-p 0ffeb000 00:00 0
> 10000000-10008000 r-xp 00000000 00:0d 9093222    /sbin/init
> 10017000-10018000 rw-p 00007000 00:0d 9093222    /sbin/init
> 10018000-1001c000 rwxp 10018000 00:00 0          [heap]
> 48000000-48013000 r-xp 00000000 00:0d 7127082    /lib/ld-2.2.5.so
> 48022000-48026000 rw-p 00012000 00:0d 7127082    /lib/ld-2.2.5.so
> bfd0e000-bfd23000 rwxp bffeb000 00:00 0          [stack]

Re: issues w/init

[Benjamin Herrenschmidt]

On Thu, 2009-04-16 at 14:27 -0500, Kumar Gala wrote:
> On Apr 16, 2009, at 2:25 PM, Scott Wood wrote:
> 
> > Kumar Gala wrote:
> >> [root:~] cat /proc/1/maps
> >> 00100000-00103000 r-xp 00100000 00:00 0          [vdso]
> >> 0feab000-0ffbe000 r-xp 00000000 00:0d 7127086    /lib/libc-2.2.5.so
> >> 0ffbe000-0ffcb000 ---p 00113000 00:0d 7127086    /lib/libc-2.2.5.so
> >> 0ffcb000-0ffeb000 rw-p 00110000 00:0d 7127086    /lib/libc-2.2.5.so
> >
> > IIRC, this libc is old enough to execute out of non-exec memory in  
> > some cases.
> 
> Yeah, I'm going to that realization.  The question is are we ok with  
> breaking older versions of libc like this.

I'm pretty sure they are broken already when running off a 64-bit
kernel, but yeah, as I said, we may want a .config option to enable
support for that old junk which also disables support from per-page
execute permission.

Ben.

Re: issues w/init

[Paul Mackerras]

Benjamin Herrenschmidt writes:

> I'm pretty sure they are broken already when running off a 64-bit
> kernel,

Maybe not... for 64-bit kernels we have our definition of
elf_read_implies_exec in arch/powerpc/include/asm/elf.h, which IIRC
sets the read-implies-exec personality thing on old 32-bit binaries.

Paul.

Re: issues w/init

[Benjamin Herrenschmidt]

On Fri, 2009-04-17 at 20:05 +1000, Paul Mackerras wrote:
> Benjamin Herrenschmidt writes:
> 
> > I'm pretty sure they are broken already when running off a 64-bit
> > kernel,
> 
> Maybe not... for 64-bit kernels we have our definition of
> elf_read_implies_exec in arch/powerpc/include/asm/elf.h, which IIRC
> sets the read-implies-exec personality thing on old 32-bit binaries.

All or old ? ie. can we differenciate the broken from the "good" ones ? 
In which case we can just fix that up here too.

Cheers,
Ben.

Re: issues w/init

[Benjamin Herrenschmidt]

On Fri, 2009-04-17 at 20:05 +1000, Paul Mackerras wrote:
> Benjamin Herrenschmidt writes:
> 
> > I'm pretty sure they are broken already when running off a 64-bit
> > kernel,
> 
> Maybe not... for 64-bit kernels we have our definition of
> elf_read_implies_exec in arch/powerpc/include/asm/elf.h, which IIRC
> sets the read-implies-exec personality thing on old 32-bit binaries.

Ok, so looking at that more closely, if that had worked, we would
have had PROT_EXEC, and thus VM_EXEC added to the vma by mmap... which
apparently didn't occur.

I think what may be happening is that the binary he's running has the
PT_GNU_STACK but his libc doesn't ... the kernel will set the
personality flag based on the binary you are trying to run, not any
library it uses...

Kumar, can you try to instrument that a bit and tell us ? What the
personality flag gets set to and whether mmap sets PROT_EXEC ?

Cheers,
Ben.

Re: issues w/init

[Kumar Gala]

On Apr 17, 2009, at 5:41 AM, Benjamin Herrenschmidt wrote:

> On Fri, 2009-04-17 at 20:05 +1000, Paul Mackerras wrote:
>> Benjamin Herrenschmidt writes:
>>
>>> I'm pretty sure they are broken already when running off a 64-bit
>>> kernel,
>>
>> Maybe not... for 64-bit kernels we have our definition of
>> elf_read_implies_exec in arch/powerpc/include/asm/elf.h, which IIRC
>> sets the read-implies-exec personality thing on old 32-bit binaries.
>
> Ok, so looking at that more closely, if that had worked, we would
> have had PROT_EXEC, and thus VM_EXEC added to the vma by mmap... which
> apparently didn't occur.
>
> I think what may be happening is that the binary he's running has the
> PT_GNU_STACK but his libc doesn't ... the kernel will set the
> personality flag based on the binary you are trying to run, not any
> library it uses...
>
> Kumar, can you try to instrument that a bit and tell us ? What the
> personality flag gets set to and whether mmap sets PROT_EXEC ?

I can and might have already done this, need to double check.  But  
what I was seeing from elf_map is that we don't get PROT_EXEC set for  
this PHDR, but get VM_MAY_EXEC.  I'm not clear on what VM_MAY_EXEC is  
intended for.

- k

Re: issues w/init

[Kumar Gala]

On Apr 17, 2009, at 5:41 AM, Benjamin Herrenschmidt wrote:

> On Fri, 2009-04-17 at 20:05 +1000, Paul Mackerras wrote:
>> Benjamin Herrenschmidt writes:
>>
>>> I'm pretty sure they are broken already when running off a 64-bit
>>> kernel,
>>
>> Maybe not... for 64-bit kernels we have our definition of
>> elf_read_implies_exec in arch/powerpc/include/asm/elf.h, which IIRC
>> sets the read-implies-exec personality thing on old 32-bit binaries.
>
> Ok, so looking at that more closely, if that had worked, we would
> have had PROT_EXEC, and thus VM_EXEC added to the vma by mmap... which
> apparently didn't occur.
>
> I think what may be happening is that the binary he's running has the
> PT_GNU_STACK but his libc doesn't ... the kernel will set the
> personality flag based on the binary you are trying to run, not any
> library it uses...
>
> Kumar, can you try to instrument that a bit and tell us ? What the
> personality flag gets set to and whether mmap sets PROT_EXEC ?

On ppc32 personality is clearly set to 0.  On ppc64 the  
elf_read_implies_exec does work around the issue (ie I implemented  
elf_read_implies_exec as (exec_stk != EXSTACK_DISABLE_X) on ppc32).   
However I think its just masking it.

Hear are the PHDRs for the version of init I'm running:

Program Headers:
   Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg  
Align
   PHDR           0x000034 0x10000034 0x10000034 0x000e0 0x000e0 R E 0x4
   INTERP         0x000114 0x10000114 0x10000114 0x0000d 0x0000d R   0x1
       [Requesting program interpreter: /lib/ld.so.1]
   LOAD           0x000000 0x10000000 0x10000000 0x072a0 0x072a0 R E  
0x10000
   LOAD           0x0072a0 0x100172a0 0x100172a0 0x00358 0x00988 RW   
0x10000
   DYNAMIC        0x0074bc 0x100174bc 0x100174bc 0x000c8 0x000c8 RW  0x4
   NOTE           0x000130 0x10000130 0x10000130 0x00020 0x00020 R    
0x10

I don't think elf_read_implies_exec() is the right solution on ppc32.   
As I asked before, can VM_MAYEXEC be of any use to us?

- k

Re: issues w/init

[Benjamin Herrenschmidt]

On Fri, 2009-04-17 at 08:23 -0500, Kumar Gala wrote:
> I can and might have already done this, need to double check.  But  
> what I was seeing from elf_map is that we don't get PROT_EXEC set for  
> this PHDR, but get VM_MAY_EXEC.  I'm not clear on what VM_MAY_EXEC is  
> intended for.

Looking at a 2.6.29 tree here, I don't see any reference to VM_MAY_EXEC
but I see a VM_MAYEXEC :-) Though it shouldn't be relevant here.

What I see, and what should be checked is:

 1- In binfmt_elf.c, what happens with this statement ?

        if (elf_read_implies_exec(loc->elf_ex, executable_stack))
                current->personality |= READ_IMPLIES_EXEC;

(ie. is the personality flag set ?)

 2- When the phdr's are mmap'ed, do you hit this:

        if ((prot & PROT_READ) && (current->personality & READ_IMPLIES_EXEC))
                if (!(file && (file->f_path.mnt->mnt_flags & MNT_NOEXEC)))
                        prot |= PROT_EXEC;

(Which should later be turned into VM_EXEC by calc_vm_prot_bits().

If any of these isn't happening, then we need to figure out why.

Cheers,
Ben.

Re: issues w/init

[Benjamin Herrenschmidt]

> On ppc32 personality is clearly set to 0.  On ppc64 the  
> elf_read_implies_exec does work around the issue (ie I implemented  
> elf_read_implies_exec as (exec_stk != EXSTACK_DISABLE_X) on ppc32).   
> However I think its just masking it.

How so ? IE, it checks for PT_GNU_STACK, if not present, it then
sets read to imply exec. Things compiled with a fixed toolchain
have that no ?

Ben.

> Hear are the PHDRs for the version of init I'm running:
> 
> Program Headers:
>    Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg  
> Align
>    PHDR           0x000034 0x10000034 0x10000034 0x000e0 0x000e0 R E 0x4
>    INTERP         0x000114 0x10000114 0x10000114 0x0000d 0x0000d R   0x1
>        [Requesting program interpreter: /lib/ld.so.1]
>    LOAD           0x000000 0x10000000 0x10000000 0x072a0 0x072a0 R E  
> 0x10000
>    LOAD           0x0072a0 0x100172a0 0x100172a0 0x00358 0x00988 RW   
> 0x10000
>    DYNAMIC        0x0074bc 0x100174bc 0x100174bc 0x000c8 0x000c8 RW  0x4
>    NOTE           0x000130 0x10000130 0x10000130 0x00020 0x00020 R    
> 0x10
> 
> I don't think elf_read_implies_exec() is the right solution on ppc32.   
> As I asked before, can VM_MAYEXEC be of any use to us?
> 
> - k

Re: issues w/init

[Kumar Gala]

On Apr 17, 2009, at 12:03 PM, Benjamin Herrenschmidt wrote:

> On Fri, 2009-04-17 at 08:23 -0500, Kumar Gala wrote:
>> I can and might have already done this, need to double check.  But
>> what I was seeing from elf_map is that we don't get PROT_EXEC set for
>> this PHDR, but get VM_MAY_EXEC.  I'm not clear on what VM_MAY_EXEC is
>> intended for.
>
> Looking at a 2.6.29 tree here, I don't see any reference to  
> VM_MAY_EXEC
> but I see a VM_MAYEXEC :-) Though it shouldn't be relevant here.
>
> What I see, and what should be checked is:
>
> 1- In binfmt_elf.c, what happens with this statement ?
>
>        if (elf_read_implies_exec(loc->elf_ex, executable_stack))
>                current->personality |= READ_IMPLIES_EXEC;

on ppc32 noting as its elf_read_implies_exec is not implemented.

> (ie. is the personality flag set ?)
>
> 2- When the phdr's are mmap'ed, do you hit this:
>
>        if ((prot & PROT_READ) && (current->personality &  
> READ_IMPLIES_EXEC))
>                if (!(file && (file->f_path.mnt->mnt_flags &  
> MNT_NOEXEC)))
>                        prot |= PROT_EXEC;
>
> (Which should later be turned into VM_EXEC by calc_vm_prot_bits().
>
> If any of these isn't happening, then we need to figure out why.

As I said before current->personality = 0 in ppc32 in the setup I've  
got. (because we don't have elf_read_implies_exec defined).  I can  
define it and doing so makes this all work properly.  I'm just  
questioning if defining elf_read_implies_exec for ppc32 is actually  
the right answer or not.

- k

Re: issues w/init

[Benjamin Herrenschmidt]

On Fri, 2009-04-17 at 12:40 -0500, Kumar Gala wrote:
> > 1- In binfmt_elf.c, what happens with this statement ?
> >
> >        if (elf_read_implies_exec(loc->elf_ex, executable_stack))
> >                current->personality |= READ_IMPLIES_EXEC;
> 
> on ppc32 noting as its elf_read_implies_exec is not implemented.
> 
Right, I got confused, somewhat assumed you were on BookE 64-bit or
something :-) We just need to make 32-bit do the same as 64-bit for now
and that should do the trick.

Ben.