Bug 38212 - [mipsel] posix_spawn with POSIX_SPAWN_RESETIDS does not work
Summary: [mipsel] posix_spawn with POSIX_SPAWN_RESETIDS does not work
Status: ASSIGNED
Alias: None
Product: Sisyphus
Classification: Development
Component: glibc-core (show other bugs)
Version: unstable
Hardware: mipsel Linux
: P5 normal
Assignee: Ivan A. Melnikov
QA Contact: qa-sisyphus
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-12 15:22 MSK by Ivan A. Melnikov
Modified: 2020-04-23 12:31 MSK (History)
5 users (show)

See Also:


Attachments
Reproducer (771 bytes, text/x-csrc)
2020-03-12 15:22 MSK, Ivan A. Melnikov
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ivan A. Melnikov 2020-03-12 15:22:09 MSK
Created attachment 8661 [details]
Reproducer

The attached program (spawn.c) works on my x86_64 Sisyphus machine, but fails on mipsel:

$ gcc -o spawn ./spawn.c && ./spawn
posix_spawn: Operation not permitted


Running the reproducer several times under strace reveals that the first argument to setresuid syscall in the child process (made after fork, before exec) seems to be uninitialized:

[builder@localhost ~]$ strace -o spawn.strace -ff ./spawn
posix_spawn: Operation not permitted
[builder@localhost ~]$ strace -o spawn.strace -ff ./spawn
posix_spawn: Operation not permitted
[builder@localhost ~]$ strace -o spawn.strace -ff ./spawn
posix_spawn: Operation not permitted
[builder@localhost ~]$ strace -o spawn.strace -ff ./spawn
posix_spawn: Operation not permitted
[builder@localhost ~]$ strace -o spawn.strace -ff ./spawn
posix_spawn: Operation not permitted
[builder@localhost ~]$ grep EPERM *.strace*
spawn.strace.27609:setresuid(1529032192, 502, -1)          = -1 EPERM (Operation not permitted)
spawn.strace.27617:setresuid(3483932160, 502, -1)          = -1 EPERM (Operation not permitted)
spawn.strace.27622:setresuid(2563495936, 502, -1)          = -1 EPERM (Operation not permitted)
spawn.strace.27627:setresuid(1103515904, 502, -1)          = -1 EPERM (Operation not permitted)
spawn.strace.27632:setresuid(2333245696, 502, -1)          = -1 EPERM (Operation not permitted)

The first argument should be -1, of course. And it is on x86_64.

The bug is triggered by python 3.8 testsuite, which means that python3-3.8.1-alt1 cannot be build for mipsel.
Comment 1 Ivan A. Melnikov 2020-03-12 15:31:00 MSK
I've taken a quick look through the assembly dump of __spawni_child function (the piece of code that makes the syscall gets inlined into it); the setresuid syscall (4185) is easy to find, and indeed, I cannot find where $a0 is initialized for it. All the relevant INLINE_SYSCALL seems legit, so this may also be a GCC bug...
Comment 2 Ivan A. Melnikov 2020-04-23 12:31:31 MSK
Ok, seems like I'm getting to something.

Here is what the relevant piece of code looks like (sysdeps/posix/spawni.c, line 140):

  /* Set the effective user and group IDs.  */
  if ((attr->__flags & POSIX_SPAWN_RESETIDS) != 0
      && (local_seteuid (__getuid ()) != 0
	  || local_setegid (__getgid ())) != 0)
    goto fail;

We are getting EPERM from the local_setuid call.

local_seteuid is a macro that should call setresuid syscall with three arguments: -1, it's argument, and one more -1. For mipsel platform, it's definition from sysdeps/unix/sysv/linux/local-setxid.h, line 8 is used:

# define local_seteuid(id) INLINE_SYSCALL (setresuid, 3, -1, id, -1)

With a few preprocessor tricks, this INLINE_SYSCALL is expanded into a statement expression that is a rather simple wrapper around an asm volatile statement that with a syscall instruction. Preprocessed and reformatted a bit, local_seteuid (__getuid ()) looks like this:


