Release v3.6.3

This release improves full-file creation to make it thread-safe,
allowing parallelized runs of many instances of swupd_make_fullfiles.

Signed-off-by: Matthew Johnson <matthew.johnson@intel.com>

.travis.yml

@@ -11,9 +11,9 @@ before_install:
         - sudo apt-get install -y libmagic-dev
 
 install:
-        - wget http://downloads.sourceforge.net/project/check/check/0.10.0/check-0.10.0.tar.gz
-        - tar -xvf check-0.10.0.tar.gz
-        - pushd check-0.10.0 && ./configure --prefix=/usr && make -j48 && sudo make install && popd
+        - wget https://github.com/libcheck/check/releases/download/0.11.0/check-0.11.0.tar.gz
+        - tar -xvf check-0.11.0.tar.gz
+        - pushd check-0.11.0 && ./configure --prefix=/usr && make -j48 && sudo make install && popd
         - wget https://github.com/clearlinux/bsdiff/releases/download/v1.0.2/bsdiff-1.0.2.tar.xz
         - tar -xvf bsdiff-1.0.2.tar.xz
         - pushd bsdiff-1.0.2 && ./configure --prefix=/usr --disable-tests && make -j48 && sudo make install && popd

Makefile.am

@@ -111,6 +111,7 @@ dist_check_SCRIPTS = \
 	test/functional/contentsize-across-versions-includes/test.bats \
 	test/functional/delete-no-version-bump/test.bats \
 	test/functional/file-name-blacklisted/test.bats \
+	test/functional/file-name-debuginfo/test.bats \
 	test/functional/format-no-decrement/test.bats \
 	test/functional/full-run-delta/test.bats \
 	test/functional/full-run/test.bats \

configure.ac

@@ -2,7 +2,7 @@
 # Process this file with autoconf to produce a configure script.
 
 AC_PREREQ([2.66])
-AC_INIT(swupd-server, 3.6.0, matthew.johnson@intel.com)
+AC_INIT(swupd-server, 3.6.3, matthew.johnson@intel.com)
 AM_INIT_AUTOMAKE([foreign -Wall -W subdir-objects])
 AM_SILENT_RULES([yes])
 AC_PROG_CC

include/swupd.h

@@ -175,7 +175,9 @@ extern void release_configuration_data(void);
 extern char *config_image_base(void);
 extern char *config_output_dir(void);
 extern char *config_empty_dir(void);
+extern char *config_debuginfo_path(const char *path);
 extern int config_initial_version(void);
+extern bool config_ban_debuginfo(void);
 
 extern void read_current_version(char *filename);
 extern void write_new_version(char *filename, int version);
@@ -260,10 +262,18 @@ extern FILE *fopen_exclusive(const char *filename); /* no mode, opens for write
 extern void dump_file_info(struct file *file);
 extern void string_or_die(char **strp, const char *fmt, ...);
 extern void print_elapsed_time(const char *step, struct timeval *previous_time, struct timeval *current_time);
+extern int system_argv_pipe(char *const lhscmd[], char *const rhscmd[]);
+extern int system_argv_pipe_fd(int lnewstdinfd, int lnewstderrfd, char *const lhscmd[],
+			       int rnewstdoutfd, int rnewstderrfd, char *const rhscmd[]);
+extern void pipe_monitor(int lnewstdinfd, int lnewstderrfd, char *const lhscmd[],
+			 int rnewstdoutfd, int rnewstderrfd, char *const rhscmd[]);
 extern int system_argv(char *const argv[]);
-extern int system_argv_fd(char *const argv[], int newstdin, int newstdout, int newstderr);
-extern int system_argv_pipe(char *const argvp1[], int stdinp1, int stderrp1,
-			    char *const argvp2[], int stdoutp2, int stderrp2);
+extern int system_argv_fd(int newstdinfd, int newstdoutfd, int newstderrfd, char *const cmd[]);
+extern pid_t system_argv_fd_nowait(int newstdinfd, int newstdoutfd, int newstderrfd, int closefd, char *const cmd[]);
+extern void exec_cmd_fd(int newstdinfd, int newstdoutfd, int newstderrfd, int closefd, char *const cmd[]);
+extern void move_fd(int oldfd, int newfd);
+extern int wait_process_terminate(pid_t pid);
 extern int num_threads(float scaling);
+extern bool file_is_debuginfo(const char *path);
 
 #endif

server.ini

@@ -2,3 +2,8 @@
 emptydir=/var/lib/update/empty/
 imagebase=/var/lib/update/image/
 outputdir=/var/lib/update/www/
+
+[Debuginfo]
+banned=true
+lib=/usr/lib/debug/
+src=/usr/src/debug/

src/analyze_fs.c

@@ -314,6 +314,13 @@ static struct file *add_file(struct manifest *manifest,
 	GError *err = NULL;
 	struct file *file;
 
+	if (config_ban_debuginfo() && file_is_debuginfo(sub_filename)) {
+		printf("WARNING: File %s is banned ...skipping.\n", sub_filename);
+		free(sub_filename);
+		free(fullname);
+		return NULL;
+	}
+
 	if (illegal_characters(entry_name)) {
 		printf("WARNING: Filename %s includes illegal character(s) ...skipping.\n", sub_filename);
 		free(sub_filename);

src/config.c

@@ -67,6 +67,20 @@ int config_initial_version(void)
 	return version;
 }
 
+bool config_ban_debuginfo(void)
+{
+	assert(keyfile != NULL);
+
+	return g_key_file_get_boolean(keyfile, "Debuginfo", "banned", NULL);
+}
+
+char *config_debuginfo_path(const char *comp)
+{
+	assert(keyfile != NULL);
+
+	return g_key_file_get_value(keyfile, "Debuginfo", comp, NULL);
+}
+
 bool read_configuration_file(char *filename)
 {
 	GError *error = NULL;

src/fullfiles.c

@@ -50,7 +50,6 @@ static void create_fullfile(struct file *file)
 	struct stat sbuf;
 	char *empty, *indir, *outdir;
 	char *param1, *param2;
-	int stderrfd;
 
 	if (file->is_deleted) {
 		return; /* file got deleted -> by definition we cannot tar it up */
