mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] Restrictions on child context after multithreaded fork
@ 2020-08-14 21:41 Rich Felker
  2020-08-14 22:02 ` Florian Weimer
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Rich Felker @ 2020-08-14 21:41 UTC (permalink / raw)
  To: musl

musl 1.2.1 has exposed bugs in several applications and libraries
caused by async-signal-unsafe code between (multithreaded) fork and
subsequent exec. So far, dbus library code, pulseaudio library code,
and libvirt have been found to be affected. A couple of the bug
reports (with incomplete information) are:

https://gitlab.alpinelinux.org/alpine/aports/-/issues/11602
https://gitlab.alpinelinux.org/alpine/aports/-/issues/11815

Fixing the affected library code looks very straightforward; it's just
a matter of doing proper iterations of existing data/state rather than
allocating lists and using opendir on procfs and such. I've discussed
fixes with Alpine Linux folks and I believe fixes have been tested,
but I don't see any patches in aports yet.

I've seen suspicions that the switch to mallocng exposed this, but I'm
pretty sure it was:

https://git.musl-libc.org/cgit/musl/commit/?id=e01b5939b38aea5ecbe41670643199825874b26c

Before this commit, the (incorrect) lock skipping logic allowed the
child process to access inconsistent state left from the parent if it
violated the requirement not to call AS-unsafe functions. Now, the
lock attempt in the child rightly deadlocks before accessing state
that was being modified under control of the lock in the parent. This
is not specific to malloc but common with anything using libc-internal
locks.

I'll follow up on this thread once there are patches for the known
affected libraries.

Note that this is a type of bug that's possibly hard to get upstreams
to take seriously. libvirt in particular, despite having multiple
comments throughout the source warning developers that they can't do
anything AS-unsafe between fork and exec, is somehow deeming malloc an
exception to that rule because they want to use it (despite it clearly
not being necessary).

And the dbus issue has been known for a long time; see open bug:

https://gitlab.freedesktop.org/dbus/dbus/-/issues/173
(originally: https://bugs.freedesktop.org/show_bug.cgi?id=100843)

This is largely because glibc attempts to make the erroneous usage by
these libraries work (more on that below).

The next issue of POSIX (Issue 8) will drop the requirement that fork
be AS-safe, as a result of Austin Group tracker issue #62. This makes
the glibc behavior permissible/conforming, but there does not seem to
be any effort on the POSIX side to drop the requirement on
applications not to do AS-unsafe things in the child before exec, so
regardless of this change, what these libraries are doing is still
wrong.

In order to make the child environment unrestricted after fork, either
fork must hold *all* locks at the time the actual fork syscall takes
place, or it must be able to reset any state protected by a lock that
was held in the parent (or some mix of the two). It's fundamentally
impossible to do this completely (in a way that lets the child run
unrestricted), since some locks in the parent may be held arbitrarily
long such that fork waiting on them would deadlock. In particular, any
stdio FILE lock may be held indefinitely because there's a blocking
operation in progress on the underlying fd, or because the application
has called flockfile. Thus, at best, the implementation can give the
child an environment where fflush(0) and exit() still deadlock.

In case we do want to follow a direction of trying to provide some
degree of relaxation of restrictions on the child (taking the liberty
of POSIX-future drop of fork's AS-safety requirement), I did a quick
survey of libc-internal locks, and found:

- at_quick_exit
- atexit
- dlerror
- gettext
- malloc
- pthread_atfork (already necessarily held at fork)
- random
- sem_open
- stdio open file list (vs individual FILEs)
- syslog
- timezone

This list looks tractable. Aside from malloc, whose locks would need
to be taken last since the others may call malloc, these don't seem to
have any lock order dependencies between them, and each one's lock
functions could be provided as strong overrides to weak no-op
definitions in fork.c.


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

end of thread, other threads:[~2020-09-30 18:38 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-14 21:41 [musl] Restrictions on child context after multithreaded fork Rich Felker
2020-08-14 22:02 ` Florian Weimer
2020-08-14 22:14   ` Rich Felker
2020-08-15  0:47 ` A. Wilcox
2020-08-15  2:40   ` Rich Felker
2020-08-15  2:07 ` Ariadne Conill
2020-08-15  3:02   ` Rich Felker
2020-08-15  6:51 ` Timo Teras
2020-08-15 11:51   ` Natanael Copa
2020-08-15 16:25     ` Rich Felker
2020-08-16  1:27       ` Rich Felker
2020-08-16 12:48         ` Natanael Copa
2020-08-16  3:57 ` Rich Felker
2020-08-16  9:10   ` Florian Weimer
2020-08-16 16:56     ` Rich Felker
2020-08-16 17:11       ` Florian Weimer
2020-08-16 18:33         ` Rich Felker
2020-08-16  7:05 ` Pirmin Walthert
2020-08-16 16:55   ` Rich Felker
2020-09-30 18:38 ` 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).