[PATCH] xvfork: override die_func to _exit on child processes

Dominique Martinet asmadeus at codewreck.org
Wed Nov 8 02:57:57 UTC 2023


From: Dominique Martinet <dominique.martinet at atmark-techno.com>

On alpine linux, calling busybox time with a nonexisting command would
hang forever:
```
time: can't execute 'fdsa': No such file or directory
Command exited with non-zero status 127
real	0m 0.00s
user	0m 0.00s
sys	0m 0.00s
<hang>
```

This is because time uses vfork then BB_EXECVP_or_die, which ultimately
calls exit() after execvp fails. exit() is explicitly listed as unsafe
to call after vfork as the child process shares its memory with the
parent; in particular on musl exit will take the init_fini_lock() and
calling it twice in the same address space will just hang.
_exit() on the other hand is safe to call, so replace die_func with it.

For this particular problem die_func could just be overriden in
time.c's run_command after vfork, but this problem actually came up
before in shell/hush.c as per the fflush_and__exit comment so it seems
more appropriate to do this globally in the xvfork macro and get rid of
this whole class of bugs forever.

Unfortunately, since it is a macro the code change impacts all its
callers. The bloatcheck below depends on the compiler and
build options. This bloatcheck has been obtained with alpine 3.18's
busyboxconfig on debian testing (gcc 12.2.0):

function                                             old     new   delta
vfork_compressor                                     213     233     +20
timeout_main                                         379     399     +20
run_command                                          199     217     +18
xvfork_parent_waits_and_exits                         67      81     +14
launch_helper                                        200     214     +14
dolisten                                             934     948     +14
only__exit                                             -      13     +13
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 6/0 up/down: 113/0)             Total: 113 bytes
   text	   data	    bss	    dec	    hex	filename
 796233	  14332	   1976	 812541	  c65fd	busybox_old
 796360	  14332	   1976	 812668	  c667c	busybox_unstripped
---
 include/libbb.h   | 6 +++++-
 libbb/xfunc_die.c | 4 ++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/libbb.h b/include/libbb.h
index 0883fb565fa9..720271fecba5 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -1228,12 +1228,15 @@ int BB_EXECVP(const char *file, char *const argv[]) FAST_FUNC;
 void BB_EXECVP_or_die(char **argv) NORETURN FAST_FUNC;
 
 /* xvfork() can't be a _function_, return after vfork in child mangles stack
- * in the parent. It must be a macro. */
+ * in the parent. It must be a macro.
+ * Child process must also not call exit so override die func in child */
 #define xvfork() \
 ({ \
 	pid_t bb__xvfork_pid = vfork(); \
 	if (bb__xvfork_pid < 0) \
 		bb_simple_perror_msg_and_die("vfork"); \
+	if (bb__xvfork_pid == 0) \
+		die_func = only__exit; \
 	bb__xvfork_pid; \
 })
 #if BB_MMU
@@ -1426,6 +1429,7 @@ extern smallint logmode;
 extern uint8_t xfunc_error_retval;
 extern void (*die_func)(void);
 void xfunc_die(void) NORETURN FAST_FUNC;
+void only__exit(void) NORETURN;
 void bb_show_usage(void) NORETURN FAST_FUNC;
 void bb_error_msg(const char *s, ...) __attribute__ ((format (printf, 1, 2))) FAST_FUNC;
 void bb_simple_error_msg(const char *s) FAST_FUNC;
diff --git a/libbb/xfunc_die.c b/libbb/xfunc_die.c
index 25b99066df3e..2e67f07801f3 100644
--- a/libbb/xfunc_die.c
+++ b/libbb/xfunc_die.c
@@ -19,3 +19,7 @@ void FAST_FUNC xfunc_die(void)
 		die_func();
 	exit(xfunc_error_retval);
 }
+
+void only__exit(void) {
+	_exit(xfunc_error_retval);
+}
-- 
2.39.2



More information about the busybox mailing list