cca7c611f

Denys Vlasenko

2018-01-12

SUCCESS Duration: 143s

Subject

which: fix TODO with NOFORK+malloc_failure misbehaving

No binary (build failed)

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

(log yok)

debianutils/which.c+8 -8

@@ -30,12 +30,15 @@
	int which_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
	int which_main(int argc UNUSED_PARAM, char **argv)
	{
-	const char *env_path;
+	char *env_path;
	int status = 0;
+	/* This sizeof(): bb_default_root_path is shorter than BB_PATH_ROOT_PATH */
+	char buf[sizeof(BB_PATH_ROOT_PATH)];

	env_path = getenv("PATH");
	if (!env_path)
-	env_path = bb_default_root_path;
+	/* env_path must be writable, and must not alloc, so... */
+	env_path = strcpy(buf, bb_default_root_path);

	getopt32(argv, "^" "a" "\0" "-1"/at least one arg/);
	argv += optind;
@@ -51,20 +54,17 @@ int which_main(int argc UNUSED_PARAM, char **argv)
	}
	} else {
	char *path;
-	char *tmp;
	char *p;

-	path = tmp = xstrdup(env_path);
-	//NOFORK FIXME: nested xmallocs (one is inside find_executable())
-	//can leak memory on failure
-	while ((p = find_executable(*argv, &tmp)) != NULL) {
+	path = env_path;
+	/* NOFORK NB: xmalloc inside find_executable(), must have no allocs above! */
+	while ((p = find_executable(*argv, &path)) != NULL) {
	missing = 0;
	puts(p);
	free(p);
	if (!option_mask32) /* -a not set */
	break;
	}
-	free(path);
	}
	status \|= missing;
	} while (*++argv);

include/libbb.h+9 -3

@@ -2005,10 +2005,16 @@ extern const char bb_path_wtmp_file[] ALIGN1;

	#define bb_dev_null "/dev/null"
	extern const char bb_busybox_exec_path[] ALIGN1;
-	/* util-linux manpage says /sbin:/bin:/usr/sbin:/usr/bin,
-	* but I want to save a few bytes here */
-	extern const char bb_PATH_root_path[] ALIGN1; /* "PATH=/sbin:/usr/sbin:/bin:/usr/bin" */
+	/* allow default system PATH to be extended via CFLAGS */
+	#ifndef BB_ADDITIONAL_PATH
+	#define BB_ADDITIONAL_PATH ""
+	#endif
+	#define BB_PATH_ROOT_PATH "PATH=/sbin:/usr/sbin:/bin:/usr/bin" BB_ADDITIONAL_PATH
+	extern const char bb_PATH_root_path[] ALIGN1; /* BB_PATH_ROOT_PATH */
	#define bb_default_root_path (bb_PATH_root_path + sizeof("PATH"))
+	/* util-linux manpage says /sbin:/bin:/usr/sbin:/usr/bin,
+	* but I want to save a few bytes here:
+	*/
	#define bb_default_path (bb_PATH_root_path + sizeof("PATH=/sbin:/usr/sbin"))

	extern const int const_int_0;

libbb/executable.c+10 -8

@@ -25,7 +25,8 @@ int FAST_FUNC file_is_executable(const char *name)
	* you may call find_executable again with this PATHp to continue
	* (if it's not NULL).
	* return NULL otherwise; (PATHp is undefined)
-	* in all cases (*PATHp) contents will be trashed (s/:/NUL/).
+	* in all cases (*PATHp) contents are temporarily modified
+	* but are restored on return (s/:/NUL/ and back).
	*/
	char* FAST_FUNC find_executable(const char filename, char *PATHp)
	{
@@ -41,14 +42,17 @@ char* FAST_FUNC find_executable(const char filename, char *PATHp)

	p = *PATHp;
	while (p) {
+	int ex;
+
	n = strchr(p, ':');
-	if (n)
-	*n++ = '\0';
+	if (n) *n = '\0';
	p = concat_path_file(
	p[0] ? p : ".", /* handle "::" case */
	filename
	);
-	if (file_is_executable(p)) {
+	ex = file_is_executable(p);
+	if (n) *n++ = ':';
+	if (ex) {
	*PATHp = n;
	return p;
	}
@@ -64,10 +68,8 @@ char* FAST_FUNC find_executable(const char filename, char *PATHp)
	*/
	int FAST_FUNC executable_exists(const char *filename)
	{
-	char *path = xstrdup(getenv("PATH"));
-	char *tmp = path;
-	char *ret = find_executable(filename, &tmp);
-	free(path);
+	char *path = getenv("PATH");
+	char *ret = find_executable(filename, &path);
	free(ret);
	return ret != NULL;
	}

libbb/messages.c+1 -7

@@ -6,11 +6,6 @@
	*/
	#include "libbb.h"

-	/* allow default system PATH to be extended via CFLAGS */
-	#ifndef BB_ADDITIONAL_PATH
-	#define BB_ADDITIONAL_PATH ""
-	#endif
-
	/* allow version to be extended, via CFLAGS */
	#ifndef BB_EXTRA_VERSION
	#define BB_EXTRA_VERSION " ("AUTOCONF_TIMESTAMP")"
@@ -36,8 +31,7 @@ const char bb_busybox_exec_path[] ALIGN1 = CONFIG_BUSYBOX_EXEC_PATH;
	const char bb_default_login_shell[] ALIGN1 = LIBBB_DEFAULT_LOGIN_SHELL;
	/* util-linux manpage says /sbin:/bin:/usr/sbin:/usr/bin,
	* but I want to save a few bytes here. Check libbb.h before changing! */
-	const char bb_PATH_root_path[] ALIGN1 =
-	"PATH=/sbin:/usr/sbin:/bin:/usr/bin" BB_ADDITIONAL_PATH;
+	const char bb_PATH_root_path[] ALIGN1 = BB_PATH_ROOT_PATH;


	//const int const_int_1 = 1;