diff mbox series

Drop separate installation path for local file

Message ID 20201223134043.787666-1-sbabic@denx.de
State Accepted
Headers show
Series Drop separate installation path for local file | expand

Commit Message

Stefano Babic Dec. 23, 2020, 1:40 p.m. UTC
There are currently two separate paths for installing, via network
thread (used by OTA / IPC library) and from a local file. This increases
efforts during regression tests because both paths must be tested. In
case of local path, no network thread is created and file is scanned via
seek(), adding an implicit "installed-directly" to all artifacts.
Nevertheless, it happened to find dicrepancies between OTA and local
file update.
This patch is a big cleanup and drops the path from local file. The
network thread is always generated, andcode from swupdate-client is
integrated into SWUpdate to use the generic update path.
Specific functions duplicated for local path are dropped, and it is not
requested to forward in all steps if an updated is coming from local
file or from OTA.

These changes are thought to be compatible with the past, and updating
via -i (local file) is still supported. Indeed, the following different
behaviors are introduced:

- SWU check via -c is the same as dry-run mode. Not just sw-description
is parsed, but the whole update is checked without installing. This
extends the check's functionality and verifies the whole SWU.
- before these changes, -i results in an implicit "installed-drirectly"
for all artifacts. This is not maintained, and users should add the
attribute to own sw-description to avoid temporary copies on tmpdir.

Signed-off-by: Stefano Babic <sbabic@denx.de>
---
 core/Makefile            |   1 +
 core/install_from_file.c | 113 ++++++++++++++++
 core/installer.c         | 109 +++++++---------
 core/stream_interface.c  |   2 +-
 core/swupdate.c          | 275 +++++----------------------------------
 include/installer.h      |   4 +-
 6 files changed, 200 insertions(+), 304 deletions(-)
 create mode 100644 core/install_from_file.c
diff mbox series

Patch

diff --git a/core/Makefile b/core/Makefile
index ad94120..11bc183 100644
--- a/core/Makefile
+++ b/core/Makefile
@@ -12,6 +12,7 @@  obj-y += swupdate.o \
 	 cpio_utils.o \
 	 notifier.o \
 	 handler.o \
+	 install_from_file.o \
 	 util.o \
 	 parser.o \
 	 pctl.o \
