[musl] Namespace violation in system()?

[musl] Namespace violation in system()?

[Markus Wichmann]

Hi all,

I stumbled upon the source code of system() today. It is this at the
moment:

|int system(const char *cmd)
|{
|	pid_t pid;
|	sigset_t old, reset;
|	struct sigaction sa = { .sa_handler = SIG_IGN }, oldint, oldquit;
|	int status = -1, ret;
|	posix_spawnattr_t attr;
|
|	pthread_testcancel();
|
|	if (!cmd) return 1;
|
|	sigaction(SIGINT, &sa, &oldint);
|	sigaction(SIGQUIT, &sa, &oldquit);
|	sigaddset(&sa.sa_mask, SIGCHLD);
|	sigprocmask(SIG_BLOCK, &sa.sa_mask, &old);
|
|	sigemptyset(&reset);
|	if (oldint.sa_handler != SIG_IGN) sigaddset(&reset, SIGINT);
|	if (oldquit.sa_handler != SIG_IGN) sigaddset(&reset, SIGQUIT);
|	posix_spawnattr_init(&attr);
|	posix_spawnattr_setsigmask(&attr, &old);
|	posix_spawnattr_setsigdefault(&attr, &reset);
|	posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSIGDEF|POSIX_SPAWN_SETSIGMASK);
|	ret = posix_spawn(&pid, "/bin/sh", 0, &attr,
|		(char *[]){"sh", "-c", (char *)cmd, 0}, __environ);
|	posix_spawnattr_destroy(&attr);
|
|	if (!ret) while (waitpid(pid, &status, 0)<0 && errno == EINTR);
|	sigaction(SIGINT, &oldint, NULL);
|	sigaction(SIGQUIT, &oldquit, NULL);
|	sigprocmask(SIG_SETMASK, &old, NULL);
|
|	if (ret) errno = ret;
|	return status;
|}

Aren't all of those calls namespace violations? system() is an ISO-C
function, so the only symbols it is allowed to pull into the link are
other ISO-C functions or hidden double-underscore symbols, right? But
all the functions called here POSIX functions. And while POSIX contains
the rule that posix_* functions are reserved, that is in POSIX, not
ISO-C. And even with that rule, there are all the other calls.

Does someone need to pour out a bucket of underscores over this
function?

Ciao,
Markus

Re: [musl] Namespace violation in system()?

[Rich Felker]

On Thu, May 04, 2023 at 06:33:27PM +0200, Markus Wichmann wrote:
> Hi all,
> 
> I stumbled upon the source code of system() today. It is this at the
> moment:
> 
> |int system(const char *cmd)
> |{
> |	pid_t pid;
> |	sigset_t old, reset;
> |	struct sigaction sa = { .sa_handler = SIG_IGN }, oldint, oldquit;
> |	int status = -1, ret;
> |	posix_spawnattr_t attr;
> |
> |	pthread_testcancel();
> |
> |	if (!cmd) return 1;
> |
> |	sigaction(SIGINT, &sa, &oldint);
> |	sigaction(SIGQUIT, &sa, &oldquit);
> |	sigaddset(&sa.sa_mask, SIGCHLD);
> |	sigprocmask(SIG_BLOCK, &sa.sa_mask, &old);
> |
> |	sigemptyset(&reset);
> |	if (oldint.sa_handler != SIG_IGN) sigaddset(&reset, SIGINT);
> |	if (oldquit.sa_handler != SIG_IGN) sigaddset(&reset, SIGQUIT);
> |	posix_spawnattr_init(&attr);
> |	posix_spawnattr_setsigmask(&attr, &old);
> |	posix_spawnattr_setsigdefault(&attr, &reset);
> |	posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSIGDEF|POSIX_SPAWN_SETSIGMASK);
> |	ret = posix_spawn(&pid, "/bin/sh", 0, &attr,
> |		(char *[]){"sh", "-c", (char *)cmd, 0}, __environ);
> |	posix_spawnattr_destroy(&attr);
> |
> |	if (!ret) while (waitpid(pid, &status, 0)<0 && errno == EINTR);
> |	sigaction(SIGINT, &oldint, NULL);
> |	sigaction(SIGQUIT, &oldquit, NULL);
> |	sigprocmask(SIG_SETMASK, &old, NULL);
> |
> |	if (ret) errno = ret;
> |	return status;
> |}
> 
> Aren't all of those calls namespace violations? system() is an ISO-C
> function, so the only symbols it is allowed to pull into the link are
> other ISO-C functions or hidden double-underscore symbols, right? But
> all the functions called here POSIX functions. And while POSIX contains
> the rule that posix_* functions are reserved, that is in POSIX, not
> ISO-C. And even with that rule, there are all the other calls.
> 
> Does someone need to pour out a bucket of underscores over this
> function?

