ramips: fix netgear r6120 factory image generation

as indicated in commit c5bf408ed6bd "(ramips: fix image generation for mt76x8")
more rework was needed to fix the other issues.

Building on another machine, but using the same arch, showed
the application failing again for different reasons.

Fix this by completely rewriting the application, fixing following found issues:

- buffer overflows, resulting in stack corruption
- flaws in memory requirement calculations (too small, too large)
- memory leaks
- missing bounds checking on string handling
- non-reproducable images, by using unitilized memory in checksum calculation
- missing error handling, resulting in succes on specific image errors
- endianness errors when building on BE machines
- various minor build warnings
- documentation did not match the code actions (header item locations)
- allowing input to be decimal, hex or octal now

Signed-off-by: Koen Vandeputte <[email protected]>

target/linux/ramips/image/mt76x8.mk

@@ -115,10 +115,10 @@ define Device/netgear_r6120
   IMAGE_SIZE := $(ralink_default_fw_size_16M)
   DEVICE_TITLE := Netgear AC1200 R6120
   DEVICE_PACKAGES := kmod-usb2 kmod-usb-ohci
-  SERCOMM_KERNEL_OFFSET := 90000
+  SERCOMM_KERNEL_OFFSET := 0x90000
   SERCOMM_HWID := CGQ
   SERCOMM_HWVER := A001
-  SERCOMM_SWVER := 0040
+  SERCOMM_SWVER := 0x0040
   IMAGES += factory.img
   IMAGE/default := append-kernel | pad-to $$$$(BLOCKSIZE)| append-rootfs | pad-rootfs
   IMAGE/sysupgrade.bin := $$(IMAGE/default) | append-metadata | check-size $$$$(IMAGE_SIZE)

tools/firmware-utils/src/mksercommfw.c

@@ -2,139 +2,198 @@
 #include <string.h>
 #include <stdlib.h>
 #include <sys/types.h>
+#include <unistd.h>
+#include <asm/byteorder.h>
 
 /* #define DEBUG 1 */
 
-/*
- * Fw Header Layout for Netgear / Sercomm devices
- * */
-static const char *magic = "sErCoMm"; /* 7 */
-/* 7-11: version control/download control ? */
-unsigned char version[4] = {0x00, 0x01, 0x00, 0x00};
-char *hwID = ""; /* 11-43 , ASCII/HEX */
-char *hwVer = ""; /* 44-57 , ASCII/HEX */
-char *swVer = ""; /* 58-62 , ASCII/HEX */
-/* magic again. */
+#ifdef DEBUG
+#define DBG(...) {printf(__VA_ARGS__); }
+#else
+#define DBG(...) {}
+#endif
 
+#define ERR(...) {printf(__VA_ARGS__); }
+#define ALIGN(a,b) ((a) + ((b) - ((a) % (b))))
+#define ROOTFS_ALIGN 128
 #define HEADER_SIZE 71
 
-/* null bytes until 511 */
-u_int32_t checksum = 0xFF; /* checksum */
-/* 512 onwards -> ZIP containing rootfs with the same Header */
+/*
+ * Fw Header Layout for Netgear / Sercomm devices (bytes)
+ *
+ * Size : 512 bytes + zipped image size
+ *
+ * Locations:
+ * magic  : 0-6    ASCII
+ * version: 7-11   fixed
+ * hwID   : 11-44  ASCII
+ * hwVer  : 45-54  ASCII
+ * swVer  : 55-62  uint32_t in BE
+ * magic  : 63-69  ASCII
+ * ChkSum : 511    Inverse value of the full image checksum while this location is 0x00
+ */
+
+static const char* magic = "sErCoMm"; /* 7 */
 
+/* 7-11: version control/download control ? */
+static const unsigned char version[4] = { 0x00, 0x01, 0x00, 0x00 };
 
-/* appended on rootfs for the Header. */
-const int footer_size = 128;
+
+/* 512 onwards -> ZIP containing rootfs with the same Header */
 
 struct file_info {
-	char		*file_name;	/* name of the file */
-	char		*file_data;	/* data of the file in memory */
-	u_int32_t	 file_size;	/* length of the file */
+	char* file_name; /* name of the file */
+	char* file_data; /* data of the file in memory */
+	u_int32_t file_size; /* length of the file */
 };
 
