Merge pull request #161 from igoropaniuk/memleaks_sec_issues

Address memory leaks/mitigate buffer overflows

Makefile

@@ -2,8 +2,9 @@ QDL := qdl
 RAMDUMP := qdl-ramdump
 VERSION := $(or $(VERSION), $(shell git describe --dirty --always --tags 2>/dev/null), "unknown-version")
 
-CFLAGS += -O2 -Wall -g `pkg-config --cflags libxml-2.0 libusb-1.0`
-LDFLAGS += `pkg-config --libs libxml-2.0 libusb-1.0`
+PKG_CONFIG ?= pkg-config
+CFLAGS += -O2 -Wall -g `$(PKG_CONFIG) --cflags libxml-2.0 libusb-1.0`
+LDFLAGS += `$(PKG_CONFIG) --libs libxml-2.0 libusb-1.0`
 ifeq ($(OS),Windows_NT)
 LDFLAGS += -lws2_32
 endif

firehose.c

@@ -322,8 +322,8 @@ static int firehose_send_configure(struct qdl_device *qdl, size_t payload_size,
 	node = xmlNewChild(root, NULL, (xmlChar *)"configure", NULL);
 	xml_setpropf(node, "MemoryName", memory_names[storage]);
 	xml_setpropf(node, "MaxPayloadSizeToTargetInBytes", "%lu", payload_size);
-	xml_setpropf(node, "verbose", "%d", 0);
-	xml_setpropf(node, "ZLPAwareHost", "%d", 1);
+	xml_setpropf(node, "Verbose", "%d", 0);
+	xml_setpropf(node, "ZlpAwareHost", "%d", 1);
 	xml_setpropf(node, "SkipStorageInit", "%d", skip_storage_init);
 
 	firehose_write(qdl, doc);
@@ -370,7 +370,8 @@ static int firehose_try_configure(struct qdl_device *qdl, bool skip_storage_init
 
 	if (storage != QDL_STORAGE_NAND) {
 		max_sector_size = sector_sizes[ARRAY_SIZE(sector_sizes) - 1];
-		buf = malloc(max_sector_size);
+		buf = alloca(max_sector_size);
+
 		memset(&op, 0, sizeof(op));
 		op.partition = 0;
 		op.start_sector = "1";
@@ -608,7 +609,7 @@ static int firehose_program(struct qdl_device *qdl, struct program *program, int
 
 	t = time(NULL) - t0;
 
-	ret = firehose_read(qdl, 30000, firehose_generic_parser, NULL);
+	ret = firehose_read(qdl, 120000, firehose_generic_parser, NULL);
 	if (ret) {
 		ux_err("flashing of %s failed\n", program->label);
 	} else if (t) {
@@ -938,6 +939,7 @@ static int firehose_reset(struct qdl_device *qdl)
 
 	node = xmlNewChild(root, NULL, (xmlChar *)"power", NULL);
 	xml_setpropf(node, "value", "reset");
+	xml_setpropf(node, "DelayInSeconds", "10"); // Add a delay to prevent reboot fail
 
 	ret = firehose_write(qdl, doc);
 	xmlFreeDoc(doc);
@@ -955,7 +957,7 @@ static int firehose_reset(struct qdl_device *qdl)
 }
 
 static int firehose_detect_and_configure(struct qdl_device *qdl,
-					 bool skip_storage_init __unused,
+					 bool skip_storage_init,
 					 enum qdl_storage_type storage,
 					 unsigned int timeout_s)
 {
@@ -966,7 +968,7 @@ static int firehose_detect_and_configure(struct qdl_device *qdl,
 	gettimeofday(&now, NULL);
 	timeradd(&now, &timeout, &timeout);
 	for (;;) {
-		ret = firehose_try_configure(qdl, false, storage);
+		ret = firehose_try_configure(qdl, skip_storage_init, storage);
 
 		if (ret == FIREHOSE_ACK) {
 			break;
@@ -1015,7 +1017,7 @@ int firehose_run(struct qdl_device *qdl)
 
 	ux_info("waiting for programmer...\n");
 
-	ret = firehose_detect_and_configure(qdl, true, qdl->storage_type, 5);
+	ret = firehose_detect_and_configure(qdl, false, qdl->storage_type, 5);
 	if (ret)
 		return ret;
 

gpt.c

@@ -11,6 +11,7 @@
 #include <assert.h>
 #include <dirent.h>
 #include <fcntl.h>
+#include <inttypes.h>
 #include <stdbool.h>
 #include <stdint.h>
 #include <unistd.h>
@@ -140,8 +141,8 @@ static int gpt_load_table_from_partition(struct qdl_device *qdl, unsigned int ph
 	uint8_t buf[4096];
 	struct read_op op;
 	unsigned int offset;
-	unsigned int lba;
-	char lba_buf[10];
+	uint64_t lba;
+	char lba_buf[21];
 	uint16_t name_utf16le[36];
 	char name[36 * 4];
 	int ret;
@@ -178,7 +179,7 @@ static int gpt_load_table_from_partition(struct qdl_device *qdl, unsigned int ph
 
 		if (offset == 0) {
 			lba = gpt.part_entry_lba + i * gpt.part_entry_size / qdl->sector_size;
-			sprintf(lba_buf, "%u", lba);
+			snprintf(lba_buf, sizeof(lba_buf), "%" PRIu64, lba);
 			op.start_sector = lba_buf;
 
 			memset(buf, 0, sizeof(buf));

ks.c

@@ -134,7 +134,7 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
-	ret = sahara_run(&qdl, mappings, false, NULL, NULL);
+	ret = sahara_run(&qdl, mappings, NULL, NULL);
 	if (ret < 0)
 		return 1;
 

qdl.c

@@ -149,19 +149,20 @@ static uint32_t parse_ascii_hex32(const char *s)
 
 /**
  * decode_programmer_archive() - Attempt to decode a programmer CPIO archive
- * @images: List of Sahara images, with @images[0] populated
+ * @blob: Loaded image to be decoded as archive
+ * @images: List of Sahara images to populate
  *
- * The single blob provided in @images[0] might be a CPIO archive containing
- * Sahara images, in files with names in the format "<id>:<filename>". Load
- * each such Sahara image into the relevant spot in the @images array.
+ * The blob might be a CPIO archive containing Sahara images, in files with
+ * names in the format "<id>:<filename>". Load each such Sahara image into the
+ * relevant spot in the @images array.
  *
- * The original blob (in @images[0]) is freed once it has been consumed.
+ * The blob is always consumed (freed) on both success and error paths.
+ * On error, any partially-populated @images entries are also freed.
  *
  * Returns: 0 if no archive was found, 1 if archive was decoded, -1 on error
  */
-static int decode_programmer_archive(struct sahara_image *images)
+static int decode_programmer_archive(struct sahara_image *blob, struct sahara_image *images)
 {
-	struct sahara_image *blob = &images[0];
 	struct cpio_newc_header *hdr;
 	size_t filesize;
 	size_t namesize;
@@ -178,13 +179,13 @@ static int decode_programmer_archive(struct sahara_image *images)
 	for (;;) {
 		if (ptr + sizeof(*hdr) > end) {
 			ux_err("programmer archive is truncated\n");
-			return -1;
+			goto err;
 		}
 		hdr = ptr;
 
 		if (memcmp(hdr->c_magic, "070701", 6)) {
 			ux_err("expected cpio header in programmer archive\n");
-			return -1;
+			goto err;
 		}
 
 		filesize = parse_ascii_hex32(hdr->c_filesize);
@@ -193,12 +194,12 @@ static int decode_programmer_archive(struct sahara_image *images)
 		ptr += sizeof(*hdr);
 		if (ptr + namesize > end || ptr + filesize + namesize > end) {
 			ux_err("programmer archive is truncated\n");
-			return -1;
+			goto err;
 		}
 
 		if (namesize > sizeof(name)) {
 			ux_err("unexpected filename length in progammer archive\n");
-			return -1;
+			goto err;
 		}
 		memcpy(name, ptr, namesize);
 
@@ -209,7 +210,7 @@ static int decode_programmer_archive(struct sahara_image *images)
 		id = strtoul(tok, NULL, 0);
 		if (id == 0 || id >= MAPPING_SZ) {
 			ux_err("invalid image id \"%s\" in programmer archive\n", tok);
-			return -1;
+			goto err;
 		}
 
 		ptr += namesize;
@@ -231,10 +232,18 @@ static int decode_programmer_archive(struct sahara_image *images)
 	blob->len = 0;
 
 	return 1;
+
+err:
+	sahara_images_free(images, MAPPING_SZ);
+	free(blob->ptr);
+	blob->ptr = NULL;
+	blob->len = 0;
+	return -1;
 }
 
 /**
  * decode_sahara_config() - Attempt to decode a Sahara config XML document
+ * @blob: Loaded image to be decoded as Sahara config
  * @images: List of Sahara images, with @images[0] populated
  *
  * The single blob provided in @images[0] might be a XML blob containing
@@ -246,9 +255,8 @@ static int decode_programmer_archive(struct sahara_image *images)
  *
  * Returns: 0 if no archive was found, 1 if archive was decoded, -1 on error
  */
-static int decode_sahara_config(struct sahara_image *images)
+static int decode_sahara_config(struct sahara_image *blob, struct sahara_image *images)
 {
-	struct sahara_image *blob = &images[0];
 	char image_path_full[PATH_MAX];
 	const char *image_path;
 	unsigned int image_id;
@@ -335,6 +343,10 @@ static int decode_sahara_config(struct sahara_image *images)
 	return 1;
 
 err_free_doc:
+	sahara_images_free(images, MAPPING_SZ);
+	free(blob->ptr);
+	blob->ptr = NULL;
+	blob->len = 0;
 	xmlFreeDoc(doc);
 	free(blob_name_buf);
 	return -1;
@@ -344,15 +356,15 @@ err_free_doc:
  * decode_programmer() - decodes the programmer specifier
  * @s: programmer specifier, from the user
  * @images: array of images to populate
- * @single: legacy single image specifier, for which image id should be ignored
  *
  * This parses the progammer specifier @s, which can either be a single
  * filename, or a comma-separated series of <id>:<filename> entries.
  *
- * In the first case @images[0] is assigned the provided filename and @single is
- * set to true. In the second case, each comma-separated entry will be split on
- * ':' and the given <filename> will be assigned to the @image entry indicated
- * by the given <id>.
+ * In the first case an attempt will be made to decode the Sahara archive and
+ * each programmer part will be loaded into their requestd @images entry. If
+ * the file isn't an archive @images[SAHARA_ID_EHOSTDL_IMG] is assigned. In the
+ * second case, each comma-separated entry will be split on ':' and the given
+ * <filename> will be assigned to the @image entry indicated by the given <id>.
  *
  * Memory is not allocated for the various strings, instead @s will be modified
  * by the tokenizer and pointers to the individual parts will be stored in the
@@ -360,57 +372,50 @@ err_free_doc:
  *
  * Returns: 0 on success, -1 otherwise.
  */
-static int decode_programmer(char *s, struct sahara_image *images, bool *single)
+static int decode_programmer(char *s, struct sahara_image *images)
 {
+	struct sahara_image archive;
 	char *filename;
 	char *save1;
-	char *save2;
 	char *pair;
-	char *id_str;
+	char *tail;
 	long id;
 	int ret;
 
-	if (!strchr(s, ':')) {
-		ret = load_sahara_image(s, &images[0]);
-		if (ret < 0)
-			return -1;
+	strtoul(s, &tail, 0);
+	if (tail != s && tail[0] == ':') {
+		for (pair = strtok_r(s, ",", &save1); pair; pair = strtok_r(NULL, ",", &save1)) {
+			id = strtoul(pair, &tail, 0);
+			if (tail == pair) {
+				ux_err("invalid programmer specifier\n");
+				return -1;
+			}
 
-		ret = decode_programmer_archive(images);
-		if (ret < 0)
-			return -1;
+			if (id == 0 || id >= MAPPING_SZ) {
+				ux_err("invalid image id \"%s\"\n", pair);
+				return -1;
+			}
 
-		if (!ret) {
-			ret = decode_sahara_config(images);
+			filename = &tail[1];
+			ret = load_sahara_image(filename, &images[id]);
 			if (ret < 0)
 				return -1;
 		}
-
-		*single = (ret == 0);
-
-		return 0;
-	}
-
-	for (pair = strtok_r(s, ",", &save1); pair; pair = strtok_r(NULL, ",", &save1)) {
-		id_str = strtok_r(pair, ":", &save2);
-		filename = strtok_r(NULL, ":", &save2);
-
-		if (!id_str || !filename) {
-			ux_err("failed to parse programmer specifier\n");
-			return -1;
-		}
-
-		id = strtoul(id_str, NULL, 0);
-		if (id == 0 || id >= MAPPING_SZ) {
-			ux_err("invalid image id \"%s\"\n", id_str);
-			return -1;
-		}
-
-		ret = load_sahara_image(filename, &images[id]);
+	} else {
+		ret = load_sahara_image(s, &archive);
 		if (ret < 0)
 			return -1;
-	}
 
-	*single = false;
+		ret = decode_programmer_archive(&archive, images);
+		if (ret < 0 || ret == 1)
+			return ret;
+
+		ret = decode_sahara_config(&archive, images);
+		if (ret < 0 || ret == 1)
+			return ret;
+
+		images[SAHARA_ID_EHOSTDL_IMG] = archive;
+	}
 
 	return 0;
 }
@@ -421,6 +426,8 @@ static void print_usage(FILE *out)
 
 	fprintf(out, "Usage: %s [options] <prog.mbn> (<program-xml> | <patch-xml> | <read-xml>)...\n", __progname);
 	fprintf(out, "       %s [options] <prog.mbn> ((read | write) <address> <binary>)...\n", __progname);
+	fprintf(out, "       %s list\n", __progname);
+	fprintf(out, "       %s ramdump [--debug] [-o <ramdump-path>] [<segment-filter>,...]\n", __progname);
 	fprintf(out, " -d, --debug\t\t\tPrint detailed debug info\n");
 	fprintf(out, " -v, --version\t\t\tPrint the current version and exit\n");
 	fprintf(out, " -n, --dry-run\t\t\tDry run execution, no device reading or flashing\n");
@@ -434,21 +441,122 @@ static void print_usage(FILE *out)
 	fprintf(out, " -T, --slot=T\t\t\tSet slot number T for multiple storage devices\n");
 	fprintf(out, " -D, --vip-table-path=T\t\tUse digest tables in the T folder for VIP\n");
 	fprintf(out, " -h, --help\t\t\tPrint this usage info\n");
-	fprintf(out, " <program-xml>\txml file containing <program> or <erase> directives\n");
-	fprintf(out, " <patch-xml>\txml file containing <patch> directives\n");
-	fprintf(out, " <read-xml>\txml file containing <read> directives\n");
-	fprintf(out, " <address>\tdisk address specifier, can be one of <P>, <P/S>, <P/S+L>, <name>, or\n");
-	fprintf(out, "          \t<P/name>, to specify a physical partition number P, a starting sector\n");
-	fprintf(out, "          \tnumber S, the number of sectors to follow L, or partition by \"name\"\n");
+	fprintf(out, " <program-xml>\t\txml file containing <program> or <erase> directives\n");
+	fprintf(out, " <patch-xml>\t\txml file containing <patch> directives\n");
+	fprintf(out, " <read-xml>\t\txml file containing <read> directives\n");
+	fprintf(out, " <address>\t\tdisk address specifier, can be one of <P>, <P/S>, <P/S+L>, <name>, or\n");
+	fprintf(out, "          \t\t<P/name>, to specify a physical partition number P, a starting sector\n");
+	fprintf(out, "          \t\tnumber S, the number of sectors to follow L, or partition by \"name\"\n");
+	fprintf(out, " <ramdump-path>\t\tpath where ramdump should stored\n");
+	fprintf(out, " <segment-filter>\toptional glob-pattern to select which segments to ramdump\n");
 	fprintf(out, "\n");
 	fprintf(out, "Example: %s prog_firehose_ddr.elf rawprogram*.xml patch*.xml\n", __progname);
 }
 
-int main(int argc, char **argv)
+static int qdl_list(FILE *out)
+{
+	struct qdl_device_desc *devices;
+	unsigned int count;
+	unsigned int i;
+
+	devices = usb_list(&count);
+	if (!devices)
+		return 1;
+
+	if (count == 0) {
+		fprintf(out, "No devices found\n");
+	} else {
+		for (i = 0; i < count; i++)
+			fprintf(out, "%04x:%04x\t%s\n",
+				devices[i].vid, devices[i].pid, devices[i].serial);
+	}
+
+	free(devices);
+
+	return 0;
+}
+
+static int qdl_ramdump(int argc, char **argv)
+{
+	struct qdl_device *qdl;
+	char *ramdump_path = ".";
+	char *filter = NULL;
+	char *serial = NULL;
+	int ret = 0;
+	int opt;
+
+	static struct option options[] = {
+		{"debug", no_argument, 0, 'd'},
+		{"version", no_argument, 0, 'v'},
+		{"output", required_argument, 0, 'o'},
+		{"serial", required_argument, 0, 'S'},
+		{"help", no_argument, 0, 'h'},
+		{0, 0, 0, 0}
+	};
+
+	while ((opt = getopt_long(argc, argv, "dvo:S:h", options, NULL)) != -1) {
+		switch (opt) {
+		case 'd':
+			qdl_debug = true;
+			break;
+		case 'v':
+			print_version();
+			return 0;
+		case 'o':
+			ramdump_path = optarg;
+			break;
+		case 'S':
+			serial = optarg;
+			break;
+		case 'h':
+			print_usage(stdout);
+			return 0;
+		default:
+			print_usage(stderr);
+			return 1;
+		}
+	}
+
+	if (optind < argc)
+		filter = argv[optind++];
+
+	if (optind != argc) {
+		print_usage(stderr);
+		return 1;
+	}
+
+	ux_init();
+
+	qdl = qdl_init(QDL_DEVICE_USB);
+	if (!qdl)
+		return 1;
+
+	if (qdl_debug)
+		print_version();
+
+	ret = qdl_open(qdl, serial);
+	if (ret) {
+		ret = 1;
+		goto out_cleanup;
+	}
+
+	ret = sahara_run(qdl, NULL, ramdump_path, filter);
+	if (ret < 0) {
+		ret = 1;
+		goto out_cleanup;
+	}
+
+out_cleanup:
+	qdl_close(qdl);
+	qdl_deinit(qdl);
+
+	return ret;
+}
+
+static int qdl_flash(int argc, char **argv)
 {
 	enum qdl_storage_type storage_type = QDL_STORAGE_UFS;
 	struct sahara_image sahara_images[MAPPING_SZ] = {};
-	bool single_image = true;
 	char *incdir = NULL;
 	char *serial = NULL;
 	const char *vip_generate_dir = NULL;
@@ -570,7 +678,7 @@ int main(int argc, char **argv)
 	if (qdl_debug)
 		print_version();
 
-	ret = decode_programmer(argv[optind++], sahara_images, &single_image);
+	ret = decode_programmer(argv[optind++], sahara_images);
 	if (ret < 0)
 		exit(1);
 
@@ -635,7 +743,7 @@ int main(int argc, char **argv)
 
 	qdl->storage_type = storage_type;
 
-	ret = sahara_run(qdl, sahara_images, single_image, NULL, NULL);
+	ret = sahara_run(qdl, sahara_images, NULL, NULL);
 	if (ret < 0)
 		goto out_cleanup;
 
@@ -651,6 +759,9 @@ out_cleanup:
 		vip_gen_finalize(qdl);
 
 	qdl_close(qdl);
+
+	sahara_images_free(sahara_images, MAPPING_SZ);
+
 	free_programs();
 	free_patches();
 
@@ -661,3 +772,13 @@ out_cleanup:
 
 	return !!ret;
 }
+
+int main(int argc, char **argv)
+{
+	if (argc == 2 && !strcmp(argv[1], "list"))
+		return qdl_list(stdout);
+	if (argc >= 2 && !strcmp(argv[1], "ramdump"))
+		return qdl_ramdump(argc - 1, argv + 1);
+
+	return qdl_flash(argc, argv);
+}

qdl.h

@@ -2,6 +2,13 @@
 #ifndef __QDL_H__
 #define __QDL_H__
 
+#ifdef _WIN32
+#include <malloc.h>
+#define alloca _alloca
+#else
+#include <alloca.h>
+#endif
+
 #include <stdbool.h>
 
 #include "patch.h"
@@ -36,6 +43,8 @@
 
 #define MAPPING_SZ 64
 
+#define SAHARA_ID_EHOSTDL_IMG	13
+
 enum QDL_DEVICE_TYPE {
 	QDL_DEVICE_USB,
 	QDL_DEVICE_SIM,
@@ -70,7 +79,7 @@ struct qdl_device {
 };
 
 struct sahara_image {
-	const char *name;
+	char *name;
 	void *ptr;
 	size_t len;
 };
@@ -89,13 +98,22 @@ int qdl_vip_transfer_enable(struct qdl_device *qdl, const char *vip_table_path);
 struct qdl_device *usb_init(void);
 struct qdl_device *sim_init(void);
 
+struct qdl_device_desc {
+	int vid;
+	int pid;
+	char serial[16];
+};
+
+struct qdl_device_desc *usb_list(unsigned int *devices_found);
+
 int firehose_run(struct qdl_device *qdl);
 int firehose_provision(struct qdl_device *qdl);
 int firehose_read_buf(struct qdl_device *qdl, struct read_op *read_op, void *out_buf, size_t out_size);
 int sahara_run(struct qdl_device *qdl, const struct sahara_image *images,
-	       bool single_image, const char *ramdump_path,
+	       const char *ramdump_path,
 	       const char *ramdump_filter);
 int load_sahara_image(const char *filename, struct sahara_image *image);
+void sahara_images_free(struct sahara_image *images, size_t count);
 void print_hex_dump(const char *prefix, const void *buf, size_t len);
 unsigned int attr_as_unsigned(xmlNode *node, const char *attr, int *errors);
 const char *attr_as_string(xmlNode *node, const char *attr, int *errors);

ramdump.c

@@ -76,6 +76,8 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	ux_init();
+
 	if (qdl_debug)
 		print_version();
 
@@ -85,7 +87,7 @@ int main(int argc, char **argv)
 		goto out_cleanup;
 	}
 
-	ret = sahara_run(qdl, NULL, true, ramdump_path, filter);
+	ret = sahara_run(qdl, NULL, ramdump_path, filter);
 	if (ret < 0) {
 		ret = 1;
 		goto out_cleanup;

sahara.c

@@ -145,8 +145,7 @@ static void sahara_hello(struct qdl_device *qdl, struct sahara_pkt *pkt)
 }
 
 static void sahara_read(struct qdl_device *qdl, struct sahara_pkt *pkt,
-			const struct sahara_image *images,
-			bool single_image)
+			const struct sahara_image *images)
 {
 	const struct sahara_image *image;
 	unsigned int image_idx;
@@ -159,13 +158,10 @@ static void sahara_read(struct qdl_device *qdl, struct sahara_pkt *pkt,
 	ux_debug("READ image: %d offset: 0x%x length: 0x%x\n",
 		 pkt->read_req.image, pkt->read_req.offset, pkt->read_req.length);
 
-	if (single_image)
-		image_idx = 0;
-	else
-		image_idx = pkt->read_req.image;
-
+	image_idx = pkt->read_req.image;
 	if (image_idx >= MAPPING_SZ || !images[image_idx].ptr) {
-		ux_err("device requested invalid image: %u\n", image_idx);
+		ux_err("device requested unknown image id %u, ensure that all Sahara images are provided\n",
+		       image_idx);
 		sahara_send_reset(qdl);
 		return;
 	}
@@ -185,8 +181,7 @@ static void sahara_read(struct qdl_device *qdl, struct sahara_pkt *pkt,
 }
 
 static void sahara_read64(struct qdl_device *qdl, struct sahara_pkt *pkt,
-			  const struct sahara_image *images,
-			  bool single_image)
+			  const struct sahara_image *images)
 {
 	const struct sahara_image *image;
 	unsigned int image_idx;
@@ -199,13 +194,10 @@ static void sahara_read64(struct qdl_device *qdl, struct sahara_pkt *pkt,
 	ux_debug("READ64 image: %" PRId64 " offset: 0x%" PRIx64 " length: 0x%" PRIx64 "\n",
 		 pkt->read64_req.image, pkt->read64_req.offset, pkt->read64_req.length);
 
-	if (single_image)
-		image_idx = 0;
-	else
-		image_idx = pkt->read64_req.image;
-
+	image_idx = pkt->read64_req.image;
 	if (image_idx >= MAPPING_SZ || !images[image_idx].ptr) {
-		ux_err("device requested invalid image: %u\n", image_idx);
+		ux_err("device requested unknown image id %u, ensure that all Sahara images are provided\n",
+		       image_idx);
 		sahara_send_reset(qdl);
 		return;
 	}
@@ -317,6 +309,8 @@ static ssize_t sahara_debug64_one(struct qdl_device *qdl,
 		qdl_read(qdl, buf, DEBUG_BLOCK_SIZE, 10);
 
 		chunk += DEBUG_BLOCK_SIZE;
+
+		ux_progress("%s", chunk, region.length, region.filename);
 	}
 out:
 
@@ -396,8 +390,10 @@ static void sahara_debug64(struct qdl_device *qdl, struct sahara_pkt *pkt,
 		return;
 
 	for (i = 0; i < pkt->debug64_req.length / sizeof(table[0]); i++) {
-		if (sahara_debug64_filter(table[i].filename, filter))
+		if (sahara_debug64_filter(table[i].filename, filter)) {
+			ux_info("%s skipped per filter\n", table[i].filename);
 			continue;
+		}
 
 		ux_debug("%-2d: type 0x%" PRIx64 " address: 0x%" PRIx64 " length: 0x%"
 			 PRIx64 " region: %s filename: %s\n",
@@ -407,6 +403,8 @@ static void sahara_debug64(struct qdl_device *qdl, struct sahara_pkt *pkt,
 		n = sahara_debug64_one(qdl, table[i], ramdump_path);
 		if (n < 0)
 			break;
+
+		ux_info("%s dumped successfully\n", table[i].filename);
 	}
 
 	free(table);
@@ -414,12 +412,26 @@ static void sahara_debug64(struct qdl_device *qdl, struct sahara_pkt *pkt,
 	sahara_send_reset(qdl);
 }
 
-static void sahara_debug_list_images(const struct sahara_image *images, bool single_image)
+static bool sahara_has_done_pending_quirk(const struct sahara_image *images)
 {
+	unsigned int count = 0;
 	int i;
 
-	if (single_image)
-		ux_debug("Sahara image id in read requests will be ignored\n");
+	/*
+	 * E.g MSM8916 EDL reports done = pending, allow this when one a single
+	 * image is provided, and it's used as SAHARA_ID_EHOSTDL_IMG.
+	 */
+	for (i = 0; i < MAPPING_SZ; i++) {
+		if (images[i].ptr)
+			count++;
+	}
+
+	return count == 1 && images[SAHARA_ID_EHOSTDL_IMG].ptr;
+}
+
+static void sahara_debug_list_images(const struct sahara_image *images)
+{
+	int i;
 
 	ux_debug("Sahara images:\n");
 	for (i = 0; i < MAPPING_SZ; i++) {
@@ -429,7 +441,7 @@ static void sahara_debug_list_images(const struct sahara_image *images, bool sin
 }
 
 int sahara_run(struct qdl_device *qdl, const struct sahara_image *images,
-	       bool single_image, const char *ramdump_path,
+	       const char *ramdump_path,
 	       const char *ramdump_filter)
 {
 	struct sahara_pkt *pkt;
@@ -438,7 +450,8 @@ int sahara_run(struct qdl_device *qdl, const struct sahara_image *images,
 	bool done = false;
 	int n;
 
-	sahara_debug_list_images(images, single_image);
+	if (images)
+		sahara_debug_list_images(images);
 
 	/*
 	 * Don't need to do anything in simulation mode with Sahara,
@@ -465,7 +478,7 @@ int sahara_run(struct qdl_device *qdl, const struct sahara_image *images,
 			sahara_hello(qdl, pkt);
 			break;
 		case SAHARA_READ_DATA_CMD:
-			sahara_read(qdl, pkt, images, single_image);
+			sahara_read(qdl, pkt, images);
 			break;
 		case SAHARA_END_OF_IMAGE_CMD:
 			sahara_eoi(qdl, pkt);
@@ -474,14 +487,14 @@ int sahara_run(struct qdl_device *qdl, const struct sahara_image *images,
 			done = sahara_done(qdl, pkt);
 
 			/* E.g MSM8916 EDL reports done = 0 here */
-			if (single_image)
+			if (sahara_has_done_pending_quirk(images))
 				done = true;
 			break;
 		case SAHARA_MEM_DEBUG64_CMD:
 			sahara_debug64(qdl, pkt, ramdump_path, ramdump_filter);
 			break;
 		case SAHARA_READ_DATA64_CMD:
-			sahara_read64(qdl, pkt, images, single_image);
+			sahara_read64(qdl, pkt, images);
 			break;
 		case SAHARA_RESET_RESP_CMD:
 			assert(pkt->length == SAHARA_RESET_LENGTH);

tests/test_vip_generation.sh

@@ -9,7 +9,7 @@ FLAT_BUILD=${SCRIPT_PATH}/data
 
 REP_ROOT=${SCRIPT_PATH}/..
 VIP_PATH=${FLAT_BUILD}/vip
-EXPECTED_DIGEST="a05e1124edbe34dc504a327544fb66572591353dc3fa25e6e7eafbe4803e63e0"
+EXPECTED_DIGEST="d93fc596a037abe4977f50ca68e1bf57377299a496cb1436a9421579517cef13"
 VIP_TABLE_FILE=${VIP_PATH}/DigestsToSign.bin
 
 mkdir -p $VIP_PATH

usb.c

@@ -223,6 +223,89 @@ static int usb_open(struct qdl_device *qdl, const char *serial)
 	return -1;
 }
 
+struct qdl_device_desc *usb_list(unsigned int *devices_found)
+{
+	struct libusb_device_descriptor desc;
+	struct libusb_device_handle *handle;
+	struct qdl_device_desc *result;
+	struct libusb_device **devices;
+	struct libusb_device *dev;
+	unsigned long serial_len;
+	unsigned char buf[128];
+	ssize_t device_count;
+	unsigned int count = 0;
+	char *serial;
+	int ret;
+	int i;
+
+	ret = libusb_init(NULL);
+	if (ret < 0)
+		err(1, "failed to initialize libusb");
+
+	device_count = libusb_get_device_list(NULL, &devices);
+	if (device_count < 0)
+		err(1, "failed to list USB devices");
+	if (device_count == 0)
+		return NULL;
+
+	result = calloc(device_count, sizeof(struct qdl_device));
+	if (!result)
+		err(1, "failed to allocate devices array\n");
+
+	for (i = 0; i < device_count; i++) {
+		dev = devices[i];
+
+		ret = libusb_get_device_descriptor(dev, &desc);
+		if (ret < 0) {
+			warnx("failed to get USB device descriptor");
+			continue;
+		}
+
+		if (desc.idVendor != 0x05c6)
+			continue;
+		if (desc.idProduct != 0x9008 && desc.idProduct != 0x900e && desc.idProduct != 0x901d)
+			continue;
+
+		ret = libusb_open(dev, &handle);
+		if (ret < 0) {
+			warnx("unable to open USB device");
+			continue;
+		}
+
+		ret = libusb_get_string_descriptor_ascii(handle, desc.iProduct, buf, sizeof(buf) - 1);
+		if (ret < 0) {
+			warnx("failed to read iProduct descriptor: %s", libusb_strerror(ret));
+			continue;
+		}
+		buf[ret] = '\0';
+
+		serial = strstr((char *)buf, "_SN:");
+		if (!serial) {
+			ux_err("ignoring device with no serial number\n");
+			continue;
+		}
+
+		serial += strlen("_SN:");
+		serial_len = strcspn(serial, " _");
+		if (serial_len + 1 > sizeof(result[count].serial)) {
+			ux_err("ignoring device with unexpectedly long serial number\n");
+			continue;
+		}
+
+		memcpy(result[count].serial, serial, serial_len);
+		result[count].serial[serial_len] = '\0';
+
+		result[count].vid = desc.idVendor;
+		result[count].pid = desc.idProduct;
+		count++;
+	}
+
+	libusb_free_device_list(devices, 1);
+	*devices_found = count;
+
+	return result;
+}
+
 static void usb_close(struct qdl_device *qdl)
 {
 	struct qdl_device_usb *qdl_usb = container_of(qdl, struct qdl_device_usb, base);
@@ -245,7 +328,7 @@ static int usb_read(struct qdl_device *qdl, void *buf, size_t len, unsigned int
 		return -ETIMEDOUT;
 
 	/* If what we read equals the endpoint's Max Packet Size, consume the ZLP explicitly */
-	if (len == actual && !(actual % qdl_usb->in_maxpktsize)) {
+	if (len == (size_t)actual && !(actual % qdl_usb->in_maxpktsize)) {
 		ret = libusb_bulk_transfer(qdl_usb->usb_handle, qdl_usb->in_ep,
 					   NULL, 0, NULL, timeout);
 		if (ret)

util.c

@@ -242,6 +242,10 @@ int load_sahara_image(const char *filename, struct sahara_image *image)
 	lseek(fd, 0, SEEK_SET);
 
 	ptr = malloc(len);
+	if (!ptr) {
+		ux_err("failed to init buffer for content of \"%s\"\n", filename);
+		goto err_close;
+	}
 
 	n = read(fd, ptr, len);
 	if (n != len) {
@@ -262,3 +266,12 @@ err_close:
 	close(fd);
 	return -1;
 }
+
+void sahara_images_free(struct sahara_image *images, size_t count)
+{
+	for (size_t i = 0; i < count; i++) {
+		free(images[i].name);
+		free(images[i].ptr);
+		images[i] = (struct sahara_image){};
+	}
+}

ux.c

@@ -151,6 +151,9 @@ void ux_progress(const char *fmt, unsigned int value, unsigned int max, ...)
 			return;
 	}
 
+	if (value > max)
+		value = max;
+
 	va_start(ap, max);
 	vsnprintf(task_name, sizeof(task_name), fmt, ap);
 	va_end(ap);