The behavior of system() is implementation-defined, so we define it as
calling those functions. :-)

Rich

Re: [musl] Namespace violation in system()?

[Petr Skocik]

On 5/4/23 19:52, Rich Felker wrote:
> On Thu, May 04, 2023 at 06:33:27PM +0200, Markus Wichmann wrote:
>> Hi all,
>>
>> I stumbled upon the source code of system() today. It is this at the
>> moment:
>>
>> |int system(const char *cmd)
>> |{
>> |	pid_t pid;
>> |	sigset_t old, reset;
>> |	struct sigaction sa = { .sa_handler = SIG_IGN }, oldint, oldquit;
>> |	int status = -1, ret;
>> |	posix_spawnattr_t attr;
>> |
>> |	pthread_testcancel();
>> |
>> |	if (!cmd) return 1;
>> |
>> |	sigaction(SIGINT, &sa, &oldint);
>> |	sigaction(SIGQUIT, &sa, &oldquit);
>> |	sigaddset(&sa.sa_mask, SIGCHLD);
>> |	sigprocmask(SIG_BLOCK, &sa.sa_mask, &old);
>> |
>> |	sigemptyset(&reset);
>> |	if (oldint.sa_handler != SIG_IGN) sigaddset(&reset, SIGINT);
>> |	if (oldquit.sa_handler != SIG_IGN) sigaddset(&reset, SIGQUIT);
>> |	posix_spawnattr_init(&attr);
>> |	posix_spawnattr_setsigmask(&attr, &old);
>> |	posix_spawnattr_setsigdefault(&attr, &reset);
>> |	posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSIGDEF|POSIX_SPAWN_SETSIGMASK);
>> |	ret = posix_spawn(&pid, "/bin/sh", 0, &attr,
>> |		(char *[]){"sh", "-c", (char *)cmd, 0}, __environ);
>> |	posix_spawnattr_destroy(&attr);
>> |
>> |	if (!ret) while (waitpid(pid, &status, 0)<0 && errno == EINTR);
>> |	sigaction(SIGINT, &oldint, NULL);
>> |	sigaction(SIGQUIT, &oldquit, NULL);
>> |	sigprocmask(SIG_SETMASK, &old, NULL);
>> |
>> |	if (ret) errno = ret;
>> |	return status;
>> |}
>>
>> Aren't all of those calls namespace violations? system() is an ISO-C
>> function, so the only symbols it is allowed to pull into the link are
>> other ISO-C functions or hidden double-underscore symbols, right? But
>> all the functions called here POSIX functions. And while POSIX contains
>> the rule that posix_* functions are reserved, that is in POSIX, not
>> ISO-C. And even with that rule, there are all the other calls.
>>
>> Does someone need to pour out a bucket of underscores over this
>> function?
> The behavior of system() is implementation-defined, so we define it as
> calling those functions. :-)
>
> Rich

Hi. I think Markus Wichmann makes a valid point.

Unless it's valid behavior for