-u_int8_t getCheckSum(char *data, int len)
-{
+static u_int8_t getCheckSum(char* data, int len) {
+	u_int8_t new = 0;
 
-	int32_t previous = 0;
-	u_int32_t new = 0;
+	if (!data) {
+		ERR("Invalid pointer provided!\n");
+		return 0;
+	}
 
-	for (u_int32_t i = 0; i < len; i++) {
-		new = (data[i] + previous) % 256;
-		previous = new | previous & -256;
+	for (int i = 0; i < len; i++) {
+		new += data[i];
 	}
-	return (u_int8_t) new;
+
+	return new;
 }
 
-void *bufferFile(struct file_info *finfo, int dontload)
-{
+static int bufferFile(struct file_info* finfo) {
 	int fs = 0;
-	FILE *f = NULL;
+	FILE* fp = NULL;
 
-#ifdef DEBUG
-	printf("Opening file: %s\n", finfo->file_name);
-#endif
-	f = fopen(finfo->file_name, "rb");
-	if (f == NULL) {
-		perror("Error");
-		exit(1);
+	if (!finfo || !finfo->file_name) {
+		ERR("Invalid pointer provided!\n");
+		return -1;
 	}
 
-	fseek(f, 0L, SEEK_END);
-	fs = ftell(f);
-	rewind(f);
+	DBG("Opening file: %s\n", finfo->file_name);
 
-#ifdef DEBUG
-	printf("Filesize: %i .\n", fs);
-#endif
+	if (!(fp = fopen(finfo->file_name, "rb"))) {
+		ERR("Error opening file: %s\n", finfo->file_name);
+		return -1;
+	}
 
-	finfo->file_size = fs;
+	/* Get filesize */
+	rewind(fp);
+	fseek(fp, 0L, SEEK_END);
+	fs = ftell(fp);
+	rewind(fp);
 
-	if (dontload) {
-		return 0;
+	if (fs < 0) {
+		ERR("Error getting filesize: %s\n", finfo->file_name);
+		fclose(fp);
+		return -1;
 	}
 
-	char *data = malloc(fs);
-	finfo->file_data = data;
+	DBG("Filesize: %i\n", fs);
+	finfo->file_size = fs;
 
-	int read = fread(data, fs, 1, f);
+	if (!(finfo->file_data = malloc(fs))) {
+		ERR("Out of memory!\n");
+		fclose(fp);
+		return -1;
+	}
 
-	if (read != 1) {
-		printf("Error reading file %s.", finfo->file_name);
-		exit(1);
+	if (fread(finfo->file_data, 1, fs, fp) != fs) {
+		ERR("Error reading file %s\n", finfo->file_name);
+		fclose(fp);
+		return -1;
 	}
 
-#ifdef DEBUG
-	printf("File: read successfully %i bytes.\n", read*fs);
-#endif
-	fclose(f);
+	DBG("File: read successful\n");
+	fclose(fp);
+
+	return 0;
 }
 
-void *writeFile(struct file_info *finfo)
-{
+static int writeFile(struct file_info* finfo) {
+	FILE* fp;
 
-#ifdef DEBUG
-	printf("Writing file: %s.\n", finfo->file_name);
-#endif
+	if (!finfo || !finfo->file_name) {
+		ERR("Invalid pointer provided!\n");
+		return -1;
+	}
+
+	DBG("Opening file: %s\n", finfo->file_name);
+
+	if (!(fp = fopen(finfo->file_name, "w"))) {
+		ERR("Error opening file: %s\n", finfo->file_name);
+		return -1;
+	}
 
-	FILE *fout = fopen(finfo->file_name, "w");
+	DBG("Writing file: %s\n", finfo->file_name);
 
-	if (!fwrite(finfo->file_data, finfo->file_size, 1, fout)) {
-		printf("Wanted to write, but something went wrong.\n");
-		fclose(fout);
-		exit(1);
+	if (fwrite(finfo->file_data, 1, finfo->file_size, fp) != finfo->file_size) {
+		ERR("Wanted to write, but something went wrong!\n");
+		fclose(fp);
+		return -1;
 	}
-	fclose(fout);
+
+	fclose(fp);
+	return 0;
 }
 
-void *rmFile(struct file_info *finfo)
-{
-	remove(finfo->file_name);
-	free(finfo->file_data);
+static void fi_clean(struct file_info* finfo) {
+	if (!finfo)
+		return;
+
+	if (finfo->file_name) {
+		finfo->file_name = NULL;
+	}
+
+	if (finfo->file_data) {
+		free(finfo->file_data);
+		finfo->file_data = NULL;
+	}
+
 	finfo->file_size = 0;
 }
 
-void *usage(char *argv[])
-{
-	printf("Usage: %s <sysupgradefile> <kernel_offset> <HWID> <HWVER> <SWID>\n"
-		"All are positional arguments ...	\n"
-		"	sysupgradefile:		File with the kernel uimage at 0\n"
-		"	kernel_offset:		Offset in Hex where the kernel is located\n"
-		"	HWID:			Hardware ID, ASCII\n"
-		"	HWVER:			Hardware Version, ASCII\n"
-		"	SWID:			Software Version, Hex\n"
-		"	\n"
-		"	", argv[0]);
+static void usage(char* argv[]) {
+    printf("Usage: %s <sysupgradefile> <kernel_offset> <HWID> <HWVER> <SWID>\n"
+           "All are positional arguments ...	\n"
+           "	sysupgradefile:     File with the kernel uimage at 0\n"
+           "	kernel_offset:      Offset where the kernel is located (decimal, hex or octal notation)\n"
+           "	HWID:               Hardware ID, ASCII\n"
+           "	HWVER:              Hardware Version, ASCII\n"
+           "	SWID:               Software Version (decimal, hex or octal notation)\n"
+           "	\n"
+           , argv[0]);
 }
 
-int main(int argc, char *argv[])
-{
-	printf("Building fw image for sercomm devices.\n");
+int main(int argc, char* argv[]) {
+	int ret = 1;
+	int rootfsname_sz;
+	int zipfsname_sz;
+	int zipcmd_sz;
+	u_int32_t kernel_offset = 0x90000; /* offset for the kernel inside the rootfs, default val */
+	u_int32_t swVer = 0;
+	struct file_info sysupgrade = { 0 };
+	struct file_info header = { 0 };
+	struct file_info rootfs = { 0 };
+	struct file_info zippedfs = { 0 };
+	struct file_info image = { 0 };
+	char* hwID = NULL;
+	char* hwVer = NULL;
+	char* rootfsname = NULL;
+	char* zipfsname = NULL;
+	char* zipcmd = NULL;
+	u_int8_t chkSum;
 
 	if (argc == 2) {
-		struct file_info myfile = {argv[1], 0, 0};
-		bufferFile(&myfile, 0);
-		char chksum = getCheckSum(myfile.file_data, myfile.file_size);
-		printf("Checksum for File: %X.\n", chksum);
+		struct file_info myfile = { argv[1], 0, 0 };
+
+		if (bufferFile(&myfile))
+			return 1;
+
+		chkSum = getCheckSum(myfile.file_data, myfile.file_size);
+		printf("Checksum for File: 0x%hhX\n", chkSum);
+
 		return 0;
 	}
 
@@ -143,113 +202,202 @@ int main(int argc, char *argv[])
 		return 1;
 	}
 
-	/* Args */
+	printf("Building fw image for sercomm devices ..\n");
 
-	struct file_info sysupgrade = {argv[1], 0, 0};
-	bufferFile(&sysupgrade, 0);
+	/* process args */
+	hwID = argv[3];
+	hwVer = argv[4];
 
-	int kernel_offset = 0x90000; /* offset for the kernel inside the rootfs, default val */
-	sscanf(argv[2], "%X", &kernel_offset);
-#ifdef DEBUG
-	printf("Kernel_offset: at %X/%i bytes.\n", kernel_offset, kernel_offset);
-#endif
-	char *hwID = argv[3];
-	char *hwVer = argv[4];
-	u_int32_t swVer = 0;
-	sscanf(argv[5],"%4X",&swVer);
-	swVer = bswap_32(swVer);
+	sysupgrade.file_name = argv[1];
+	image.file_name = argv[1];
+	kernel_offset = (u_int32_t) strtol(argv[2], NULL, 0);
+	swVer = (u_int32_t) strtol(argv[5], NULL, 0);
+	swVer = __cpu_to_be32(swVer);
 
-	char *rootfsname = malloc(2*strlen(sysupgrade.file_name) + 8);
-	sprintf(rootfsname, "%s.rootfs", sysupgrade.file_name);
+	/* Check if files actually exist */
+	if (access(sysupgrade.file_name, (F_OK | R_OK))) {
+		/* Error */
+		ERR("File not found: %s\n", sysupgrade.file_name);
+		goto cleanup;
+	}
 
-	char *zipfsname = malloc(2*strlen(rootfsname) + 5);
-	sprintf(zipfsname, "%s.zip", rootfsname);
-	/* / Args */
+	/* Calculate amount of required memory (incl. 0-term) */
+	rootfsname_sz = strlen(sysupgrade.file_name) + 7 + 1;
+	zipfsname_sz = strlen(sysupgrade.file_name) + 7 + 4 + 1;
+
+	/* Allocate required memory */
+	if (!(rootfsname = (char*) malloc(rootfsname_sz)) || !(zipfsname =
+			(char*) malloc(zipfsname_sz))) {
+		/* Error */
+		ERR("Out of memory!\n");
+		goto cleanup;
+	}
+
+	/* Create filenames */
+	if (snprintf(rootfsname, rootfsname_sz, "%s.rootfs", sysupgrade.file_name)
+			>= rootfsname_sz
+			|| snprintf(zipfsname, zipfsname_sz, "%s.rootfs.zip",
+					sysupgrade.file_name) >= zipfsname_sz) {
+		/* Error */
+		ERR("Buffer too small!\n");
+		goto cleanup;
+	}
+
+	/* Buffer all files */
+	if (bufferFile(&sysupgrade)) {
+		/* Error */
+		goto cleanup;
+	}
+
+	DBG("Building header: %s %s %2X %s\n", hwID, hwVer, swVer, magic);
 
-#ifdef DEBUG
-	printf("Building header: %s %s %2X %s.\n", hwID , hwVer, swVer, magic);
-#endif
 	/* Construct the firmware header/magic */
-	struct file_info header = {0, 0, 0};
+	header.file_name = NULL;
 	header.file_size = HEADER_SIZE;
-	header.file_data = malloc(HEADER_SIZE);
-	bzero(header.file_data, header.file_size);
 
-	char *tg = header.file_data;
-	strcpy(tg, magic);
-	memcpy(tg+7, version, 4*sizeof(char));
-	strcpy(tg+11, hwID);
-	strcpy(tg+45, hwVer);
-	memcpy(tg+55, &swVer,sizeof(u_int32_t));
-	strcpy(tg+63, magic);
+	if (!(header.file_data = (char*) calloc(1, HEADER_SIZE))) {
+		/* Error */
+		ERR("Out of memory!\n");
+		goto cleanup;
+	}
+
+	strncpy(header.file_data + 0, magic, 7);
+	memcpy(header.file_data + 7, version, sizeof(version));
+	strncpy(header.file_data + 11, hwID, 34);
+	strncpy(header.file_data + 45, hwVer, 10);
+	memcpy(header.file_data + 55, &swVer, sizeof(swVer));
+	strncpy(header.file_data + 63, magic, 7);
+
+	DBG("Creating rootfs ..\n");
 
-#ifdef DEBUG
-	printf("Header done, now creating rootfs.");
-#endif
 	/* Construct a rootfs */
-	struct file_info rootfs = {0, 0, 0};
-	rootfs.file_size = sysupgrade.file_size + kernel_offset + footer_size;
-	rootfs.file_data =  malloc(rootfs.file_size);
-	bzero(rootfs.file_data, rootfs.file_size);
 	rootfs.file_name = rootfsname;
+	rootfs.file_size = ALIGN(
+			sysupgrade.file_size + kernel_offset + header.file_size,
+			ROOTFS_ALIGN);
+
+	if (!(rootfs.file_data = calloc(1, rootfs.file_size))) {
+		/* Error */
+		ERR("Out of memory!\n");
+		goto cleanup;
+	}
 
-	/* copy Owrt image to Kernel location */
-	memcpy(rootfs.file_data+kernel_offset, sysupgrade.file_data, sysupgrade.file_size);
+	/* copy Owrt image to kernel location */
+	memcpy(rootfs.file_data + kernel_offset, sysupgrade.file_data,
+			sysupgrade.file_size);
 
-	/* 22 added to get away from sysup image, no other reason.
-	 *  updater searches for magic anyway */
-	tg = rootfs.file_data + kernel_offset + sysupgrade.file_size+22;
+	/* Append header after the owrt image.  The updater searches for it */
+	memcpy(rootfs.file_data + kernel_offset + sysupgrade.file_size,
+			header.file_data, header.file_size);
 
-	memcpy(tg, header.file_data, header.file_size);
-	writeFile(&rootfs);
+	/* Write to file */
+	if (writeFile(&rootfs)) {
+		/* Error */
+		goto cleanup;
+	}
+
+	/* Construct a zip */
+	DBG("Preparing to zip ..\n");
 
-#ifdef DEBUG
-	printf("Preparing to zip.\n");
-#endif
 	/* now that we got the rootfs, repeat the whole thing again(sorta):
 	 * 1. zip the rootfs */
-	char *zipper = malloc(5 + 2*strlen(rootfs.file_name) + 6);
-	sprintf(zipper, "%s %s %s", "zip ", zipfsname, rootfs.file_name);
-	int ret = system(zipper);
+	zipcmd_sz = 3 + 1 + strlen(zipfsname) + 1 + strlen(rootfs.file_name) + 1;
+
+	if (!(zipcmd = malloc(zipcmd_sz))) {
+		/* Error */
+		ERR("Out of memory!\n");
+		goto cleanup;
+	}
 
-	/* clear rootfs file */
-	rmFile(&rootfs);
+	if (snprintf(zipcmd, zipcmd_sz, "%s %s %s", "zip", zipfsname,
+			rootfs.file_name) >= zipcmd_sz) {
+		/* Error */
+		ERR("Buffer too small!\n");
+		goto cleanup;
+	}
+
+	if (system(zipcmd)) {
+		/* Error */
+		ERR("Error creating a zip file!\n");
+		goto cleanup;
+	}
 
 	/* and load zipped fs */
-	struct file_info zippedfs = {zipfsname, 0, 0};
-	bufferFile(&zippedfs, 0);
+	zippedfs.file_name = zipfsname;
 
-#ifdef DEBUG
-	printf("Creating Image.\n");
-#endif
+	if (bufferFile(&zippedfs)) {
+		/* Error */
+		goto cleanup;
+	}
 
-	/* 2. create new file 512+rootfs size */
-	struct file_info image = {argv[1], 0, 0};
-	image.file_data = malloc(zippedfs.file_size + 512);
-	image.file_size = zippedfs.file_size + 512;
+	DBG("Creating Image.\n");
 
-	/* 3. copy zipfile at loc 512 */
-	memcpy(image.file_data+512, zippedfs.file_data, zippedfs.file_size);
-	rmFile(&zippedfs);
+	/* 2. create new file 512 + rootfs size */
+	image.file_size = zippedfs.file_size + 512;
+	if (!(image.file_data = malloc(zippedfs.file_size + 512))) {
+		/* Error */
+		ERR("Out of memory!\n");
+		goto cleanup;
+	}
 
-	/* 4. add header to file */
+	/* 3. add header to file */
 	memcpy(image.file_data, header.file_data, header.file_size);
 
-	/* 5. do a checksum run, and compute checksum */
-	char chksum = getCheckSum(image.file_data, image.file_size);
-#ifdef DEBUG
-	printf("Checksum for Image: %X.\n", chksum);
-#endif
+	/* 4. clear remaining space */
+	if (header.file_size < 512)
+		memset(image.file_data + header.file_size, 0, 512 - header.file_size);
 
-	/* 6. write the checksum invert into byte 511 to bring it to 0 */
-	chksum = (chksum ^ 0xFF) + 1;
-	memcpy(image.file_data+511, &chksum, 1);
+	/* 5. copy zipfile at loc 512 */
+	memcpy(image.file_data + 512, zippedfs.file_data, zippedfs.file_size);
 
-	chksum = getCheckSum(image.file_data, image.file_size);
-#ifdef DEBUG
-	printf("Checksum for after fix: %X.\n", chksum);
-#endif
-	/* 7. pray that the updater will accept the file */
-	writeFile(&image);
-	return 0;
+	/* 6. do a checksum run, and compute checksum */
+	chkSum = getCheckSum(image.file_data, image.file_size);
+
+	DBG("Checksum for Image: %hhX\n", chkSum);
+
+	/* 7. write the checksum inverted into byte 511 to bring it to 0 on verification */
+	chkSum = (chkSum ^ 0xFF) + 1;
+	image.file_data[511] = (char) chkSum;
+
+	chkSum = getCheckSum(image.file_data, image.file_size);
+	DBG("Checksum for after fix: %hhX\n", chkSum);
+
+	if (chkSum != 0) {
+		ERR("Invalid checksum!\n")
+		goto cleanup;
+	}
+
+	/* 8. pray that the updater will accept the file */
+	if (writeFile(&image)) {
+		/* Error */
+		goto cleanup;
+	}
+
+	/* All seems OK */
+	ret = 0;
+
+	cleanup:
+
+	if (rootfs.file_name && !access(rootfs.file_name, F_OK | W_OK))
+		remove(rootfs.file_name);
+
+	if (zippedfs.file_name && !access(zippedfs.file_name, F_OK | W_OK))
+		remove(zippedfs.file_name);
+
+	fi_clean(&sysupgrade);
+	fi_clean(&header);
+	fi_clean(&rootfs);
+	fi_clean(&zippedfs);
+	fi_clean(&image);
+
+	if (rootfsname)
+		free(rootfsname);
+
+	if (zipfsname)
+		free(zipfsname);
+
+	if (zipcmd)
+		free(zipcmd);
+
+	return ret;
 }