[LEDE-DEV] [RFC 03/13] procd: upgraded: link dynamically, chroot during exec

Matthias Schiffer mschiffer at universe-factory.net
Sun Apr 23 17:06:52 PDT 2017


The chroot ensures we don't reference anything on the rootfs and is
reverted after the upgraded exec. While we're at it, also improve error
handling and logging at bit.

This change also required changes to sysupgrade, as the dynamically linked
version is expected at a different location, and libraries need to be made
available.

Signed-off-by: Matthias Schiffer <mschiffer at universe-factory.net>

Note: obviously, this patch should go into the procd repo when it is
actually applied; it is provided like this for now to allow easier testing
of the whole series.
---
 ...raded-link-dynamically-chroot-during-exec.patch | 231 +++++++++++++++++++++
 1 file changed, 231 insertions(+)
 create mode 100644 package/system/procd/patches/0002-upgraded-link-dynamically-chroot-during-exec.patch

diff --git a/package/system/procd/patches/0002-upgraded-link-dynamically-chroot-during-exec.patch b/package/system/procd/patches/0002-upgraded-link-dynamically-chroot-during-exec.patch
new file mode 100644
index 0000000000..0840540334
--- /dev/null
+++ b/package/system/procd/patches/0002-upgraded-link-dynamically-chroot-during-exec.patch
@@ -0,0 +1,231 @@
+From d212b2f6a5e2e2779f3fc4bfb5fe217fdeed03a8 Mon Sep 17 00:00:00 2001
+Message-Id: <d212b2f6a5e2e2779f3fc4bfb5fe217fdeed03a8.1492975837.git.mschiffer at universe-factory.net>
+In-Reply-To: <3462ccc0c91964ec92c1c61cde803a6504b2fb24.1492975837.git.mschiffer at universe-factory.net>
+References: <3462ccc0c91964ec92c1c61cde803a6504b2fb24.1492975837.git.mschiffer at universe-factory.net>
+From: Matthias Schiffer <mschiffer at universe-factory.net>
+Date: Sun, 23 Apr 2017 02:28:13 +0200
+Subject: [PATCH 2/4] upgraded: link dynamically, chroot during exec
+
+The chroot ensures we don't reference anything on the rootfs and is
+reverted after the upgraded exec. While we're at it, also improve error
+handling a bit.
+
+This change also required changes to sysupgrade, as the dynamically linked
+version is expected at a different location, and libraries need to be made
+available.
+
+Signed-off-by: Matthias Schiffer <mschiffer at universe-factory.net>
+---
+ system.c                | 25 +++++++++++++++++++------
+ upgraded/CMakeLists.txt | 10 +---------
+ upgraded/upgraded.c     | 26 +++++++++++++++++++++-----
+ watchdog.c              |  9 +++++++--
+ watchdog.h              |  4 ++--
+ 5 files changed, 50 insertions(+), 24 deletions(-)
+
+diff --git a/system.c b/system.c
+index bb2abe5..193c9b0 100644
+--- a/system.c
++++ b/system.c
+@@ -345,27 +345,40 @@ static int proc_signal(struct ubus_context *ctx, struct ubus_object *obj,
+ 
+ enum {
+ 	SYSUPGRADE_PATH,
++	SYSUPGRADE_PREFIX,
+ 	__SYSUPGRADE_MAX
+ };
+ 
+ static const struct blobmsg_policy sysupgrade_policy[__SYSUPGRADE_MAX] = {
+ 	[SYSUPGRADE_PATH] = { .name = "path", .type = BLOBMSG_TYPE_STRING },
++	[SYSUPGRADE_PREFIX] = { .name = "prefix", .type = BLOBMSG_TYPE_STRING },
+ };
+ 
+ static void
+-procd_spawn_upgraded(char *path)
++procd_exec_upgraded(const char *prefix, char *path)
+ {
+ 	char *wdt_fd = watchdog_fd();
+-	char *argv[] = { "/tmp/upgraded", NULL, NULL};
++	char *argv[] = { "/sbin/upgraded", NULL, NULL};
++
++	if (chroot(prefix)) {
++		fprintf(stderr, "Failed to chroot for upgraded exec.\n");
++		return;
++	}
+ 
+ 	argv[1] = path;
+ 
+ 	DEBUG(2, "Exec to upgraded now\n");
+ 	if (wdt_fd) {
+-		watchdog_no_cloexec();
++		watchdog_set_cloexec(false);
+ 		setenv("WDTFD", wdt_fd, 1);
+ 	}
+ 	execvp(argv[0], argv);
++
++	/* Cleanup on failure */
++	fprintf(stderr, "Failed to exec upgraded.\n");
++	unsetenv("WDTFD");
++	watchdog_set_cloexec(true);
++	chroot(".");
+ }
+ 
+ static int sysupgrade(struct ubus_context *ctx, struct ubus_object *obj,
+@@ -378,11 +391,11 @@ static int sysupgrade(struct ubus_context *ctx, struct ubus_object *obj,
+ 		return UBUS_STATUS_INVALID_ARGUMENT;
+ 
+ 	blobmsg_parse(sysupgrade_policy, __SYSUPGRADE_MAX, tb, blob_data(msg), blob_len(msg));
+-	if (!tb[SYSUPGRADE_PATH])
++	if (!tb[SYSUPGRADE_PATH] || !tb[SYSUPGRADE_PREFIX])
+ 		return UBUS_STATUS_INVALID_ARGUMENT;
+ 
+-	procd_spawn_upgraded(blobmsg_get_string(tb[SYSUPGRADE_PATH]));
+-	fprintf(stderr, "Yikees, something went wrong. no /sbin/upgraded ?\n");
++	procd_exec_upgraded(blobmsg_get_string(tb[SYSUPGRADE_PREFIX]),
++			    blobmsg_get_string(tb[SYSUPGRADE_PATH]));
+ 	return 0;
+ }
+ 
+diff --git a/upgraded/CMakeLists.txt b/upgraded/CMakeLists.txt
+index 093dba2..00d8ce5 100644
+--- a/upgraded/CMakeLists.txt
++++ b/upgraded/CMakeLists.txt
+@@ -2,16 +2,8 @@ cmake_minimum_required(VERSION 2.6)
+ 
+ PROJECT(upgraded C)
+ ADD_DEFINITIONS(-Os -ggdb -Wall -Werror --std=gnu99 -Wmissing-declarations)
+-set(CMAKE_EXE_LINKER_FLAGS "-static -fPIC")
+-set(CMAKE_FIND_LIBRARY_SUFFIXES .a)
+-set(CMAKE_EXE_LINK_DYNAMIC_C_FLAGS)
+-set(CMAKE_EXE_LINK_DYNAMIC_CXX_FLAGS)
+-set(CMAKE_SHARED_LIBRARY_C_FLAGS)
+-set(CMAKE_SHARED_LIBRARY_CXX_FLAGS)
+-set(CMAKE_SHARED_LIBRARY_LINK_C_FLAGS)
+-set(CMAKE_SHARED_LIBRARY_LINK_CXX_FLAGS)
+ ADD_EXECUTABLE(upgraded upgraded.c ../watchdog.c)
+-TARGET_LINK_LIBRARIES(upgraded ubox rt -lc -lgcc_pic)
++TARGET_LINK_LIBRARIES(upgraded ubox)
+ INSTALL(TARGETS upgraded
+ 	RUNTIME DESTINATION sbin
+ )
+diff --git a/upgraded/upgraded.c b/upgraded/upgraded.c
+index d7433e7..aa0b4ff 100644
+--- a/upgraded/upgraded.c
++++ b/upgraded/upgraded.c
+@@ -14,6 +14,7 @@
+ 
+ #include <sys/reboot.h>
+ 
++#include <fcntl.h>
+ #include <stdio.h>
+ #include <stdlib.h>
+ #include <string.h>
+@@ -24,6 +25,10 @@
+ 
+ #include "../watchdog.h"
+ 
++#ifndef O_PATH
++#define O_PATH		010000000
++#endif
++
+ static struct uloop_process upgrade_proc;
+ unsigned int debug = 2;
+ 
+@@ -34,7 +39,7 @@ static void upgrade_proc_cb(struct uloop_process *proc, int ret)
+ 	uloop_end();
+ }
+ 
+-static void sysupgarde(char *folder)
++static void sysupgrade(char *folder)
+ {
+ 	char *args[] = { "/sbin/sysupgrade", "nand", NULL, NULL };
+ 
+@@ -47,7 +52,7 @@ static void sysupgarde(char *folder)
+ 		exit(-1);
+ 	}
+ 	if (upgrade_proc.pid <= 0) {
+-		fprintf(stderr, "Failed to start sysupgarde\n");
++		fprintf(stderr, "Failed to start sysupgrade\n");
+ 		uloop_end();
+ 	}
+ }
+@@ -60,10 +65,21 @@ int main(int argc, char **argv)
+ 		fprintf(stderr, "this tool needs to run as pid 1\n");
+ 		return -1;
+ 	}
+-	if (chdir("/tmp") == -1) {
+-		fprintf(stderr, "failed to chdir to /tmp: %s\n", strerror(errno));
++
++	int fd = open("/", O_DIRECTORY|O_PATH);
++	if (fd < 0) {
++		fprintf(stderr, "unable to open prefix directory: %s\n", strerror(errno));
+ 		return -1;
+ 	}
++
++	chroot(".");
++
++	if (fchdir(fd) == -1) {
++		fprintf(stderr, "failed to chdir to prefix directory: %s\n", strerror(errno));
++		return -1;
++	}
++	close(fd);
++
+ 	if (argc != 2) {
+ 		fprintf(stderr, "sysupgrade stage 2 failed, no folder specified\n");
+ 		return -1;
+@@ -71,7 +87,7 @@ int main(int argc, char **argv)
+ 
+ 	uloop_init();
+ 	watchdog_init(0);
+-	sysupgarde(argv[1]);
++	sysupgrade(argv[1]);
+ 	uloop_run();
+ 
+ 	reboot(RB_AUTOBOOT);
+diff --git a/watchdog.c b/watchdog.c
+index 592ae7e..780b321 100644
+--- a/watchdog.c
++++ b/watchdog.c
+@@ -126,10 +126,15 @@ void watchdog_init(int preinit)
+ }
+ 
+ 
+-void watchdog_no_cloexec(void)
++void watchdog_set_cloexec(bool val)
+ {
+ 	if (wdt_fd < 0)
+ 		return;
+ 
+-	fcntl(wdt_fd, F_SETFD, fcntl(wdt_fd, F_GETFD) & ~FD_CLOEXEC);
++	int flags = fcntl(wdt_fd, F_GETFD);
++	if (val)
++		flags |= FD_CLOEXEC;
++	else
++		flags &= ~FD_CLOEXEC;
++	fcntl(wdt_fd, F_SETFD,  flags);
+ }
+diff --git a/watchdog.h b/watchdog.h
+index e5c696a..641e7e1 100644
+--- a/watchdog.h
++++ b/watchdog.h
+@@ -22,7 +22,7 @@ int watchdog_timeout(int timeout);
+ int watchdog_frequency(int frequency);
+ void watchdog_set_stopped(bool val);
+ bool watchdog_get_stopped(void);
+-void watchdog_no_cloexec(void);
++void watchdog_set_cloexec(bool val);
+ void watchdog_ping(void);
+ #else
+ static inline void watchdog_init(int preinit)
+@@ -53,7 +53,7 @@ static inline bool watchdog_get_stopped(void)
+ 	return true;
+ }
+ 
+-static inline void watchdog_no_cloexec(void)
++static inline void watchdog_set_cloexec(bool val)
+ {
+ }
+ 
+-- 
+2.12.2
+
-- 
2.12.2




More information about the Lede-dev mailing list