[git commit] hush: fix a bug in FEATURE_SH_STANDALONE=y config. Closes 9186

Denys Vlasenko vda.linux at googlemail.com
Fri Aug 19 16:43:06 UTC 2016


commit: https://git.busybox.net/busybox/commit/?id=215b0ca6e4fe466c6942d21a1bba62d97f2d5e5d
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

Run this in a "sh SCRIPT":

sha256sum /dev/null
echo END

sha256sum is a NOEXEC applet. It runs in a forked child. Then child exit()s.
By this time, entire script is read, and buffered in a FILE object
from fopen("SCRIPT"). But fgetc() did not consume entire input.
exit() lseeks back by -9 bytes, from <eof> to 'e' in 'echo'.
(this may be libc-specific).
This change of fd position *is shared with the parent*!

Now parent can read more, and it thinks there is another "echo END".
End result: two "echo END"s are run.

Fix this by _exit()ing instead.

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 libbb/appletlib.c       |  4 +++-
 shell/hush.c            | 23 ++++++++++++++++++++---
 shell/hush_test/run-all |  3 ++-
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/libbb/appletlib.c b/libbb/appletlib.c
index c341817..f760af2 100644
--- a/libbb/appletlib.c
+++ b/libbb/appletlib.c
@@ -877,7 +877,9 @@ void FAST_FUNC run_applet_no_and_exit(int applet_no, char **argv)
 	}
 	if (ENABLE_FEATURE_SUID)
 		check_suid(applet_no);
-	exit(applet_main[applet_no](argc, argv));
+	xfunc_error_retval = applet_main[applet_no](argc, argv);
+	/* Note: applet_main() may also not return (die on a xfunc or such) */
+	xfunc_die();
 }
 # endif /* NUM_APPLETS > 0 */
 
diff --git a/shell/hush.c b/shell/hush.c
index ab192e2..be5c98a 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -1580,11 +1580,11 @@ static void hush_exit(int exitcode)
 	}
 #endif
 
-#if ENABLE_HUSH_JOB
 	fflush_all();
+#if ENABLE_HUSH_JOB
 	sigexit(- (exitcode & 0xff));
 #else
-	exit(exitcode);
+	_exit(exitcode);
 #endif
 }
 
@@ -6466,7 +6466,23 @@ static void dump_cmd_in_x_mode(char **argv)
  * Never returns.
  * Don't exit() here.  If you don't exec, use _exit instead.
  * The at_exit handlers apparently confuse the calling process,
- * in particular stdin handling.  Not sure why? -- because of vfork! (vda) */
+ * in particular stdin handling. Not sure why? -- because of vfork! (vda)
+ * Also, it was observed that on exit(), fgetc'ed buffered data
+ * gets "unwound" by some libcs, via lseek(fd, -NUM, SEEK_CUR).
+ * With the net effect that even after fork(), not vfork(),
+ * exit() in NOEXECed applet in "sh SCRIPT":
+ *	noexec_applet_here
+ *	echo END_OF_SCRIPT
+ * lseeks fd in input FILE object from EOF to "e" in "echo END_OF_SCRIPT".
+ * This makes "echo END_OF_SCRIPT" executed twice. exexit() is the fix.
+ */
+#if ENABLE_FEATURE_SH_STANDALONE
+static void exexit(void)
+{
+	fflush_all();
+	_exit(xfunc_error_retval);
+}
+#endif
 static void pseudo_exec_argv(nommu_save_t *nommu_save,
 		char **argv, int assignment_cnt,
 		char **argv_expanded) NORETURN;
@@ -6547,6 +6563,7 @@ static NOINLINE void pseudo_exec_argv(nommu_save_t *nommu_save,
 # if BB_MMU /* see above why on NOMMU it is not allowed */
 			if (APPLET_IS_NOEXEC(a)) {
 				debug_printf_exec("running applet '%s'\n", argv[0]);
+				die_func = exexit;
 				run_applet_no_and_exit(a, argv);
 			}
 # endif
diff --git a/shell/hush_test/run-all b/shell/hush_test/run-all
index 64a7abc..837b3f7 100755
--- a/shell/hush_test/run-all
+++ b/shell/hush_test/run-all
@@ -64,11 +64,12 @@ do_test()
 	echo -n "$1/$x:"
 	(
 		"$THIS_SH" "./$x" >"$name.xx" 2>&1
+		r=$?
 		# filter C library differences
 		sed -i \
 			-e "/: invalid option /s:'::g" \
 			"$name.xx"
-		test $? -eq 77 && rm -f "../$1-$x.fail" && exit 77
+		test $r -eq 77 && rm -f "../$1-$x.fail" && exit 77
 		diff -u "$name.xx" "$name.right" >"../$1-$x.fail" && rm -f "$name.xx" "../$1-$x.fail"
 	)
 	case $? in


More information about the busybox-cvs mailing list