@@ -98,17 +97,13 @@ static void create_fullfile(struct file *file)
 		char *const tarcfcmd[] = { TAR_COMMAND, "-C", dir, TAR_PERM_ATTR_ARGS_STRLIST, "-cf", "-", param1, param2, NULL };
 		char *const tarxfcmd[] = { TAR_COMMAND, "-C", rename_tmpdir, TAR_PERM_ATTR_ARGS_STRLIST, "-xf", "-", NULL };
 
-		stderrfd = open("/dev/null", O_WRONLY);
-		if (stderrfd == -1) {
-			LOG(NULL, "Failed to open /dev/null", "");
-			assert(0);
-		}
-		if (system_argv_pipe(tarcfcmd, -1, stderrfd, tarxfcmd, -1, stderrfd) != 0) {
+		int tarcmdresult = system_argv_pipe(tarcfcmd, tarxfcmd);
+		if (tarcmdresult != 0) {
+			LOG(NULL, "Tar command for copying directory full file failed with code %d", tarcmdresult);
 			assert(0);
 		}
 		free(param1);
 		free(param2);
-		close(stderrfd);
 
 		string_or_die(&rename_source, "%s/%s", rename_tmpdir, base);
 		string_or_die(&rename_target, "%s/%s", rename_tmpdir, file->hash);

src/helpers.c

@@ -149,130 +149,117 @@ void concat_str_array(char **output, char *const argv[])
 	}
 }
 