({
     long _sc_err __attribute__ ((unused));
     long result_var = ({
             long _sys_result;
             {
                 register long __s0 asm ("$16") __attribute__ ((unused)) = ((4000 + 185));
                 register long __v0 asm ("$2");
                 register long __a0 asm ("$4") = (long) (-1);
                 register long __a1 asm ("$5") = (long) (__getuid ());
                 register long __a2 asm ("$6") = (long) (-1);
                 register long __a3 asm ("$7");
                 __asm__ volatile (
                         ".set\tnoreorder\n\t"
                         "li\t%0, %2\t\t\t# " "setresuid" "\n\t"
                         "syscall\n\t"
                         ".set\treorder"
                   : "=r" (__v0), "=r" (__a3)
                   : "IK" ((4000 + 185)), "r" (__a0), "r" (__a1), "r" (__a2)
                   : "$1", "$3", "$8", "$9", "$10", "$11", "$12", "$13", "$14", "$15", "$24", "$25", "hi", "lo", "memory"
                   );
                 _sc_err = __a3; _sys_result = __v0;
               }
               _sys_result;
         });
     if ( ((void) (result_var), (long) (_sc_err)) ) {
             (__libc_errno = (((void) (_sc_err), result_var)));
             result_var = -1L;
     }
     result_var;
})

Note that on mips32 the first 4 syscall arguments are put into registers a0-a3 (r4-r7).

The important thing to see in this preprocessed code is that a0 register is initalized before __getuid() is called. Indeed, this is what we can see if we compile the code with -O0 (you'll have to comment out the #error directive in include/libc-symbols.h):

$LBB11 = .
        .loc 2 189 11
        li      $16,4185                        # 0x1059
        li      $4,-1                   # 0xffffffffffffffff
        lui     $2,%hi(__getuid)
        addiu   $2,$2,%lo(__getuid)
        move    $25,$2
        .reloc  1f,R_MIPS_JALR,__getuid
1:      jalr    $25
$LVL11 = .
        lw      $28,16($fp)
        move    $5,$2
        li      $6,-1                   # 0xffffffffffffffff
#APP
 # 189 "../sysdeps/unix/sysv/linux/spawni.c" 1
        .set    noreorder
        li      $2, 4185                        # setresuid
        syscall
        .set    reorder
 # 0 "" 2
#NO_APP

Fist, $4 AKA a0 register is initialized, then __getuid is called -- as a function. And since, according to o32 ABI, a0 register does not have to be preserved across a function call, optimizer can assume that a0 is changed by __getuid, and drop its initialization. And this is exactly what is does. Here's what we get with -O2:

$LBB106 = .
        .loc 1 189 11 view $LVU329
        .loc 1 189 11 view $LVU330
        .loc 1 189 11 view $LVU331
        .loc 1 189 11 view $LVU332
        addiu   $25,$25,%lo(__getuid)
        .reloc  1f,R_MIPS_JALR,__getuid
1:      jalr    $25
$LVL92 = .
        li      $6,-1                   # 0xffffffffffffffff
        lw      $28,16($sp)
        move    $5,$2
        .loc 1 189 11 view $LVU333
        .loc 1 189 11 view $LVU334
        .loc 1 189 11 view $LVU335
#APP
 # 189 "../sysdeps/unix/sysv/linux/spawni.c" 1
        .set    noreorder
        li      $2, 4185                        # setresuid
        syscall
        .set    reorder
 # 0 "" 2
        .loc 1 189 11 view $LVU336
$LVL93 = .
        .loc 1 189 11 view $LVU337
        .loc 1 189 11 is_stmt 0 view $LVU338
#NO_APP

r4 (AKA a0) initialization is simply not there.

Apparently, internal_syscall2, internal_syscall3 and internal_syscall4 macros from sysdeps/unix/sysv/linux/mips/mips32/sysdep.h should be rewritten to evaluate their arguments before initializing the registers.