#include <stdio.h>
#include <stdlib.h>
void sigprocmask(void){
     puts("Hi, I'm a user-defined sigprocmask");
     system("echo hello world");
}
int main(void){ sigprocmask(); }

to keep outputting "Hi, I'm a user-defined sigprocmask" until it crashes 
from stack overflow (musl-gcc -static).

Cheers, Petr Skocik

Re: [musl] Namespace violation in system()?

[Rich Felker]

On Thu, May 04, 2023 at 08:53:03PM +0200, Petr Skocik wrote:
> On 5/4/23 19:52, Rich Felker wrote:
> >On Thu, May 04, 2023 at 06:33:27PM +0200, Markus Wichmann wrote:
> >>Hi all,
> >>
> >>I stumbled upon the source code of system() today. It is this at the
> >>moment:
> >>
> >>|int system(const char *cmd)
> >>|{
> >>|	pid_t pid;
> >>|	sigset_t old, reset;
> >>|	struct sigaction sa = { .sa_handler = SIG_IGN }, oldint, oldquit;
> >>|	int status = -1, ret;
> >>|	posix_spawnattr_t attr;
> >>|
> >>|	pthread_testcancel();
> >>|
> >>|	if (!cmd) return 1;
> >>|
> >>|	sigaction(SIGINT, &sa, &oldint);
> >>|	sigaction(SIGQUIT, &sa, &oldquit);
> >>|	sigaddset(&sa.sa_mask, SIGCHLD);
> >>|	sigprocmask(SIG_BLOCK, &sa.sa_mask, &old);
> >>|
> >>|	sigemptyset(&reset);
> >>|	if (oldint.sa_handler != SIG_IGN) sigaddset(&reset, SIGINT);
> >>|	if (oldquit.sa_handler != SIG_IGN) sigaddset(&reset, SIGQUIT);
> >>|	posix_spawnattr_init(&attr);
> >>|	posix_spawnattr_setsigmask(&attr, &old);
> >>|	posix_spawnattr_setsigdefault(&attr, &reset);
> >>|	posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSIGDEF|POSIX_SPAWN_SETSIGMASK);
> >>|	ret = posix_spawn(&pid, "/bin/sh", 0, &attr,
> >>|		(char *[]){"sh", "-c", (char *)cmd, 0}, __environ);
> >>|	posix_spawnattr_destroy(&attr);
> >>|
> >>|	if (!ret) while (waitpid(pid, &status, 0)<0 && errno == EINTR);
> >>|	sigaction(SIGINT, &oldint, NULL);
> >>|	sigaction(SIGQUIT, &oldquit, NULL);
> >>|	sigprocmask(SIG_SETMASK, &old, NULL);
> >>|
> >>|	if (ret) errno = ret;
> >>|	return status;
> >>|}
> >>
> >>Aren't all of those calls namespace violations? system() is an ISO-C
> >>function, so the only symbols it is allowed to pull into the link are
> >>other ISO-C functions or hidden double-underscore symbols, right? But
> >>all the functions called here POSIX functions. And while POSIX contains
> >>the rule that posix_* functions are reserved, that is in POSIX, not
> >>ISO-C. And even with that rule, there are all the other calls.
> >>
> >>Does someone need to pour out a bucket of underscores over this
> >>function?
> >The behavior of system() is implementation-defined, so we define it as
> >calling those functions. :-)
> >
> >Rich
> 
> Hi. I think Markus Wichmann makes a valid point.
> 
> Unless it's valid behavior for
> 
> #include <stdio.h>
> #include <stdlib.h>
> void sigprocmask(void){
>     puts("Hi, I'm a user-defined sigprocmask");
>     system("echo hello world");
> }
> int main(void){ sigprocmask(); }
> 
> to keep outputting "Hi, I'm a user-defined sigprocmask" until it
> crashes from stack overflow (musl-gcc -static).