+int system_argv_pipe(char *const lhscmd[], char *const rhscmd[])
+{
+	return system_argv_pipe_fd(-1, -1, lhscmd, -1, -1, rhscmd);
+}
+
+int system_argv_pipe_fd(int lnewstdinfd, int lnewstderrfd, char *const lhscmd[],
+			int rnewstdoutfd, int rnewstderrfd, char *const rhscmd[])
+{
+	pid_t monitorpid = fork();
+	if (monitorpid == -1) {
+		LOG(NULL, "Failed to create child process to monitor pipe between", "command %s and command %s", lhscmd[0], rhscmd[0]);
+		return -1;
+	} else if (monitorpid == 0) {
+		pipe_monitor(lnewstdinfd, lnewstderrfd, lhscmd, rnewstdoutfd, rnewstderrfd, rhscmd);
+	}
+	return wait_process_terminate(monitorpid);
+}
+
+void pipe_monitor(int lnewstdinfd, int lnewstderrfd, char *const lhscmd[],
+		  int rnewstdoutfd, int rnewstderrfd, char *const rhscmd[])
+{
+	int pipefd[2];
+	if (pipe(pipefd) == -1) {
+		LOG(NULL, "Failed to create a pipe between", "command %s and command %s", lhscmd[0], rhscmd[0]);
+		assert(0);
+	}
+
+	pid_t lhspid = system_argv_fd_nowait(lnewstdinfd, pipefd[1], lnewstderrfd, pipefd[0], lhscmd);
+	pid_t rhspid = system_argv_fd_nowait(pipefd[0], rnewstdoutfd, rnewstderrfd, pipefd[1], rhscmd);
+
+	if (close(pipefd[1]) == -1) {
+		LOG(NULL, "Could not close write end of pipe file descriptor", "%d", pipefd[1]);
+		assert(0);
+	}
+	if (close(pipefd[0]) == -1) {
+		LOG(NULL, "Could not close read end of pipe file descriptor", "%d", pipefd[0]);
+		assert(0);
+	}
+
+	int lhsresult = wait_process_terminate(lhspid);
+	int rhsresult = wait_process_terminate(rhspid);
+	exit(rhsresult != EXIT_SUCCESS ? rhsresult : lhsresult);
+}
+
 int system_argv(char *const argv[])
 {
-	int child_exit_status;
-	pid_t pid;
-	int status = -1;
-
-	pid = fork();
-
-	if (pid == 0) { /* child */
-		execvp(*argv, argv);
-		LOG(NULL, "This line must not be reached", "");
-		assert(0);
-	} else if (pid < 0) {
-		LOG(NULL, "Failed to fork a child process", "");
-		assert(0);
-	} else {
-		pid_t ws = waitpid(pid, &child_exit_status, 0);
-
-		if (ws == -1) {
-			LOG(NULL, "Failed to wait for child process", "");
-			assert(0);
-		}
-
-		if (WIFEXITED(child_exit_status)) {
-			status = WEXITSTATUS(child_exit_status);
-		} else {
-			LOG(NULL, "Child process didn't exit", "");
-			assert(0);
-		}
-
-		if (status != 0) {
-			char *cmdline = NULL;
-
-			concat_str_array(&cmdline, argv);
-			LOG(NULL, "Failed to run command:", "%s", cmdline);
-			free(cmdline);
-		}
-	}
-
-	return status;
+	return system_argv_fd(-1, -1, -1, argv);
 }
 
-int system_argv_fd(char *const argv[], int newstdin, int newstdout, int newstderr)
+int system_argv_fd(int newstdinfd, int newstdoutfd, int newstderrfd, char *const cmd[])
 {
-	int child_exit_status;
-	pid_t pid;
-	int status = -1;
-
-	pid = fork();
-
-	if (pid == 0) { /* child */
-		if (newstdin >= 0) {
-			if (dup2(newstdin, STDIN_FILENO) == -1) {
-				LOG(NULL, "Could not redirect stdin", "");
-				assert(0);
-			}
-			close(newstdin);
-		}
-		if (newstdout >= 0) {
-			if (dup2(newstdout, STDOUT_FILENO) == -1) {
-				LOG(NULL, "Could not redirect stdout", "");
-				assert(0);
-			}
-			close(newstdout);
-		}
-		if (newstderr >= 0) {
-			if (dup2(newstderr, STDERR_FILENO) == -1) {
-				LOG(NULL, "Could not redirect stderr", "");
-				assert(0);
-			}
-			close(newstderr);
-		}
-
-		execvp(*argv, argv);
-		LOG(NULL, "This line must not be reached", "");
-		assert(0);
-	} else if (pid < 0) {
-		LOG(NULL, "Failed to fork a child process", "");
-		assert(0);
-	} else {
-		pid_t ws = waitpid(pid, &child_exit_status, 0);
-
-		if (ws == -1) {
-			LOG(NULL, "Failed to wait for child process", "");
-			assert(0);
-		}
-
-		if (WIFEXITED(child_exit_status)) {
-			status = WEXITSTATUS(child_exit_status);
-		} else {
-			LOG(NULL, "Child process didn't exit", "");
-			assert(0);
-		}
-
-		if (status != 0) {
-			char *cmdline = NULL;
-
-			concat_str_array(&cmdline, argv);
-			LOG(NULL, "Failed to run command:", "%s", cmdline);
-			free(cmdline);
-		}
-	}
-
-	return status;
+	pid_t cmdpid = system_argv_fd_nowait(newstdinfd, newstdoutfd, newstderrfd, -1, cmd);
+	return wait_process_terminate(cmdpid);
 }
 
