[PATCH] Fix execl sentinels

[Rich Felker]

The attached patch fixes calls to execl-type functions to use the
correct sentinel, (char *)0, rather than NULL, which can be defined as
any null pointer constant, and thus has either integer type or
pointer-to-void type, but never pointer-to-character type. The only
reason the current code works is that NULL happens to be defined as
((void *)0), which in turn happens to be passed the same way as a null
character pointer would be, and thus gets successfully processed by
va_arg as char *. However, formally the type is incorrect.

The motivation for this patch was that, in previous versions of musl,
NULL was defined as 0 for C++, and ((void *)0) for C. Our testers
uncovered very subtle, dangerous, difficult-to-track-down bugs on
64-bit systems due to C++ code using NULL (a 32-bit integer 0) as a
sentinel, and ending up with junk in the upper bits of the pointer.
After a long discussion of the best course of action, we chose to
define NULL on musl as 0L for both C and C++. This ensures (assuming
long and pointer have the same size and argument passing convention,
which they do on all real targets we care about) that the "junk in the
high bits" issue won't happen, and at the same time, gets the compiler
to warn about incorrect usage of NULL as a sentinel for variadic
functions, which is a useful warning since such usage is non-portable.

So in summary, I don't think the current code has any chance of
actually breaking on current targets Busybox supports, but it does
generate warnings on musl, and musl's definition of NULL in a way that
causes these warnings to be generated is very intentional (to catch
bugs in applications which might have more stringent portability
requirements than Busybox does). So I think it would be nice to change
it.

Patch attached.

Rich

-------------- next part --------------
diff --git a/archival/libarchive/data_extract_to_command.c b/archival/libarchive/data_extract_to_command.c
index a2ce33b..5b32c2e 100644
--- a/archival/libarchive/data_extract_to_command.c
+++ b/archival/libarchive/data_extract_to_command.c
@@ -103,7 +103,7 @@ void FAST_FUNC data_extract_to_command(archive_handle_t *archive_handle)
 				archive_handle->tar__to_command_shell,
 				"-c",
 				archive_handle->tar__to_command,
-				NULL);
+				(char *)0);
 			bb_perror_msg_and_die("can't execute '%s'", archive_handle->tar__to_command_shell);
 		}
 		close(p[0]);
diff --git a/archival/tar.c b/archival/tar.c
index f46f7bb..c0ceff5 100644
--- a/archival/tar.c
+++ b/archival/tar.c
@@ -567,7 +567,7 @@ static void NOINLINE vfork_compressor(int tar_fd, int gzip)
 		xmove_fd(gzipDataPipe.rd, 0);
 		xmove_fd(tar_fd, 1);
 		/* exec gzip/bzip2 program/applet */
-		BB_EXECLP(zip_exec, zip_exec, "-f", NULL);
+		BB_EXECLP(zip_exec, zip_exec, "-f", (char *)0);
 		vfork_exec_errno = errno;
 		_exit(EXIT_FAILURE);
 	}
diff --git a/loginutils/getty.c b/loginutils/getty.c
index e5d13be..0f060ae 100644
--- a/loginutils/getty.c
+++ b/loginutils/getty.c
@@ -695,6 +695,6 @@ int getty_main(int argc UNUSED_PARAM, char **argv)
 	/* We use PATH because we trust that root doesn't set "bad" PATH,
 	 * and getty is not suid-root applet */
 	/* With -n, logname == NULL, and login will ask for username instead */
-	BB_EXECLP(G.login, G.login, "--", logname, NULL);
+	BB_EXECLP(G.login, G.login, "--", logname, (char *)0);
 	bb_error_msg_and_die("can't execute '%s'", G.login);
 }