Merge pull request #262 from freifunk-gluon/autoupdater-fixes

Autoupdater fixes (use-after-free, better signal handling)

admin/autoupdater/src/autoupdater.c

@@ -39,6 +39,7 @@
 #include <fcntl.h>
 #include <getopt.h>
 #include <math.h>
+#include <signal.h>
 #include <stdbool.h>
 #include <stdlib.h>
 #include <stdio.h>
@@ -281,6 +282,7 @@ static bool autoupdate(const char *mirror, struct settings *s, int lock_fd) {
 	struct recv_manifest_ctx manifest_ctx = { .s = s };
 	manifest_ctx.ptr = manifest_ctx.buf;
 	struct manifest *m = &manifest_ctx.m;
+	int interrupted = 0;
 
 	/**** Get and check manifest *****************************************/
 	/* Construct manifest URL */
@@ -295,6 +297,7 @@ static bool autoupdate(const char *mirror, struct settings *s, int lock_fd) {
 	int err_code = get_url(manifest_url, recv_manifest_cb, &manifest_ctx, -1, s->old_version);
 	if (err_code != 0) {
 		fprintf(stderr, "autoupdater: warning: error downloading manifest: %s\n", uclient_get_errmsg(err_code));
+		interrupted = uclient_interrupted_signal(err_code);
 		goto out;
 	}
 
@@ -362,6 +365,7 @@ static bool autoupdate(const char *mirror, struct settings *s, int lock_fd) {
 		puts("");
 		if (err_code != 0) {
 			fprintf(stderr, "autoupdater: warning: error downloading image: %s\n", uclient_get_errmsg(err_code));
+			interrupted = uclient_interrupted_signal(err_code);
 			close(image_ctx.fd);
 			goto fail_after_download;
 		}
@@ -431,6 +435,14 @@ fail_after_download:
 
 out:
 	clear_manifest(m);
+
+	/* If we were interrupted by a signal, restore original signal handlers
+	 * and re-raise signal to terminate process */
+	if (interrupted) {
+		uloop_done();
+		raise(interrupted);
+	}
+
 	return ret;
 }
 

admin/autoupdater/src/uclient.c

@@ -43,14 +43,21 @@ enum uclient_own_error_code {
 	UCLIENT_ERROR_CONNECTION_RESET_PREMATURELY,
 	UCLIENT_ERROR_SIZE_MISMATCH,
 	UCLIENT_ERROR_STATUS_CODE = 1024,
+	UCLIENT_ERROR_INTERRUPTED = 2048,
 };
 
 
 const char *uclient_get_errmsg(int code) {
-	static char http_code_errmsg[16];
+	static char http_code_errmsg[34];
 	if (code & UCLIENT_ERROR_STATUS_CODE) {
-		snprintf(http_code_errmsg, 16, "HTTP error %d",
-			code & (~UCLIENT_ERROR_STATUS_CODE));
+		snprintf(http_code_errmsg, sizeof(http_code_errmsg),
+			"HTTP error %d", code & (~UCLIENT_ERROR_STATUS_CODE));
+		return http_code_errmsg;
+	}
+	if (code & UCLIENT_ERROR_INTERRUPTED) {
+		snprintf(http_code_errmsg, sizeof(http_code_errmsg),
+			"Interrupted by signal %d",
+			code & (~UCLIENT_ERROR_INTERRUPTED));
 		return http_code_errmsg;
 	}
 	switch(code) {
@@ -71,10 +78,16 @@ const char *uclient_get_errmsg(int code) {
 	}
 }
 
+int uclient_interrupted_signal(int code) {
+	if (code & UCLIENT_ERROR_INTERRUPTED)
+		return code & (~UCLIENT_ERROR_INTERRUPTED);
+
+	return 0;
+}
+
 
 static void request_done(struct uclient *cl, int err_code) {
 	uclient_data(cl)->err_code = err_code;
-	uclient_disconnect(cl);
 	uloop_end();
 }
 
@@ -159,6 +172,7 @@ int get_url(const char *url, void (*read_cb)(struct uclient *cl), void *cb_data,
 		.data_eof = eof_cb,
 		.error = request_done,
 	};
+	int ret = UCLIENT_ERROR_CONNECT;
 
 	struct uclient *cl = uclient_new(url, NULL, &cb);
 	if (!cl)
@@ -181,17 +195,26 @@ int get_url(const char *url, void (*read_cb)(struct uclient *cl), void *cb_data,
 	}
 	if (uclient_request(cl))
 		goto err;
-	uloop_run();
-	uclient_free(cl);
 
-	if (!d.err_code && d.length >= 0 && d.downloaded != d.length)
-		return UCLIENT_ERROR_SIZE_MISMATCH;
+	ret = uloop_run();
+	if (ret) {
+		/* uloop_run() returns a signal number when interrupted */
+		ret |= UCLIENT_ERROR_INTERRUPTED;
+		goto err;
+	}
 
-	return d.err_code;
+	if (!d.err_code && d.length >= 0 && d.downloaded != d.length) {
+		ret = UCLIENT_ERROR_SIZE_MISMATCH;
+		goto err;
+	}
+
+	ret = d.err_code;
 
 err:
-	if (cl)
+	if (cl) {
+		uclient_disconnect(cl);
 		uclient_free(cl);
+	}
 
-	return UCLIENT_ERROR_CONNECT;
+	return ret;
 }

admin/autoupdater/src/uclient.h

@@ -52,3 +52,4 @@ ssize_t uclient_read_account(struct uclient *cl, char *buf, int len);
 
 int get_url(const char *url, void (*read_cb)(struct uclient *cl), void *cb_data, ssize_t len, const char *firmware_version);
 const char *uclient_get_errmsg(int code);
+int uclient_interrupted_signal(int code);