-int system_argv_pipe(char *const argvp1[], int stdinp1, int stderrp1,
-		     char *const argvp2[], int stdoutp2, int stderrp2)
+pid_t system_argv_fd_nowait(int newstdinfd, int newstdoutfd, int newstderrfd, int closefd, char *const cmd[])
 {
-	int statusp2;
-	int pipefd[2];
-
-	if (pipe(pipefd)) {
-		LOG(NULL, "Failed to create a pipe", "");
-		return -1;
+	pid_t cmdpid = fork();
+	if (cmdpid == -1) {
+		LOG(NULL, "Failed to fork to execute command", "%s", cmd[0]);
+		assert(0);
+	} else if (cmdpid == 0) {
+		exec_cmd_fd(newstdinfd, newstdoutfd, newstderrfd, closefd, cmd);
 	}
-	system_argv_fd(argvp1, stdinp1, pipefd[1], stderrp1);
-	close(pipefd[1]);
-	statusp2 = system_argv_fd(argvp2, pipefd[0], stdoutp2, stderrp2);
-	close(pipefd[0]);
+	return cmdpid;
+}
 
-	/* Returns the status of the failed process if any
-       If both processes failed returns the status of first one */
-	return statusp2;
+void exec_cmd_fd(int newstdinfd, int newstdoutfd, int newstderrfd, int closefd, char *const cmd[])
+{
+	move_fd(newstdinfd, STDIN_FILENO);
+	move_fd(newstdoutfd, STDOUT_FILENO);
+	move_fd(newstderrfd, STDERR_FILENO);
+	if (closefd >= 0 && close(closefd) == -1) {
+		LOG(NULL, "Could not close file descriptor", "%d", closefd);
+		assert(0);
+	}
+	execvp(*cmd, cmd);
+	LOG(NULL, "Command", "%s failed", cmd[0]);
+	assert(0);
+}
+
+void move_fd(int oldfd, int newfd)
+{
+	if (oldfd < 0 || newfd < 0 || oldfd == newfd) {
+		return;
+	}
+	if (dup2(oldfd, newfd) == -1) {
+		LOG(NULL, "Could not create duplicate file descriptor", "%d from %d", newfd, oldfd);
+		assert(0);
+	}
+	if (close(oldfd) == -1) {
+		LOG(NULL, "Could not close file descriptor", "%d", oldfd);
+		assert(0);
+	}
+}
+
+int wait_process_terminate(pid_t pid)
+{
+	int status;
+	do {
+		if (waitpid(pid, &status, 0) == -1) {
+			LOG(NULL, "Failed to wait for PID", "%d", pid);
+			return -1;
+		}
+	} while (!WIFEXITED(status) && !WIFSIGNALED(status));
+	// Exit statuses fall in the range of [0, 255].  Make signal statuses fall in a non-overlapping range starting with 256.
+	if (WIFEXITED(status)) {
+		return WEXITSTATUS(status);
+	} else {
+		return 256 + WTERMSIG(status);
+	}
 }
 
 void check_root(void)
@@ -309,3 +296,31 @@ int num_threads(float scaling)
 
 	return result;
 }
+
+/* This function is called when configuration specifies a ban on debuginfo files
+ * from manifests. Returns true if the passed file path matches the src or lib
+ * debuginfo configuration. */
+bool file_is_debuginfo(const char *path)
+{
+	bool ret = false;
+	char *lib;
+	char *src;
+
+	lib = config_debuginfo_path("lib");
+	src = config_debuginfo_path("src");
+
+	if (lib && (strncmp(path, lib, strlen(lib)) == 0)) {
+		ret = true;
+		goto out;
+	}
+
+	if (src && (strncmp(path, src, strlen(src)) == 0)) {
+		ret = true;
+		goto out;
+	}
+
+out:
+	free(lib);
+	free(src);
+	return ret;
+}

src/manifest.c

@@ -1055,6 +1055,14 @@ int prune_manifest(struct manifest *manifest)
 			// LOG(file, "Skipping deleted boot file in manifest write", "component %s", manifest->component);
 			manifest->files = g_list_delete_link(manifest->files, list);
 			manifest->count--;
+		} else if (config_ban_debuginfo() && file_is_debuginfo(file->filename)) {
+			/* The configuration option to ban debuginfo from the manifests was
+			 * set in server.ini via the [Debuginfo][banned] option. Although
+			 * debuginfo additions are banned via analyze_fs, prune it here
+			 * to insure mistakenly included debuginfo from old versions is
+			 * removed from the manifests. */
+			manifest->files = g_list_delete_link(manifest->files, list);
+			manifest->count--;
 		}
 		list = next;
 	}

src/pack.c