diff --git a/core/install_from_file.c b/core/install_from_file.c
new file mode 100644
index 0000000..a595c30
--- /dev/null
+++ b/core/install_from_file.c
@@ -0,0 +1,113 @@ 
+/*
+ * (C) Copyright 2020
+ * Stefano Babic, DENX Software Engineering, sbabic@denx.de.
+ *
+ * SPDX-License-Identifier:     GPL-2.0-or-later
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <errno.h>
+#include <sys/ioctl.h>
+#include <fcntl.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <pthread.h>
+#include "network_ipc.h"
+#include "util.h"
+#include "installer.h"
+
+static pthread_mutex_t mymutex;
+
+static char buf[16 * 1024];
+static int fd = STDIN_FILENO;
+static int end_status = EXIT_SUCCESS;
+/*
+ * this is the callback to get a new chunk of the
+ * image.
+ * It is called by a thread generated by the library and
+ * can block.
+ */
+static int readimage(char **p, int *size) {
+	int ret;
+
+	ret = read(fd, buf, sizeof(buf));
+
+	*p = buf;
+
+	*size = ret;
+
+	return ret;
+}
+
+/*
+ * this is called at the end reporting the status
+ * of the upgrade and running any post-update actions
+ * if successful
+ */
+static int endupdate(RECOVERY_STATUS status)
+{
+	end_status = (status == SUCCESS) ? EXIT_SUCCESS : EXIT_FAILURE;
+
+	INFO("Swupdate %s\n",
+		status == FAILURE ? "*failed* !" :
+			"was successful !");
+
+	pthread_mutex_unlock(&mymutex);
+
+	return 0;
+}
+
+int install_from_file(const char *filename, bool check)
+{
+	int rc;
+	int timeout_cnt = 3;
+
+	if (filename && (fd = open(filename, O_RDONLY)) < 0) {
+		fprintf(stderr, "Unable to open %s\n", filename);
+		return EXIT_FAILURE;
+	}
+
+	pthread_mutex_init(&mymutex, NULL);
+
+	/* synchronize with a mutex */
+	pthread_mutex_lock(&mymutex);
+
+
+	/* May be set non-zero by end() function on failure */
+	end_status = EXIT_SUCCESS;
+
+	struct swupdate_request req;
+	swupdate_prepare_req(&req);
+	if (check)
+		req.dry_run = RUN_DRYRUN;
+
+	while (timeout_cnt > 0) {
+		rc = swupdate_async_start(readimage, NULL,
+					  endupdate, &req, sizeof(req));
+		if (rc >= 0)
+			break;
+		timeout_cnt--;
+		sleep(1);
+	}
+
+	/* return if we've hit an error scenario */
+	if (rc < 0) {
+		ERROR ("swupdate_async_start returns %d\n", rc);
+		pthread_mutex_unlock(&mymutex);
+		close(fd);
+		return EXIT_FAILURE;
+	}
+
+	/* Now block */
+	pthread_mutex_lock(&mymutex);
+
+	/* End called, unlock and exit */
+	pthread_mutex_unlock(&mymutex);
+
+	if (filename)
+		close(fd);
+
+	return end_status;
+}
diff --git a/core/installer.c b/core/installer.c
index 981a21d..d830dae 100644
--- a/core/installer.c
+++ b/core/installer.c
@@ -92,7 +92,7 @@  int check_if_required(struct imglist *list, struct filehdr *pfdh,
  * Extract all scripts from a list from the image
  * and save them on the filesystem to be executed later
  */
-static int extract_scripts(int fd, struct imglist *head, int fromfile)
+static int extract_scripts(struct imglist *head)
 {
 	struct img_type *script;
 	int fdout;
@@ -100,6 +100,11 @@  static int extract_scripts(int fd, struct imglist *head, int fromfile)
 	const char* tmpdir_scripts = get_tmpdirscripts();
 
 	LIST_FOREACH(script, head, next) {
+		int fdin;
+		char *tmpfile;
+		unsigned long offset = 0;
+		uint32_t checksum;
+
 		if (!script->fname[0] && (script->provided == 0)) {
 			TRACE("No script provided for script of type %s",
 				script->type);
@@ -118,40 +123,31 @@  static int extract_scripts(int fd, struct imglist *head, int fromfile)
 		if (fdout < 0)
 			return fdout;
 
-		if (fromfile)
-			ret = extract_next_file(fd, fdout, script->offset, 0,
-						script->is_encrypted, script->ivt_ascii, script->sha256);
-		else {
-			int fdin;
-			char *tmpfile;
-			unsigned long offset = 0;
-			uint32_t checksum;
 
-			if (asprintf(&tmpfile, "%s%s", get_tmpdir(), script->fname) ==
-				ENOMEM_ASPRINTF) {
-				ERROR("Path too long: %s%s", get_tmpdir(), script->fname);
-				close(fdout);
-				return -ENOMEM;
-			}
-
-			fdin = open(tmpfile, O_RDONLY);
-			free(tmpfile);
-			if (fdin < 0) {
-				ERROR("Extracted script not found in %s: %s %d",
-					get_tmpdir(), script->extract_file, errno);
-				close(fdout);
-				return -ENOENT;
-			}
+		if (asprintf(&tmpfile, "%s%s", get_tmpdir(), script->fname) ==
+			ENOMEM_ASPRINTF) {
+			ERROR("Path too long: %s%s", get_tmpdir(), script->fname);
+			close(fdout);
+			return -ENOMEM;
+		}
 
-			ret = copyfile(fdin, &fdout, script->size, &offset, 0, 0,
-					script->compressed,
-					&checksum,
-					script->sha256,
-					script->is_encrypted,
-					script->ivt_ascii,
-					NULL);
-			close(fdin);
+		fdin = open(tmpfile, O_RDONLY);
+		free(tmpfile);
+		if (fdin < 0) {
+			ERROR("Extracted script not found in %s: %s %d",
+				get_tmpdir(), script->extract_file, errno);
+			close(fdout);
+			return -ENOENT;
 		}
+
+		ret = copyfile(fdin, &fdout, script->size, &offset, 0, 0,
+				script->compressed,
+				&checksum,
+				script->sha256,
+				script->is_encrypted,
+				script->ivt_ascii,
+				NULL);
+		close(fdin);
 		close(fdout);
 
 		if (ret < 0)
@@ -252,12 +248,11 @@  int install_single_image(struct img_type *img, int dry_run)
  * extract : boolean, true to enable extraction
  */
 
-int install_images(struct swupdate_cfg *sw, int fdsw, int fromfile)
+int install_images(struct swupdate_cfg *sw)
 {
 	int ret;
 	struct img_type *img, *tmp;
 	char *filename;
-	struct filehdr fdh;
 	struct stat buf;
 	const char* TMPDIR = get_tmpdir();
 	int dry_run = sw->globals.dry_run;
@@ -265,8 +260,8 @@  int install_images(struct swupdate_cfg *sw, int fdsw, int fromfile)
 
 	/* Extract all scripts, preinstall scripts must be run now */
 	const char* tmpdir_scripts = get_tmpdirscripts();
-	ret = extract_scripts(fdsw, &sw->scripts, fromfile);
-	ret |= extract_scripts(fdsw, &sw->bootscripts, fromfile);
+	ret = extract_scripts(&sw->scripts);
+	ret |= extract_scripts(&sw->bootscripts);
 	if (ret) {
 		ERROR("extracting script to %s failed", tmpdir_scripts);
 		return ret;
@@ -292,37 +287,28 @@  int install_images(struct swupdate_cfg *sw, int fdsw, int fromfile)
 		 *  This does not make sense when installed from file,
 		 *  because images are seekd (no streaming)
 		 */
-		if (!fromfile && img->install_directly)
+		if (img->install_directly)
 			continue;
 
-		if (!fromfile) {
-		    if (asprintf(&filename, "%s%s", TMPDIR, img->fname) ==
+		if (asprintf(&filename, "%s%s", TMPDIR, img->fname) ==
 				ENOMEM_ASPRINTF) {
 				ERROR("Path too long: %s%s", TMPDIR, img->fname);
 				return -1;
-			}
-
-			ret = stat(filename, &buf);
-			if (ret) {
-				TRACE("%s not found or wrong", filename);
-				free(filename);
-				return -1;
-			}
-			img->size = buf.st_size;
+		}
 
-			img->fdin = open(filename, O_RDONLY);
+		ret = stat(filename, &buf);
+		if (ret) {
+			TRACE("%s not found or wrong", filename);
 			free(filename);
-			if (img->fdin < 0) {
-				ERROR("Image %s cannot be opened",
-				img->fname);
-				return -1;
-			}
-		} else {
-			if (extract_img_from_cpio(fdsw, img->offset, &fdh) < 0)
-				return -1;
-			img->size = fdh.size;
-			img->checksum = fdh.chksum;
-			img->fdin = fdsw;
+			return -1;
+		}
+		img->size = buf.st_size;
+		img->fdin = open(filename, O_RDONLY);
+		free(filename);
+		if (img->fdin < 0) {
+			ERROR("Image %s cannot be opened",
+			img->fname);
+			return -1;
 		}
 
 		if ((strlen(img->path) > 0) &&
@@ -345,8 +331,7 @@  int install_images(struct swupdate_cfg *sw, int fdsw, int fromfile)
 			ret = install_single_image(img, dry_run);
 		}
 
-		if (!fromfile)
-			close(img->fdin);
+		close(img->fdin);
 
 		if (dropimg)
 			free_image(img);
diff --git a/core/stream_interface.c b/core/stream_interface.c
index 6d32998..c5073fd 100644
--- a/core/stream_interface.c
+++ b/core/stream_interface.c
@@ -587,7 +587,7 @@  void *network_initializer(void *data)
 			}
 
 			notify(RUN, RECOVERY_NO_ERROR, INFOLEVEL, "Installation in progress");
-			ret = install_images(software, 0, 0);
+			ret = install_images(software);
 			if (ret != 0) {
 				if (!software->globals.dry_run && software->bootloader_transaction_marker) {
 					save_state_string((char*)BOOTVAR_TRANSACTION, STATE_FAILED);
diff --git a/core/swupdate.c b/core/swupdate.c
index 9adafa8..318a245 100644
--- a/core/swupdate.c
+++ b/core/swupdate.c
@@ -178,23 +178,6 @@  static void usage(char *programname)
 #endif
 }
 
-static int check_provided(struct imglist *list)
-{
-	int ret = 0;
-	struct img_type *p;
-
-	for (p = list->lh_first; p != NULL;
-		p = p->next.le_next) {
-		if (!p->provided && !(get_handler_mask(p) & NO_DATA_HANDLER)) {
-			ERROR("Requested file not found in image: %s", \
-				p->fname);
-			ret = -1;
-		}
-	}
-
-	return ret;
-}
-
 struct swupdate_cfg *get_swupdate_cfg(void) {
 	return &swcfg;
 }
@@ -229,191 +212,6 @@  static int opt_to_hwrev(char *param, struct hw_type *hw)
 	return 0;
 }
 
-static int searching_for_image(char *name)
-{
-	char *dir, *dirc, *basec;
-	char *fpattern;
-	DIR *path;
-	struct dirent *dp;
-	int fd = -1;
-	int found;
-	char fname[MAX_IMAGE_FNAME];
-	char *buf;
-	char hex[4];
-
-	dirc = strdup(name);
-	basec = strdup(name);
-	dir = dirname(dirc);
-	fpattern = basename(basec);
-	path = opendir(dir);
-
-	TRACE("Searching image: check %s into %s",
-			basec, dirc);
-	if (!path) {
-		free(dirc);
-		free(basec);
-		return -EBADF;
-	}
-
-	dp = readdir(path);
-	do {
-		if (!dp)
-			break;
-		if (!strcmp(dp->d_name, ".") ||
-				!strcmp(dp->d_name, "..") ||
-				!strlen(dp->d_name))
-			continue;
-		found = !fnmatch(fpattern, dp->d_name, FNM_CASEFOLD);
-
-		if (found) {
-			TRACE("File found: %s :", dp->d_name);
-			/* Buffer for hexa output */
-			buf = (char *)malloc(3 * strlen(dp->d_name) + 1);
-			if (buf) {
-				for (size_t i = 0; i < strlen(dp->d_name); i++) {
-					snprintf(hex, sizeof(hex), "%x ", dp->d_name[i]);
-					memcpy(&buf[3 * i], hex, 3);
-				}
-				buf[3 * strlen(dp->d_name)] = '\0';
-				TRACE("\nFile name (hex): %s", buf);
-			}
-			/* Take the first one as image */
-			if (fd < 0) {
-				if (snprintf(fname, sizeof(fname), "%s/%s",
-					     dirc, dp->d_name) >= (int)sizeof(fname)) {
-					ERROR("Path too long: %s/%s", dirc, dp->d_name);
-				}
-				fd = open(fname, O_RDONLY);
-				if (fd > 0)
-					TRACE("\t\t**Used for upgrade");
-			}
-			free(buf);
-		}
-
-	} while ((dp = readdir(path)) !=NULL);
-
-	free(dirc);
-	free(basec);
-	closedir(path);
-
-	return fd;
-}
-
-static int install_from_file(char *fname, int check)
-{
-	int fdsw;
-	off_t pos;
-	int ret;
-	bool encrypted_sw_desc = false;
-
-#ifdef CONFIG_ENCRYPTED_SW_DESCRIPTION
-	encrypted_sw_desc = true;
-#endif
-	if (!strlen(fname)) {
-		ERROR("Image not found...please reboot");
-		exit(EXIT_FAILURE);
-	}
-
-	fdsw = open(fname, O_RDONLY);
-	if (fdsw < 0) {
-		fdsw = searching_for_image(fname);
-		if (fdsw < 0) {
-			ERROR("Image Software cannot be read...exiting !");
-			exit(EXIT_FAILURE);
-		}
-	}
-
-	pos = 0;
-	ret = extract_sw_description(fdsw, SW_DESCRIPTION_FILENAME, &pos, encrypted_sw_desc);
-#ifdef CONFIG_SIGNED_IMAGES
-	ret |= extract_sw_description(fdsw, SW_DESCRIPTION_FILENAME ".sig",
-		&pos, false);
-#endif
-	/*
-	 * Check if files could be extracted
-	 */
-	if (ret) {
-		ERROR("Failed to extract meta information");
-		exit(EXIT_FAILURE);
-	}
-
-	char* swdescfilename = alloca(strlen(get_tmpdir())+strlen(SW_DESCRIPTION_FILENAME)+1);
-	sprintf(swdescfilename, "%s%s", get_tmpdir(), SW_DESCRIPTION_FILENAME);
-	ret = parse(&swcfg, swdescfilename);
-	if (ret) {
-		ERROR("failed to parse " SW_DESCRIPTION_FILENAME "!");
-		exit(EXIT_FAILURE);
-	}
-
-	if (check_hw_compatibility(&swcfg)) {
-		ERROR("SW not compatible with hardware");
-		exit(EXIT_FAILURE);
-	}
-
-	if (cpio_scan(fdsw, &swcfg, pos) < 0) {
-		ERROR("failed to scan for pos '%lld'!", (long long)pos);
-		close(fdsw);
-		exit(EXIT_FAILURE);
-	}
-
-	/*
-	 * Check if all files described in sw-description
-	 * are in the image
-	 */
-	ret = check_provided(&swcfg.images);
-	if (ret) {
-		ERROR("failed to check images!");
-		exit(EXIT_FAILURE);
-	}
-	ret = check_provided(&swcfg.scripts);
-	if (ret) {
-		ERROR("failed to check scripts!");
-		exit(EXIT_FAILURE);
-	}
-
-	if (check) {
-		fprintf(stdout, "successfully checked '%s'\n", fname);
-		exit(EXIT_SUCCESS);
-	}
-
-	ret = preupdatecmd(&swcfg);
-	if (ret) {
-		ERROR("Failed pre-update command!");
-		exit(EXIT_FAILURE);
-	}
-
-#ifdef CONFIG_MTD
-		mtd_cleanup();
-		scan_mtd_devices();
-#endif
-	/*
-	 * Set "recovery_status" as begin of the transaction"
-	 */
-	if (!swcfg.globals.dry_run && swcfg.bootloader_transaction_marker) {
-		save_state_string((char*)BOOTVAR_TRANSACTION, STATE_IN_PROGRESS);
-	}
-
-	swcfg.globals.dry_run = swcfg.globals.default_dry_run;
-	ret = install_images(&swcfg, fdsw, 1);
-
-	swupdate_progress_end(ret == 0 ? SUCCESS : FAILURE);
-
-	close(fdsw);
-
-	if (ret) {
-		fprintf(stdout, "Software update failed\n");
-		return EXIT_FAILURE;
-	}
-
-	if (!swcfg.globals.dry_run && swcfg.bootloader_transaction_marker) {
-		unset_state((char*)BOOTVAR_TRANSACTION);
-	}
-	fprintf(stdout, "Software updated successfully\n");
-	fprintf(stdout, "Please reboot the device to start the new software\n");
-
-	return EXIT_SUCCESS;
-}
-
 static int parse_image_selector(const char *selector, struct swupdate_cfg *sw)
 {
 	char *pos;
@@ -641,7 +439,7 @@  int main(int argc, char **argv)
 	const char *software_select = NULL;
 	int opt_i = 0;
 	int opt_e = 0;
-	int opt_c = 0;
+	bool opt_c = false;
 	char image_url[MAX_URL];
 	char main_options[256];
 	unsigned int public_key_mandatory = 0;
@@ -896,7 +694,7 @@  int main(int argc, char **argv)
 			break;
 #endif
 		case 'c':
-			opt_c = 1;
+			opt_c = true;
 			break;
 		case 'p':
 			strlcpy(swcfg.globals.postupdatecmd, optarg,
@@ -1012,63 +810,60 @@  int main(int argc, char **argv)
 	get_sw_versions(cfgfname, &swcfg);
 
 	/*
-	 *  Do not start daemon if just a check is required
+	 *  Start daemon if just a check is required
 	 *  SWUpdate will exit after the check
 	 */
-	if (!opt_c) {
-		network_daemon = start_thread(network_initializer, &swcfg);
+	network_daemon = start_thread(network_initializer, &swcfg);
 
-		start_thread(progress_bar_thread, NULL);
+	start_thread(progress_bar_thread, NULL);
 
-		/* Start embedded web server */
+	/* Start embedded web server */
 #if defined(CONFIG_MONGOOSE)
-		if (opt_w) {
-			start_subprocess(SOURCE_WEBSERVER, "webserver",
-					 cfgfname, ac, av,
-					 start_mongoose);
-			freeargs(av);
-		}
+	if (opt_w) {
+		start_subprocess(SOURCE_WEBSERVER, "webserver",
+				 cfgfname, ac, av,
+				 start_mongoose);
+		freeargs(av);
+	}
 #endif
 
 #if defined(CONFIG_SURICATTA)
-		if (opt_u) {
-			start_subprocess(SOURCE_SURICATTA, "suricatta",
-					 cfgfname, argcount,
-					 argvalues, start_suricatta);
+	if (opt_u) {
+		start_subprocess(SOURCE_SURICATTA, "suricatta",
+				 cfgfname, argcount,
+				 argvalues, start_suricatta);
 
-			freeargs(argvalues);
-		}
+		freeargs(argvalues);
+	}
 #endif
 
 #ifdef CONFIG_DOWNLOAD
-		if (opt_d) {
-			start_subprocess(SOURCE_DOWNLOADER, "download",
-					 cfgfname, dwlac,
-					 dwlav, start_download);
-			freeargs(dwlav);
-		}
+	if (opt_d) {
+		start_subprocess(SOURCE_DOWNLOADER, "download",
+				 cfgfname, dwlac,
+				 dwlav, start_download);
+		freeargs(dwlav);
+	}
 #endif
 
-		/*
-		 * Start all processes added in the config file
-		 */
-		struct extproc *proc;
+	/*
+	 * Start all processes added in the config file
+	 */
+	struct extproc *proc;
 
-		LIST_FOREACH(proc, &swcfg.extprocs, next) {
-			dwlav = splitargs(proc->exec, &dwlac);
+	LIST_FOREACH(proc, &swcfg.extprocs, next) {
+		dwlav = splitargs(proc->exec, &dwlac);
 
-			dwlav[dwlac] = NULL;
+		dwlav[dwlac] = NULL;
 
-			start_subprocess_from_file(SOURCE_UNKNOWN, proc->name,
-						   cfgfname, dwlac,
-						   dwlav, dwlav[0]);
+		start_subprocess_from_file(SOURCE_UNKNOWN, proc->name,
+					   cfgfname, dwlac,
+					   dwlav, dwlav[0]);
 
-			freeargs(dwlav);
-		}
+		freeargs(dwlav);
 	}
 
 	if (opt_i) {
-
 		result = install_from_file(fname, opt_c);
 		switch (result) {
 		case EXIT_FAILURE:
diff --git a/include/installer.h b/include/installer.h
index 1351603..9fe5f2b 100644
--- a/include/installer.h
+++ b/include/installer.h
@@ -9,6 +9,7 @@ 
 #ifndef _INSTALLER_H
 #define _INSTALLER_H
 
+#include <stdbool.h>
 #include "swupdate.h"
 #include "handler.h"
 #include "cpiohdr.h"
@@ -16,8 +17,9 @@ 
 int check_if_required(struct imglist *list, struct filehdr *pfdh,
 				const char *destdir,
 				struct img_type **pimg);
-int install_images(struct swupdate_cfg *sw, int fdsw, int fromfile);
+int install_images(struct swupdate_cfg *sw);
 int install_single_image(struct img_type *img, int dry_run);
+int install_from_file(const char *filename, bool check);
 int postupdate(struct swupdate_cfg *swcfg, const char *info);
 int preupdatecmd(struct swupdate_cfg *swcfg);
 void cleanup_files(struct swupdate_cfg *software);