posix_spawn issues on i686

posix_spawn issues on i686

[Jeremy Drake]

From https://github.com/msys2/MSYS2-packages/issues/2801

MSYS2 recently rebuilt GNU make 4.3, and I found that after rebuilding, it
broke rather horribly on i686, where any attempt to run a command resulted
in "Invalid argument" errors.  Some debugging revealed that rebuilding
make resulted in it using posix_spawn now instead of vfork.  Passing
--disable-posix-spawn to make's configure script results in a working i686
make.

From the MSYS2 bug report:

"""
For reference, I tried to rebuild "make" in cygwin 32 bit and it has the
same problem:

rebuilding cygport make.cygport all results in a broken make
Adding CYGCONF_ARGS="--disable-posix-spawn" to the cygport file and
rebuilding again results in a good make
A Makefile to reproduce the issue:

all:
	echo hi
"""

In addition, make check fails rather horribly as well.

I know that 32-bit is on the way out, but it is concerning to me that
there is some latent bug lurking in this code path that is apparently not
well exercised.

Re: posix_spawn issues on i686

[Corinna Vinschen]

On Jan 10 11:38, Jeremy Drake via Cygwin wrote:
> >From https://github.com/msys2/MSYS2-packages/issues/2801
> 
> MSYS2 recently rebuilt GNU make 4.3, and I found that after rebuilding, it
> broke rather horribly on i686, where any attempt to run a command resulted
> in "Invalid argument" errors.  Some debugging revealed that rebuilding
> make resulted in it using posix_spawn now instead of vfork.  Passing
> --disable-posix-spawn to make's configure script results in a working i686
> make.
> 
> >From the MSYS2 bug report:
> 
> """
> For reference, I tried to rebuild "make" in cygwin 32 bit and it has the
> same problem:
> 
> rebuilding cygport make.cygport all results in a broken make
> Adding CYGCONF_ARGS="--disable-posix-spawn" to the cygport file and
> rebuilding again results in a good make
> A Makefile to reproduce the issue:
> 
> all:
> 	echo hi
> """
> 
> In addition, make check fails rather horribly as well.
> 
> I know that 32-bit is on the way out, but it is concerning to me that
> there is some latent bug lurking in this code path that is apparently not
> well exercised.

Can you create a simple, self-contained testcase in plain C?


Thanks,
Corinna

Re: posix_spawn issues on i686

[Jeremy Drake]

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

On Mon, 10 Jan 2022, Jeremy Drake wrote:

> From https://github.com/msys2/MSYS2-packages/issues/2801
>
> MSYS2 recently rebuilt GNU make 4.3, and I found that after rebuilding, it
> broke rather horribly on i686, where any attempt to run a command resulted
> in "Invalid argument" errors.  Some debugging revealed that rebuilding
> make resulted in it using posix_spawn now instead of vfork.  Passing
> --disable-posix-spawn to make's configure script results in a working i686
> make.
>

> Can you create a simple, self-contained testcase in plain C?


Sorry, I am not subscribed to the list so don't have the message to reply
to for threading purposes, but attached please find a C reproducer that
works on x86_64 but fails on i686.  The particular issue seems to be the
POSIX_SPAWN_RESETIDS flag - not setting that allows i686 to succeed too.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-c; name=posix_spawn.c, Size: 1148 bytes --]

#include <stdlib.h>
#include <stdio.h>
#include <string.h>

#include <unistd.h>
#include <spawn.h>
#include <sys/wait.h>

extern char **environ;

int main()
{
    pid_t pid;
    char *argv[] = {"sh", "-c", "echo hi", NULL};
    posix_spawnattr_t attr;
    int status;
    short flags = POSIX_SPAWN_RESETIDS;

    if ((status = posix_spawnattr_init(&attr)) != 0) {
        printf("posix_spawnattr_init: %s\n", strerror(status));
	return status;
    }
    if ((status = posix_spawnattr_setflags(&attr, flags)) != 0) {
        printf("posix_spawnattr_setflags: %s\n", strerror(status));
	return status;
    }

    status = posix_spawn(&pid, "/bin/sh", NULL, &attr, argv, environ);
    if (status == 0) {
        printf("Child pid: %i\n", pid);
        do {
          if (waitpid(pid, &status, 0) != -1) {
            printf("Child status %d\n", WEXITSTATUS(status));
          } else {
            perror("waitpid");
            return 1;
          }
        } while (!WIFEXITED(status) && !WIFSIGNALED(status));
    } else {
        printf("posix_spawn: %s\n", strerror(status));
    }
    return status;
}

