mailing list of musl libc
 help / color / mirror / code / Atom feed
* Re: Hangup calling setuid() from vfork() child
@ 2019-09-30 20:45 Joshua Hudson
  2019-09-30 22:47 ` Rich Felker
  2019-10-01  5:54 ` Florian Weimer
  0 siblings, 2 replies; 10+ messages in thread
From: Joshua Hudson @ 2019-09-30 20:45 UTC (permalink / raw)
  To: musl

> Basically, the vfork() child is in an invalid state and this cannot be repaired without damaging the parent.

Works on glibc just fine.

setuid() is on the list of signal-safe functions.

http://man7.org/linux/man-pages/man7/signal-safety.7.html

How about you call getpid() and check if you're on the process you
think you're on before calling __synccall? Somebody else might have
done syscall(SYS_fork).

> So you might want to enable memory overcommit.

I'm tired of paying the page fault penalty in the parent. It has a
majority of system RAM, and most of the pages are CoW long after the
vfork child hits execve.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Re: Hangup calling setuid() from vfork() child
  2019-09-30 20:45 Hangup calling setuid() from vfork() child Joshua Hudson
@ 2019-09-30 22:47 ` Rich Felker
  2019-10-01  5:54 ` Florian Weimer
  1 sibling, 0 replies; 10+ messages in thread
From: Rich Felker @ 2019-09-30 22:47 UTC (permalink / raw)
  To: Joshua Hudson; +Cc: musl

On Mon, Sep 30, 2019 at 01:45:07PM -0700, Joshua Hudson wrote:
> > Basically, the vfork() child is in an invalid state and this cannot be repaired without damaging the parent.
> 
> Works on glibc just fine.
> 
> setuid() is on the list of signal-safe functions.

AS-safety is not sufficient to make usage after vfork valid.

> http://man7.org/linux/man-pages/man7/signal-safety.7.html
> 
> How about you call getpid() and check if you're on the process you
> think you're on before calling __synccall?

That could be done, but has ABA issues, and it's just the first step
of a game of whack-a-mole and gives a false hope that things that
can't work might work.

> Somebody else might have done syscall(SYS_fork).

Bypassing the fork function to fork by yourself yields a state where
you can't use libc. There will be lots of things broken. Thread's idea
of its own tid is wrong, robust list wrongly registers ownership of
mutexes you don't own, etc.

Presently the clone() function has issues like this too. They should
be analyzed and addressed at some point.

> > So you might want to enable memory overcommit.
> 
> I'm tired of paying the page fault penalty in the parent. It has a
> majority of system RAM, and most of the pages are CoW long after the
> vfork child hits execve.

If at all possible, use posix_spawn. It solves this problem cleanly.

Rich


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Re: Hangup calling setuid() from vfork() child
  2019-09-30 20:45 Hangup calling setuid() from vfork() child Joshua Hudson
  2019-09-30 22:47 ` Rich Felker
@ 2019-10-01  5:54 ` Florian Weimer
  2019-10-01  9:29   ` Szabolcs Nagy
  1 sibling, 1 reply; 10+ messages in thread
From: Florian Weimer @ 2019-10-01  5:54 UTC (permalink / raw)
  To: Joshua Hudson; +Cc: musl

* Joshua Hudson:

>> Basically, the vfork() child is in an invalid state and this cannot
>> be repaired without damaging the parent.
>
> Works on glibc just fine.

Are you sure it's changing the credentials of the right TIDs?

Thanks,
Florian


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Re: Hangup calling setuid() from vfork() child
  2019-10-01  5:54 ` Florian Weimer
@ 2019-10-01  9:29   ` Szabolcs Nagy
  2019-10-01 11:44     ` Rich Felker
  0 siblings, 1 reply; 10+ messages in thread
From: Szabolcs Nagy @ 2019-10-01  9:29 UTC (permalink / raw)
  To: musl; +Cc: Joshua Hudson

* Florian Weimer <fweimer@redhat.com> [2019-10-01 07:54:56 +0200]:
> * Joshua Hudson:
> 
> >> Basically, the vfork() child is in an invalid state and this cannot
> >> be repaired without damaging the parent.
> >
> > Works on glibc just fine.
> 
> Are you sure it's changing the credentials of the right TIDs?