Yes, it is valid behavior. C doesn't define anything about what the
command in the string means, and in fact there are commands which
would do specifically that regardless of how we implement system. For
example, system("gdb ..."); with an appropriately nasty set of gdb
commands pointed at $PPID or something.

If you want to be assuming there's an echo command with POSIX
semantics, you're writing a POSIX program not a plain C program, and
then redefining sigprocmask has undefined behavior.

Rich

Re: [musl] Namespace violation in system()?

[Rich Felker]

On Thu, May 04, 2023 at 01:52:08PM -0400, Rich Felker wrote:
> On Thu, May 04, 2023 at 06:33:27PM +0200, Markus Wichmann wrote:
> > Hi all,
> > 
> > I stumbled upon the source code of system() today. It is this at the
> > moment:
> > 
> > |int system(const char *cmd)
> > |{
> > |	pid_t pid;
> > |	sigset_t old, reset;
> > |	struct sigaction sa = { .sa_handler = SIG_IGN }, oldint, oldquit;
> > |	int status = -1, ret;
> > |	posix_spawnattr_t attr;
> > |
> > |	pthread_testcancel();
> > |
> > |	if (!cmd) return 1;
> > |
> > |	sigaction(SIGINT, &sa, &oldint);
> > |	sigaction(SIGQUIT, &sa, &oldquit);
> > |	sigaddset(&sa.sa_mask, SIGCHLD);
> > |	sigprocmask(SIG_BLOCK, &sa.sa_mask, &old);
> > |
> > |	sigemptyset(&reset);
> > |	if (oldint.sa_handler != SIG_IGN) sigaddset(&reset, SIGINT);
> > |	if (oldquit.sa_handler != SIG_IGN) sigaddset(&reset, SIGQUIT);
> > |	posix_spawnattr_init(&attr);
> > |	posix_spawnattr_setsigmask(&attr, &old);
> > |	posix_spawnattr_setsigdefault(&attr, &reset);
> > |	posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSIGDEF|POSIX_SPAWN_SETSIGMASK);
> > |	ret = posix_spawn(&pid, "/bin/sh", 0, &attr,
> > |		(char *[]){"sh", "-c", (char *)cmd, 0}, __environ);
> > |	posix_spawnattr_destroy(&attr);
> > |
> > |	if (!ret) while (waitpid(pid, &status, 0)<0 && errno == EINTR);
> > |	sigaction(SIGINT, &oldint, NULL);
> > |	sigaction(SIGQUIT, &oldquit, NULL);
> > |	sigprocmask(SIG_SETMASK, &old, NULL);
> > |
> > |	if (ret) errno = ret;
> > |	return status;
> > |}
> > 
> > Aren't all of those calls namespace violations? system() is an ISO-C
> > function, so the only symbols it is allowed to pull into the link are
> > other ISO-C functions or hidden double-underscore symbols, right? But
> > all the functions called here POSIX functions. And while POSIX contains
> > the rule that posix_* functions are reserved, that is in POSIX, not
> > ISO-C. And even with that rule, there are all the other calls.
> > 
> > Does someone need to pour out a bucket of underscores over this
> > function?
> 
> The behavior of system() is implementation-defined, so we define it as
> calling those functions. :-)

To elaborate:

    "If string is a null pointer, the system function determines
    whether the host environment has a command processor. If string is
    not a null pointer, the system function passes the string pointed
    to by string to that command processor to be executed in a manner
    which the implementation shall document; this might then cause the
    program calling system to behave in a non-conforming manner or to
    terminate."

The manner in which we pass the string to a command processor to be
executed is via calls to external identifiers defined by POSIX.

Even if this sounds like a silly way to be "technically conforming",
it's really not. The semantics of calling system() are completely
unspecified without assuming POSIX or some other specification beyond
just plain C, so there's no reasonable way a C program running on top
of the implementation can call system() without further assumptions
about the implementation (e.g. that it's implementing POSIX).

Rich