Re: posix_spawn issues on i686

[Ken Brown]

On 1/11/2022 1:45 PM, Jeremy Drake via Cygwin wrote:
> Sorry, I am not subscribed to the list so don't have the message to reply
> to for threading purposes, but attached please find a C reproducer that
> works on x86_64 but fails on i686.  The particular issue seems to be the
> POSIX_SPAWN_RESETIDS flag - not setting that allows i686 to succeed too.

I don't have time to check this carefully, but it looks to me like the problem 
is that process_spawnattr calls setegid and seteuid instead of setegid32 and 
seteuid32.  This causes truncation of the gid and uid.

Ken

Re: posix_spawn issues on i686

[Corinna Vinschen]

On Jan 11 16:08, Ken Brown wrote:
> On 1/11/2022 1:45 PM, Jeremy Drake via Cygwin wrote:
> > Sorry, I am not subscribed to the list so don't have the message to reply
> > to for threading purposes, but attached please find a C reproducer that
> > works on x86_64 but fails on i686.  The particular issue seems to be the
> > POSIX_SPAWN_RESETIDS flag - not setting that allows i686 to succeed too.

Thanks for the STC, Jeremy!

> I don't have time to check this carefully, but it looks to me like the
> problem is that process_spawnattr calls setegid and seteuid instead of
> setegid32 and seteuid32.  This causes truncation of the gid and uid.

You're right.  Additionally the calls to getgid and getuid have to use
the internal getgid32 and getuid32 functions.

I'll apply a fix.


Thanks guys!
Corinna

Re: posix_spawn issues on i686

[Corinna Vinschen]

On Jan 12 11:41, Corinna Vinschen wrote:
> On Jan 11 16:08, Ken Brown wrote:
> > On 1/11/2022 1:45 PM, Jeremy Drake via Cygwin wrote:
> > > Sorry, I am not subscribed to the list so don't have the message to reply
> > > to for threading purposes, but attached please find a C reproducer that
> > > works on x86_64 but fails on i686.  The particular issue seems to be the
> > > POSIX_SPAWN_RESETIDS flag - not setting that allows i686 to succeed too.
> 
> Thanks for the STC, Jeremy!
> 
> > I don't have time to check this carefully, but it looks to me like the
> > problem is that process_spawnattr calls setegid and seteuid instead of
> > setegid32 and seteuid32.  This causes truncation of the gid and uid.
> 
> You're right.  Additionally the calls to getgid and getuid have to use
> the internal getgid32 and getuid32 functions.
> 
> I'll apply a fix.

New developer snapshot is up at https://cygwin.com/snapshots/
Please test.


Thanks,
Corinna

Re: posix_spawn issues on i686

[Ken Brown]

On 1/12/2022 5:41 AM, Corinna Vinschen wrote:
> On Jan 11 16:08, Ken Brown wrote:
>> I don't have time to check this carefully, but it looks to me like the
>> problem is that process_spawnattr calls setegid and seteuid instead of
>> setegid32 and seteuid32.  This causes truncation of the gid and uid.
> 
> You're right.  Additionally the calls to getgid and getuid have to use
> the internal getgid32 and getuid32 functions.

I'll be glad when this 32-bit cruft can be removed.  It is *very* confusing to 
read the code and see a function called getuid that is really only used by old 
applications, except when it's accidentally used internally.  I introduced a 
32-bit-only bug a couple years ago because of a similar confusion between fstat 
and fstat64.

Ken

Re: posix_spawn issues on i686

[Jeremy Drake]

> > > Sorry, I am not subscribed to the list so don't have the message to reply
> > > to for threading purposes

> New developer snapshot is up at https://cygwin.com/snapshots/
> Please test.

This works, and make's "make check" now gets the same results as it does
when built with --disable-posix-spawn in i686 cygwin.

Thanks

Re: posix_spawn issues on i686

[Corinna Vinschen]

On Jan 12 13:25, Jeremy Drake via Cygwin wrote:
> > > > Sorry, I am not subscribed to the list so don't have the message to reply
> > > > to for threading purposes
> 
> > New developer snapshot is up at https://cygwin.com/snapshots/
> > Please test.
> 
> This works, and make's "make check" now gets the same results as it does
> when built with --disable-posix-spawn in i686 cygwin.

Great, thanks for testing!


Corinna