i don't think it works on glibc (or any other linux
libc for that matter) reliably because the child uses
parent data structures to sync with concurrent threads
and the child also clobbers the errno of the parent.

but it will work usually on glibc because the signals
are sent with tgkill which uses getpid + target tid and
that will just fail because of the pid mismatch, i think
glibc will only deadlock if the parent concurrently
fiddles with the thread stack list.

in any case setuid is not supportable after vfork on linux
so i don't think musl should change, glibc may want to make
it fail somehow to ensure users don't get the wrong idea.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Re: Hangup calling setuid() from vfork() child
  2019-10-01  9:29   ` Szabolcs Nagy
@ 2019-10-01 11:44     ` Rich Felker
  0 siblings, 0 replies; 10+ messages in thread
From: Rich Felker @ 2019-10-01 11:44 UTC (permalink / raw)
  To: musl, Joshua Hudson

On Tue, Oct 01, 2019 at 11:29:08AM +0200, Szabolcs Nagy wrote:
> * Florian Weimer <fweimer@redhat.com> [2019-10-01 07:54:56 +0200]:
> > * Joshua Hudson:
> > 
> > >> Basically, the vfork() child is in an invalid state and this cannot
> > >> be repaired without damaging the parent.
> > >
> > > Works on glibc just fine.
> > 
> > Are you sure it's changing the credentials of the right TIDs?
> 
> i don't think it works on glibc (or any other linux
> libc for that matter) reliably because the child uses
> parent data structures to sync with concurrent threads
> and the child also clobbers the errno of the parent.
> 
> but it will work usually on glibc because the signals
> are sent with tgkill which uses getpid + target tid and
> that will just fail because of the pid mismatch, i think
> glibc will only deadlock if the parent concurrently
> fiddles with the thread stack list.
> 
> in any case setuid is not supportable after vfork on linux
                                                    ~~~~~~~~

Note that this entire problem would go away if Linux would finally
give us a working multithreaded credentials-change syscall...

Rich


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Hangup calling setuid() from vfork() child
@ 2019-09-30 19:57 Joshua Hudson
  0 siblings, 0 replies; 10+ messages in thread
From: Joshua Hudson @ 2019-09-30 19:57 UTC (permalink / raw)
  To: musl

>It's simpler than that. The (retired) specification for vfork did not
>allow anything but _exit or execve in the child after vfork, so the
>issue doesn't arise and it works perfectly fine with threads as long
>as you follow the requirement.

I'm reading the man page for vfork and it says what it actually does, that
is overlay the child process on the memory of the calling process.

posix_spawn can't be used in the originating location, and fork() is
hogging too much memory.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Hangup calling setuid() from vfork() child
  2019-09-30 17:39 ` Markus Wichmann
@ 2019-09-30 17:43   ` Rich Felker
  0 siblings, 0 replies; 10+ messages in thread
From: Rich Felker @ 2019-09-30 17:43 UTC (permalink / raw)
  To: musl

On Mon, Sep 30, 2019 at 07:39:28PM +0200, Markus Wichmann wrote:
> On Mon, Sep 30, 2019 at 08:29:16AM -0700, Joshua Hudson wrote:
> > If there is more than one thread and vfork() calls setuid(), musl libc hangs up.
> >
> > void *thfunction(void*ig) {sleep(1000);returnNULL;}
> >
> > int main()
> > {
> >     pthread_t id;
> >     pthread_create(&id, NULL, thfunction, NULL);
> >     if (vfork() == 0) {
> >         setuid(0); /* hangup */
> >         _exit(0);
> >     }
> > }
> 
> That is an interesting interaction between threads and vfork().
> 
> The child process has only one thread, but it doesn't know that. It also
> can't write it down, since it is sharing memory with the parent (it
> would overwrite the parent's variables).
> 
> POSIX no longer defines vfork(), and therefore does not define any
> safety attributes for it. Is it reasonable to define vfork() as unusable
> in a multithreaded process? Calling something as intricate as
> __synccall() in a vfork() child is going to corrupt memory on a large
> scale.

It's simpler than that. The (retired) specification for vfork did not
allow anything but _exit or execve in the child after vfork, so the
issue doesn't arise and it works perfectly fine with threads as long
as you follow the requirement.