@@ -58,11 +58,11 @@ static void empty_pack_stage(int full, int from_version, int to_version, char *m
 		// (re)create module/version/{delta,staged}
 		string_or_die(&path, "%s/%s/%i_to_%i/delta", packstage_dir, module,
 			      from_version, to_version);
-		g_mkdir_with_parents(path, S_IRWXU | S_IRWXG);
+		g_mkdir_with_parents(path, S_IRWXU);
 		free(path);
 		string_or_die(&path, "%s/%s/%i_to_%i/staged", packstage_dir, module,
 			      from_version, to_version);
-		g_mkdir_with_parents(path, S_IRWXU | S_IRWXG);
+		g_mkdir_with_parents(path, S_IRWXU);
 		free(path);
 	}
 }
@@ -79,7 +79,7 @@ static void explode_pack_stage(int from_version, int to_version, char *module)
 
 	string_or_die(&path, "%s/%s/%i_to_%i/staged", packstage_dir, module,
 		      from_version, to_version);
-	g_mkdir_with_parents(path, S_IRWXU | S_IRWXG);
+	g_mkdir_with_parents(path, S_IRWXU);
 	dir = opendir(path);
 	if (!dir) {
 		fprintf(stderr, "There are problems accessing %s, exiting\n", path);

test/functional/file-name-debuginfo/test.bats

@@ -0,0 +1,56 @@
+#!/usr/bin/env bats
+
+# common functions
+load "../swupdlib"
+
+setup() {
+  clean_test_dir
+  init_test_dir
+
+  init_server_ini
+  # unban debuginfo
+  sed -i "s|banned=true|banned=false|" $DIR/server.ini
+  set_latest_ver 0
+  init_groups_ini os-core
+  init_groups_ini test-bundle
+
+  set_os_release 10 os-core
+  track_bundle 10 os-core
+  set_os_release 10 test-bundle
+  track_bundle 10 test-bundle
+
+  gen_file_plain 10 test-bundle "/usr/lib/debug/foo"
+  gen_file_plain 10 test-bundle "/usr/src/debug/bar"
+  gen_file_plain 10 test-bundle "/usr/bin/foobar"
+
+  track_bundle 20 os-core
+  set_os_release 20 test-bundle
+  track_bundle 20 test-bundle
+
+  gen_file_plain 20 test-bundle "/usr/foo"
+}
+
+@test "debuginfo files pruned" {
+  run sudo sh -c "$CREATE_UPDATE --osversion 10 --statedir $DIR --format 3"
+  echo "$output"
+  # This should not be pruned
+  [[ 1 -eq $(grep '/usr/bin/foobar$' $DIR/www/10/Manifest.test-bundle | wc -l) ]]
+  [[ 1 -eq $(grep '/usr/lib/debug$' $DIR/www/10/Manifest.test-bundle | wc -l) ]]
+  [[ 1 -eq $(grep '/usr/src/debug$' $DIR/www/10/Manifest.test-bundle | wc -l) ]]
+  # These should not be pruned right now, but should be pruned with the next
+  # update after being banned in server.ini again
+  [[ 1 -eq $(grep '/usr/src/debug/bar$' $DIR/www/10/Manifest.test-bundle | wc -l) ]]
+  [[ 1 -eq $(grep '/usr/lib/debug/foo$' $DIR/www/10/Manifest.test-bundle | wc -l) ]]
+
+  # grab the server.ini that bans debuginfo again
+  init_server_ini
+  set_os_release 20 os-core
+  run sudo sh -c "$CREATE_UPDATE --osversion 20 --statedir $DIR --format 3"
+  # This should not be pruned
+  [[ 1 -eq $(grep '/usr/foo$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+  # These should be pruned
+  [[ 0 -eq $(grep '/usr/src/debug/bar$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+  [[ 0 -eq $(grep '/usr/lib/debug/foo$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+}
+
+# vi: ft=sh ts=8 sw=2 sts=2 et tw=80

test/functional/full-run/test.bats

@@ -40,6 +40,12 @@ setup() {
   [[ 1 -eq $(grep '/usr/share/clear/bundles$' $DIR/www/10/Manifest.os-core | wc -l) ]]
   [[ 4 -eq $(tar -tf $DIR/www/10/pack-test-bundle-from-0.tar | wc -l) ]]
   [[ 5 -eq $(tar -tf $DIR/www/10/pack-os-core-from-0.tar | wc -l) ]]
+
+  # extract test-bundle pack to make sure staged and delta are created with the
+  # correct permissions
+  sudo tar -xf $DIR/www/10/pack-test-bundle-from-0.tar --directory $DIR/www/10/
+  [[ $(stat -c %a $DIR/www/10/staged) -eq 700 ]]
+  [[ $(stat -c %a $DIR/www/10/delta) -eq 700 ]]
 }
 
 # vi: ft=sh ts=8 sw=2 sts=2 et tw=80