Re: Hangup calling setuid() from vfork() child

Re: Hangup calling setuid() from vfork() child

[Joshua Hudson]

>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.

Re: Re: Hangup calling setuid() from vfork() child

[Markus Wichmann]

On Mon, Sep 30, 2019 at 12:57:34PM -0700, Joshua Hudson wrote:
> >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 remembered that while making dinner (after sending my first response).

>
> 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.
>

I don't know about you, but my manpage quite clearly states that vfork()
is equivalent to clone(CLONE_VM | CLONE_VFORK | SIGCHLD), that is:
Parent and child share memory, parent (only the calling thread) is
suspended until child execs or exits, and when it does, the child gets a
SIGCHLD.

If the child process changes anything in memory, that is reflected in
the parent. Basically, the vfork() child is in an invalid state and this
cannot be repaired without damaging the parent.

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

fork() only "hogs" that memory which either parent or child modify
afterwards. You wish to use vfork(), so I guess the child process won't
go long before either exec or exit(), right? So you might want to enable
memory overcommit.

I don't know about your application, but your options are:

- Decouple the child part into another program proper, and use
  posix_spawn() to call it.
- Use fork() and eat the memory cost.
- Use clone() and eat the non-portability. Note that clone(CLONE_VFORK)
  has identical semantics to vfork(), so no calling setuid() there,
  either.

Ciao,
Markus

Re: Re: Hangup calling setuid() from vfork() child

[Szabolcs Nagy]

* Joshua Hudson <joshudson@gmail.com> [2019-09-30 12:57:34 -0700]:
> >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.

the internals don't matter, the interface contract is
that you can only call exec* or _exit in the child.

> 
> posix_spawn can't be used in the originating location, and fork() is

why?

> hogging too much memory.

Re: Re: Hangup calling setuid() from vfork() child

[Rich Felker]

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

Re: Re: Hangup calling setuid() from vfork() child

[Szabolcs Nagy]

* 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.

Re: Re: Hangup calling setuid() from vfork() child

[Florian Weimer]

* 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

Re: Re: Hangup calling setuid() from vfork() child

[Rich Felker]

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