> posix_spawn() circumvents the problem by calling the system calls
> directly, BTW.

Yes, posix_spawn should be used if possible. It even has an attribute
to reset ids to the real ones.

Rich


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Hangup calling setuid() from vfork() child
  2019-09-30 15:29 Joshua Hudson
  2019-09-30 17:39 ` Markus Wichmann
@ 2019-09-30 17:41 ` Rich Felker
  1 sibling, 0 replies; 10+ messages in thread
From: Rich Felker @ 2019-09-30 17:41 UTC (permalink / raw)
  To: musl

On Mon, Sep 30, 2019 at 08:29:16AM -0700, Joshua Hudson wrote:
> If there is more than one thread and vfork() calls setuid(), musl libc hangs up.
> 
> void *thfunction(void*ig) {sleep(1000);returnNULL;}
> 
> int main()
> {
>     pthread_t id;
>     pthread_create(&id, NULL, thfunction, NULL);
>     if (vfork() == 0) {
>         setuid(0); /* hangup */
>         _exit(0);
>     }
> }

This is expected; the only legal action after vfork is _exit or
execve. In practice you could probably get by with
syscall(SYS_setuid,0) or similar in the child, but this isn't
supported usage and the specification for vfork has always been clear
that you can't do arbitrary stuff in the child. If you need to, you
should be using fork.

Rich


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Hangup calling setuid() from vfork() child
  2019-09-30 15:29 Joshua Hudson
@ 2019-09-30 17:39 ` Markus Wichmann
  2019-09-30 17:43   ` Rich Felker
  2019-09-30 17:41 ` Rich Felker
  1 sibling, 1 reply; 10+ messages in thread
From: Markus Wichmann @ 2019-09-30 17:39 UTC (permalink / raw)
  To: musl

On Mon, Sep 30, 2019 at 08:29:16AM -0700, Joshua Hudson wrote:
> If there is more than one thread and vfork() calls setuid(), musl libc hangs up.
>
> void *thfunction(void*ig) {sleep(1000);returnNULL;}
>
> int main()
> {
>     pthread_t id;
>     pthread_create(&id, NULL, thfunction, NULL);
>     if (vfork() == 0) {
>         setuid(0); /* hangup */
>         _exit(0);
>     }
> }

That is an interesting interaction between threads and vfork().

The child process has only one thread, but it doesn't know that. It also
can't write it down, since it is sharing memory with the parent (it
would overwrite the parent's variables).

POSIX no longer defines vfork(), and therefore does not define any
safety attributes for it. Is it reasonable to define vfork() as unusable
in a multithreaded process? Calling something as intricate as
__synccall() in a vfork() child is going to corrupt memory on a large
scale.

posix_spawn() circumvents the problem by calling the system calls
directly, BTW.

Ciao,
Markus


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Hangup calling setuid() from vfork() child
@ 2019-09-30 15:29 Joshua Hudson
  2019-09-30 17:39 ` Markus Wichmann
  2019-09-30 17:41 ` Rich Felker
  0 siblings, 2 replies; 10+ messages in thread
From: Joshua Hudson @ 2019-09-30 15:29 UTC (permalink / raw)
  To: musl

If there is more than one thread and vfork() calls setuid(), musl libc hangs up.

void *thfunction(void*ig) {sleep(1000);returnNULL;}

int main()
{
    pthread_t id;
    pthread_create(&id, NULL, thfunction, NULL);
    if (vfork() == 0) {
        setuid(0); /* hangup */
        _exit(0);
    }
}


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-10-01 11:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-30 20:45 Hangup calling setuid() from vfork() child Joshua Hudson
2019-09-30 22:47 ` Rich Felker
2019-10-01  5:54 ` Florian Weimer
2019-10-01  9:29   ` Szabolcs Nagy
2019-10-01 11:44     ` Rich Felker
  -- strict thread matches above, loose matches on Subject: below --
2019-09-30 19:57 Joshua Hudson
2019-09-30 15:29 Joshua Hudson
2019-09-30 17:39 ` Markus Wichmann
2019-09-30 17:43   ` Rich Felker
2019-09-30 17:41 ` Rich Felker

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).