| Message ID | 20180815144403.4600-1-mail@david-bauer.net |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [OpenWrt-Devel] build: add mkrasimage | expand |
Hi David, a few code-styling nitpicks, see comments below: Am Mittwoch, 15. August 2018, 16:44:03 CEST schrieb David Bauer: > The current make-ras.sh image generation script for the ZyXEL NBG6617 > has portability issues with bash. Because of this, factory images are > currently not built correctly by the OpenWRT buildbots. > > This commit replaces the make-ras.sh by C-written mkrasimage. The old > script is still kept but can be deleted in the future. > > Signed-off-by: David Bauer <mail@david-bauer.net> > --- > include/image-commands.mk | 13 + > target/linux/ipq40xx/image/Makefile | 2 +- > tools/firmware-utils/Makefile | 1 + > tools/firmware-utils/src/mkrasimage.c | 374 ++++++++++++++++++++++++++ > 4 files changed, 389 insertions(+), 1 deletion(-) > create mode 100644 tools/firmware-utils/src/mkrasimage.c > > diff --git a/include/image-commands.mk b/include/image-commands.mk > index 3cc5dc21e1..bb5fe46e1a 100644 > --- a/include/image-commands.mk > +++ b/include/image-commands.mk > @@ -62,6 +62,19 @@ define Build/make-ras > @mv $@.new $@ > endef > > +define Build/zyxel-ras-image > + let \ > + newsize="$(subst k,* 1024,$(RAS_ROOTFS_SIZE))"; \ > + $(STAGING_DIR_HOST)/bin/mkrasimage \ > + -b $(RAS_BOARD) \ > + -v $(RAS_VERSION) \ > + -k $(call param_get_default,kernel,$(1),$(IMAGE_KERNEL)) \ > + -r $@ \ > + -s $$newsize \ > + -o $@.new > + @mv $@.new $@ > +endef > + > define Build/mkbuffaloimg > $(STAGING_DIR_HOST)/bin/mkbuffaloimg -B $(BOARDNAME) \ > -R $$(($(subst k, * 1024,$(ROOTFS_SIZE)))) \ > diff --git a/target/linux/ipq40xx/image/Makefile > b/target/linux/ipq40xx/image/Makefile index d1ee1004fd..6e4125db0b 100644 > --- a/target/linux/ipq40xx/image/Makefile > +++ b/target/linux/ipq40xx/image/Makefile > @@ -221,7 +221,7 @@ define Device/zyxel_nbg6617 > # at least as large as the one of the initial firmware image (not the > current # one on the device). This only applies to the Web-UI, the > bootlaoder ignores # this minimum-size. However, the larger image can be > flashed both ways. - IMAGE/factory.bin := append-rootfs | pad-rootfs | > check-size $$$$(ROOTFS_SIZE) | make-ras + IMAGE/factory.bin := > append-rootfs | pad-rootfs | check-size $$$$(ROOTFS_SIZE) | zyxel-ras-image > IMAGE/sysupgrade.bin/squashfs := append-rootfs | pad-rootfs | check-size > $$$$(ROOTFS_SIZE) | sysupgrade-tar rootfs=$$$$@ | append-metadata > DEVICE_PACKAGES := ipq-wifi-zyxel_nbg6617 uboot-envtools > endef > diff --git a/tools/firmware-utils/Makefile b/tools/firmware-utils/Makefile > index 436a43794c..00917c3417 100644 > --- a/tools/firmware-utils/Makefile > +++ b/tools/firmware-utils/Makefile > @@ -70,6 +70,7 @@ define Host/Compile > $(call cc,fix-u-media-header cyg_crc32,-Wall) > $(call cc,hcsmakeimage bcmalgo) > $(call cc,mkporayfw, -Wall) > + $(call cc,mkrasimage, --std=gnu99) > $(call cc,mkhilinkfw, -lcrypto) > $(call cc,mkdcs932, -Wall) > $(call cc,mkheader_gemtek,-lz) > diff --git a/tools/firmware-utils/src/mkrasimage.c > b/tools/firmware-utils/src/mkrasimage.c new file mode 100644 > index 0000000000..1cac7b5da9 > --- /dev/null > +++ b/tools/firmware-utils/src/mkrasimage.c > @@ -0,0 +1,374 @@ > +/* > + * --- ZyXEL header format --- > + * Original Version by Benjamin Berg <benjamin@sipsolutions.net> > + * C implementation based on generation-script by Christian Lamparter > <chunkeey@gmail.com> + * > + * The firmware image prefixed with a header (which is written into the MTD > device). + * The header is one erase block (~64KiB) in size, but the > checksum only convers the + * first 2KiB. Padding is 0xff. All integers are > in big-endian. > + * > + * The checksum is always a 16-Bit System V checksum (sum -s) stored in a > 32-Bit integer. + * > + * 4 bytes: checksum of the rootfs image > + * 4 bytes: length of the contained rootfs image file (big endian) > + * 32 bytes: Firmware Version string (NUL terminated, 0xff padded) > + * 4 bytes: checksum over the header partition (big endian - see below) > + * 64 bytes: Model (e.g. "NBG6617", NUL termiated, 0xff padded) > + * 4 bytes: checksum of the kernel partition > + * 4 bytes: length of the contained kernel image file (big endian) > + * rest: 0xff padding (To erase block size) > + * > + * The checksums are calculated by adding up all bytes and if a 16bit > + * overflow occurs, one is added and the sum is masked to 16 bit: > + * csum = csum + databyte; if (csum > 0xffff) { csum += 1; csum &= 0xffff > }; + * Should the file have an odd number of bytes then the byte len-0x800 > is + * used additionally. > + * > + * The checksum for the header is calculated over the first 2048 bytes with > + * the rootfs image checksum as the placeholder during calculation. + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published > + * by the Free Software Foundation. > + * > + */ > +#include <byteswap.h> > +#include <getopt.h> > +#include <libgen.h> > +#include <stdio.h> > +#include <string.h> > +#include <stdlib.h> > + > +#define VERSION_STRING_LEN 31 > +#define ROOTFS_HEADER_LEN 40 > + > +#define KERNEL_HEADER_LEN 8 > + > +#define BOARD_NAME_LEN 64 > +#define BOARD_HEADER_LEN 68 > + > +#define HEADER_PARTITION_CALC_LENGTH 2048 > +#define HEADER_PARTITION_LENGTH 0x10000 > + > +struct file_info { > + char *name; /* name of the file */ > + char *data; /* file content */ > + size_t size; /* length of the file */ > +}; > + > +static char *progname; > + > +static char *board_name = 0; > +static char *version_name = 0; > +static unsigned int rootfs_size = 0; > + > +static struct file_info kernel = {0, 0, 0}; This is technically correct, but I would prefer { NULL, NULL, 0 } (note also the whitespace), because using 0 for pointers looks unusual. > +static struct file_info rootfs = {0, 0, 0}; > +static struct file_info rootfs_out = {0, 0, 0}; > +static struct file_info out = {0, 0, 0}; > + > +#define ERR(fmt, ...) do { \ > + fflush(0); \ stderr is unbuffered by default, so no need to use fflush > + fprintf(stderr, "[%s] *** error: " fmt "\n", \ > + progname, ## __VA_ARGS__ ); \ > +} while (0) > + > +void bufferFile(struct file_info *finfo) { camelCase while the rest of your code does not use this style? Please don't use camelCase in C applications, it's not so common. > + unsigned int fs = 0; > + FILE *f = NULL; > + > + f = fopen(finfo->name, "rb"); > + if (f == NULL) { > + printf("Error while opening file %s.", finfo->name); > + exit(EXIT_FAILURE); > + } > + > + fseek(f, 0L, SEEK_END); > + fs = (unsigned int) ftell(f); > + rewind(f); > + > + finfo->size = fs; > + > + char *data = malloc(fs); I would prefer declaration at function beginning. > + finfo->data = data; > + size_t read = fread(data, fs, 1, f); Same here. And since read(2) is a well-known function, I would rename the variable. > + > + if (read != 1) { > + printf("Error reading file %s.", finfo->name); > + exit(EXIT_FAILURE); > + } > + > + fclose(f); > +} Maybe it's easier to mmap(2) the whole file instead of so much code to get the file into memory? > + > +void writeFile(struct file_info *finfo) { I would prefer the opening bracket { on the next line when function begin. > + FILE *fout = fopen(finfo->name, "w"); > + > + if (!fwrite(finfo->data, finfo->size, 1, fout)) { > + printf("Wanted to write, but something went wrong.\n"); Usually you really want to know _what_ exactly went wrong (disk full, permissions...), so use ferror(3). > + exit(1); > + } > +} > + > +void usage(int status) { > + FILE *stream = (status != EXIT_SUCCESS) ? stderr : stdout; > + > + fprintf(stream, "Usage: %s [OPTIONS...]\n", progname); > + fprintf(stream, > + "\n" > + "Options:\n" > + " -k <kernel> path for kernel image\n" > + " -r <rootfs> path for rootfs image\n" > + " -s <rfssize> size of output rootfs\n" > + " -v <version> version string\n" > + " -b <boardname> name of board to generate image for\n" > + " -h show this screen\n" > + ); > + > + exit(status); > +} > + > +static int sysv_chksm(const unsigned char *data, int size) { > + int r; > + int checksum; > + unsigned int s = 0; /* The sum of all the input bytes, modulo (UINT_MAX > + 1). */ + > + > + for (int i = 0; i < size; i++) { > + s += data[i]; > + } > + > + r = (s & 0xffff) + ((s & 0xffffffff) >> 16); > + checksum = (r & 0xffff) + (r >> 16); > + > + return checksum; > +} > + > +char *generate_rootfs_header(struct file_info rootfs, char *version) { > + size_t version_string_length; > + unsigned int chksm, size; > + char *rootfs_header; > + size_t ptr = 0; > + > + rootfs_header = malloc(ROOTFS_HEADER_LEN); What if malloc failed? > + memset(rootfs_header, 0xff, ROOTFS_HEADER_LEN); > + > + memcpy(rootfs_out.data, rootfs.data, rootfs.size); > + > + chksm = __bswap_32(sysv_chksm(rootfs_out.data, rootfs_out.size)); > + size = __bswap_32(rootfs_out.size); > + > + memcpy(rootfs_header + ptr, &chksm, 4); > + ptr += 4; > + > + memcpy(rootfs_header + ptr, &size, 4); > + ptr += 4; > + > + version_string_length = strlen(version) <= VERSION_STRING_LEN ? > strlen(version) : VERSION_STRING_LEN; + > + memcpy(rootfs_header + ptr, version, version_string_length); > + ptr += version_string_length; > + > + rootfs_header[ptr] = 0x0; > + > + return rootfs_header; > +} > + > +char *generate_kernel_header(struct file_info kernel) { > + unsigned int chksm, size; > + char *kernel_header; > + size_t ptr = 0; > + > + kernel_header = malloc(KERNEL_HEADER_LEN); > + memset(kernel_header, 0xff, KERNEL_HEADER_LEN); > + > + chksm = __bswap_32(sysv_chksm(kernel.data, kernel.size)); Calling a function starting with underscores looks really wrong. What endianess the result should have? Are you sure that this runs correctly on platforms? I would expect that swapping is only necessary on platform which have different endianess that the result should have... > + size = __bswap_32(kernel.size); > + > + memcpy(kernel_header + ptr, &chksm, 4); Some lines below, you are using the array style xyz[ptr], so I would prefer one style... > + ptr += 4; > + > + memcpy(kernel_header + ptr, &size, 4); > + > + return kernel_header; > +} > + > +unsigned int generate_board_header_checksum(char *kernel_hdr, char > *rootfs_hdr, char *boardname) { + char *board_hdr_tmp; > + size_t ptr = 0; > + > + board_hdr_tmp = malloc(HEADER_PARTITION_CALC_LENGTH); > + memset(board_hdr_tmp, 0xff, HEADER_PARTITION_CALC_LENGTH); > + > + memcpy(board_hdr_tmp, rootfs_hdr, ROOTFS_HEADER_LEN); > + ptr += ROOTFS_HEADER_LEN; > + > + memcpy(board_hdr_tmp + ptr, rootfs_hdr, 4); /* Use RootFS checksum as > placeholder */ + ptr += 4; > + > + memcpy(board_hdr_tmp + ptr, boardname, strlen(boardname)); /* Append > boardname */ + ptr += strlen(boardname); > + > + board_hdr_tmp[ptr] = 0x0; /* Add null-terminator */ > + ptr = ROOTFS_HEADER_LEN + 4 + BOARD_NAME_LEN; > + > + memcpy(board_hdr_tmp + ptr, kernel_hdr, 8); > + > + return __bswap_32(sysv_chksm(board_hdr_tmp, 2048)); > +} > + > +char *generate_board_header(char *kernel_hdr, char *rootfs_hdr, char > *boardname) { + unsigned int board_checksum; > + char *board_hdr; > + > + board_hdr = malloc(BOARD_HEADER_LEN); > + memset(board_hdr, 0xff, BOARD_HEADER_LEN); > + > + board_checksum = generate_board_header_checksum(kernel_hdr, rootfs_hdr, > boardname); + > + memcpy(board_hdr, &board_checksum, 4); > + memcpy(board_hdr + 4, boardname, strlen(boardname)); > + board_hdr[4 + strlen(boardname)] = 0x0; > + > + return board_hdr; > +} > + > +int build_image() { > + char *rootfs_header, *kernel_header, *board_header; > + > + size_t ptr; > + > + /* Load files */ > + bufferFile(&kernel); > + bufferFile(&rootfs); > + > + /* Allocate memory for temporary ouput rootfs */ typo > + rootfs_out.data = malloc(rootfs_out.size); > + memset(rootfs_out.data, 0x00, rootfs_out.size); Use calloc: https://vorpus.org/blog/why-does-calloc-exist/ > + > + /* Prepare headers */ > + rootfs_header = generate_rootfs_header(rootfs, version_name); > + kernel_header = generate_kernel_header(kernel); > + board_header = generate_board_header(kernel_header, rootfs_header, > board_name); + > + /* Prepare output file */ > + out.size = HEADER_PARTITION_LENGTH + rootfs_out.size + kernel.size; > + out.data = malloc(out.size); > + memset(out.data, 0xFF, out.size); > + > + /* Build output image */ > + memcpy(out.data, rootfs_header, ROOTFS_HEADER_LEN); > + memcpy(out.data + ROOTFS_HEADER_LEN, board_header, BOARD_HEADER_LEN); > + memcpy(out.data + ROOTFS_HEADER_LEN + BOARD_HEADER_LEN, kernel_header, > KERNEL_HEADER_LEN); + ptr = HEADER_PARTITION_LENGTH; > + memcpy(out.data + ptr, rootfs_out.data, rootfs_out.size); > + ptr += rootfs_out.size; > + memcpy(out.data + ptr, kernel.data, kernel.size); > + > + /* Write back output image */ > + writeFile(&out); > + > + /* Free allocated memory */ > + free(kernel.data); > + free(rootfs.data); > + free(out.data); > + free(rootfs_out.data); > + > + free(rootfs_header); > + free(kernel_header); > + free(board_header); > + > + return 0; > +} > + > +int check_options() { > + if (kernel.name == 0) { Please use NULL or "if (!kernel.name) {" > + ERR("No kernel filename supplied"); > + return -1; > + } > + > + if (rootfs.name == 0) { > + ERR("No rootfs filename supplied"); > + return -2; > + } > + > + if (out.name == 0) { > + ERR("No output filename supplied"); > + return -3; > + } > + > + if (board_name == 0) { > + ERR("No board-name supplied"); > + return -4; > + } > + > + if (version_name == 0) { > + ERR("No version supplied"); > + return -5; > + } > + > + if (rootfs_size <= 0) { > + ERR("Invalid rootfs size supplied"); > + return -6; > + } > + > + if (strlen(board_name) > 31) { > + ERR("Board name is to long"); > + return -7; > + } > + return 0; > +} > + > +int main(int argc, char *argv[]) { > + int ret; > + progname = basename(argv[0]); > + while (1) { > + int c; > + > + c = getopt(argc, argv, "b:k:o:r:s:v:h"); > + if (c == -1) > + break; > + > + switch (c) { > + case 'b': > + board_name = optarg; > + break; > + case 'h': > + usage(EXIT_SUCCESS); > + break; > + case 'k': > + kernel.name = optarg; > + break; > + case 'o': > + out.name = optarg; > + break; > + case 'r': > + rootfs.name = optarg; > + break; > + case 's': > + sscanf(optarg, "%u", &rootfs_size); > + break; > + case 'v': > + version_name = optarg; > + break; > + default: > + usage(EXIT_FAILURE); > + break; > + } > + } > + > + ret = check_options(); > + if (ret) > + usage(EXIT_FAILURE); > + > + /* As ZyXEL Web-GUI only accept images with a rootfs equal or larger > than the first firmware shipped + * for the device, we need to pad > rootfs partition to this size. To perform further calculations, we + * > decide the size of this part here. In case the rootfs we want to integrate > in our image is larger, + * take it's size, otherwise the supplied > size. > + * > + * Be careful! We rely on assertion of correct size to be performed > beforehand. It is unknown if images + * with a to large rootfs are > accepted or not. > + */ > + rootfs_out.size = rootfs_size < rootfs.size ? rootfs.size : > rootfs_size; + return build_image(); > +} There are also some warnings when compiling with -Wall, maybe you could have a look. Thanks, Michael
David Bauer <mail@david-bauer.net> wrote: > The current make-ras.sh image generation script for the ZyXEL > NBG6617 has portability issues with bash. Because of this, > factory images are currently not built correctly by the OpenWRT > buildbots. > > This commit replaces the make-ras.sh by C-written mkrasimage. > The old script is still kept but can be deleted in the future. 1) how bad are the portability issues that you felt reimplementing in _C_ was the best path? 2) if it's that bad, why keep it? How will future people knwo what to use. Either get rid of it, or fix it. Cheers, Karl P > > Signed-off-by: David Bauer <mail@david-bauer.net> > --- > include/image-commands.mk | 13 + > target/linux/ipq40xx/image/Makefile | 2 +- > tools/firmware-utils/Makefile | 1 + > tools/firmware-utils/src/mkrasimage.c | 374 ++++++++++++++++++++++++++ > 4 files changed, 389 insertions(+), 1 deletion(-) > create mode 100644 tools/firmware-utils/src/mkrasimage.c > > diff --git a/include/image-commands.mk > b/include/image-commands.mk index 3cc5dc21e1..bb5fe46e1a 100644 > --- a/include/image-commands.mk > +++ b/include/image-commands.mk > @@ -62,6 +62,19 @@ define Build/make-ras > @mv $@.new $@ > endef > > +define Build/zyxel-ras-image > + let \ > + newsize="$(subst k,* 1024,$(RAS_ROOTFS_SIZE))"; \ > + $(STAGING_DIR_HOST)/bin/mkrasimage \ > + -b $(RAS_BOARD) \ > + -v $(RAS_VERSION) \ > + -k $(call param_get_default,kernel,$(1),$(IMAGE_KERNEL)) \ > + -r $@ \ > + -s $$newsize \ > + -o $@.new > + @mv $@.new $@ > +endef > + > define Build/mkbuffaloimg > $(STAGING_DIR_HOST)/bin/mkbuffaloimg -B $(BOARDNAME) \ > -R $$(($(subst k, * 1024,$(ROOTFS_SIZE)))) \ > diff --git a/target/linux/ipq40xx/image/Makefile > b/target/linux/ipq40xx/image/Makefile index > d1ee1004fd..6e4125db0b 100644 > --- a/target/linux/ipq40xx/image/Makefile > +++ b/target/linux/ipq40xx/image/Makefile > @@ -221,7 +221,7 @@ define Device/zyxel_nbg6617 > # at least as large as the one of the initial firmware image (not the current > # one on the device). This only applies to the Web-UI, the bootlaoder ignores > # this minimum-size. However, the larger image can be flashed both ways. > - IMAGE/factory.bin := append-rootfs | pad-rootfs | check-size $$$$(ROOTFS_SIZE) | make-ras > + IMAGE/factory.bin := append-rootfs | pad-rootfs | check-size $$$$(ROOTFS_SIZE) | zyxel-ras-image > IMAGE/sysupgrade.bin/squashfs := append-rootfs | pad-rootfs | check-size $$$$(ROOTFS_SIZE) | sysupgrade-tar rootfs=$$$$@ | append-metadata > DEVICE_PACKAGES := ipq-wifi-zyxel_nbg6617 uboot-envtools > endef > diff --git a/tools/firmware-utils/Makefile > b/tools/firmware-utils/Makefile index 436a43794c..00917c3417 > 100644 > --- a/tools/firmware-utils/Makefile > +++ b/tools/firmware-utils/Makefile > @@ -70,6 +70,7 @@ define Host/Compile > $(call cc,fix-u-media-header cyg_crc32,-Wall) > $(call cc,hcsmakeimage bcmalgo) > $(call cc,mkporayfw, -Wall) > + $(call cc,mkrasimage, --std=gnu99) > $(call cc,mkhilinkfw, -lcrypto) > $(call cc,mkdcs932, -Wall) > $(call cc,mkheader_gemtek,-lz) > diff --git a/tools/firmware-utils/src/mkrasimage.c > b/tools/firmware-utils/src/mkrasimage.c new file mode 100644 > index 0000000000..1cac7b5da9 > --- /dev/null > +++ b/tools/firmware-utils/src/mkrasimage.c > @@ -0,0 +1,374 @@ > +/* > + * --- ZyXEL header format --- > + * Original Version by Benjamin Berg <benjamin@sipsolutions.net> > + * C implementation based on generation-script by Christian Lamparter <chunkeey@gmail.com> > + * > + * The firmware image prefixed with a header (which is written into the MTD device). > + * The header is one erase block (~64KiB) in size, but the checksum only convers the > + * first 2KiB. Padding is 0xff. All integers are in big-endian. > + * > + * The checksum is always a 16-Bit System V checksum (sum -s) stored in a 32-Bit integer. > + * > + * 4 bytes: checksum of the rootfs image > + * 4 bytes: length of the contained rootfs image file (big endian) > + * 32 bytes: Firmware Version string (NUL terminated, 0xff padded) > + * 4 bytes: checksum over the header partition (big endian - see below) > + * 64 bytes: Model (e.g. "NBG6617", NUL termiated, 0xff padded) > + * 4 bytes: checksum of the kernel partition > + * 4 bytes: length of the contained kernel image file (big endian) > + * rest: 0xff padding (To erase block size) > + * > + * The checksums are calculated by adding up all bytes and if a 16bit > + * overflow occurs, one is added and the sum is masked to 16 bit: > + * csum = csum + databyte; if (csum > 0xffff) { csum += 1; csum &= 0xffff }; > + * Should the file have an odd number of bytes then the byte len-0x800 is > + * used additionally. > + * > + * The checksum for the header is calculated over the first 2048 bytes with > + * the rootfs image checksum as the placeholder during calculation. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published > + * by the Free Software Foundation. > + * > + */ > +#include <byteswap.h> > +#include <getopt.h> > +#include <libgen.h> > +#include <stdio.h> > +#include <string.h> > +#include <stdlib.h> > + > +#define VERSION_STRING_LEN 31 > +#define ROOTFS_HEADER_LEN 40 > + > +#define KERNEL_HEADER_LEN 8 > + > +#define BOARD_NAME_LEN 64 > +#define BOARD_HEADER_LEN 68 > + > +#define HEADER_PARTITION_CALC_LENGTH 2048 > +#define HEADER_PARTITION_LENGTH 0x10000 > + > +struct file_info { > + char *name; /* name of the file */ > + char *data; /* file content */ > + size_t size; /* length of the file */ > +}; > + > +static char *progname; > + > +static char *board_name = 0; > +static char *version_name = 0; > +static unsigned int rootfs_size = 0; > + > +static struct file_info kernel = {0, 0, 0}; > +static struct file_info rootfs = {0, 0, 0}; > +static struct file_info rootfs_out = {0, 0, 0}; > +static struct file_info out = {0, 0, 0}; > + > +#define ERR(fmt, ...) do { \ > + fflush(0); \ > + fprintf(stderr, "[%s] *** error: " fmt "\n", \ > + progname, ## __VA_ARGS__ ); \ > +} while (0) > + > +void bufferFile(struct file_info *finfo) { > + unsigned int fs = 0; > + FILE *f = NULL; > + > + f = fopen(finfo->name, "rb"); > + if (f == NULL) { > + printf("Error while opening file %s.", finfo->name); > + exit(EXIT_FAILURE); > + } > + > + fseek(f, 0L, SEEK_END); > + fs = (unsigned int) ftell(f); > + rewind(f); > + > + finfo->size = fs; > + > + char *data = malloc(fs); > + finfo->data = data; > + size_t read = fread(data, fs, 1, f); > + > + if (read != 1) { > + printf("Error reading file %s.", finfo->name); > + exit(EXIT_FAILURE); > + } > + > + fclose(f); > +} > + > +void writeFile(struct file_info *finfo) { > + FILE *fout = fopen(finfo->name, "w"); > + > + if (!fwrite(finfo->data, finfo->size, 1, fout)) { > + printf("Wanted to write, but something went wrong.\n"); > + exit(1); > + } > +} > + > +void usage(int status) { > + FILE *stream = (status != EXIT_SUCCESS) ? stderr : stdout; > + > + fprintf(stream, "Usage: %s [OPTIONS...]\n", progname); > + fprintf(stream, > + "\n" > + "Options:\n" > + " -k <kernel> path for kernel image\n" > + " -r <rootfs> path for rootfs image\n" > + " -s <rfssize> size of output rootfs\n" > + " -v <version> version string\n" > + " -b <boardname> name of board to generate image for\n" > + " -h show this screen\n" > + ); > + > + exit(status); > +} > + > +static int sysv_chksm(const unsigned char *data, int size) { > + int r; > + int checksum; > + unsigned int s = 0; /* The sum of all the input bytes, modulo (UINT_MAX + 1). */ > + > + > + for (int i = 0; i < size; i++) { > + s += data[i]; > + } > + > + r = (s & 0xffff) + ((s & 0xffffffff) >> 16); > + checksum = (r & 0xffff) + (r >> 16); > + > + return checksum; > +} > + > +char *generate_rootfs_header(struct file_info rootfs, char > *version) { > + size_t version_string_length; > + unsigned int chksm, size; > + char *rootfs_header; > + size_t ptr = 0; > + > + rootfs_header = malloc(ROOTFS_HEADER_LEN); > + memset(rootfs_header, 0xff, ROOTFS_HEADER_LEN); > + > + memcpy(rootfs_out.data, rootfs.data, rootfs.size); > + > + chksm = __bswap_32(sysv_chksm(rootfs_out.data, rootfs_out.size)); > + size = __bswap_32(rootfs_out.size); > + > + memcpy(rootfs_header + ptr, &chksm, 4); > + ptr += 4; > + > + memcpy(rootfs_header + ptr, &size, 4); > + ptr += 4; > + > + version_string_length = strlen(version) <= VERSION_STRING_LEN ? strlen(version) : VERSION_STRING_LEN; > + > + memcpy(rootfs_header + ptr, version, version_string_length); > + ptr += version_string_length; > + > + rootfs_header[ptr] = 0x0; > + > + return rootfs_header; > +} > + > +char *generate_kernel_header(struct file_info kernel) { > + unsigned int chksm, size; > + char *kernel_header; > + size_t ptr = 0; > + > + kernel_header = malloc(KERNEL_HEADER_LEN); > + memset(kernel_header, 0xff, KERNEL_HEADER_LEN); > + > + chksm = __bswap_32(sysv_chksm(kernel.data, kernel.size)); > + size = __bswap_32(kernel.size); > + > + memcpy(kernel_header + ptr, &chksm, 4); > + ptr += 4; > + > + memcpy(kernel_header + ptr, &size, 4); > + > + return kernel_header; > +} > + > +unsigned int generate_board_header_checksum(char *kernel_hdr, > char *rootfs_hdr, char *boardname) { > + char *board_hdr_tmp; > + size_t ptr = 0; > + > + board_hdr_tmp = malloc(HEADER_PARTITION_CALC_LENGTH); > + memset(board_hdr_tmp, 0xff, HEADER_PARTITION_CALC_LENGTH); > + > + memcpy(board_hdr_tmp, rootfs_hdr, ROOTFS_HEADER_LEN); > + ptr += ROOTFS_HEADER_LEN; > + > + memcpy(board_hdr_tmp + ptr, rootfs_hdr, 4); /* Use RootFS checksum as placeholder */ > + ptr += 4; > + > + memcpy(board_hdr_tmp + ptr, boardname, strlen(boardname)); /* Append boardname */ > + ptr += strlen(boardname); > + > + board_hdr_tmp[ptr] = 0x0; /* Add null-terminator */ > + ptr = ROOTFS_HEADER_LEN + 4 + BOARD_NAME_LEN; > + > + memcpy(board_hdr_tmp + ptr, kernel_hdr, 8); > + > + return __bswap_32(sysv_chksm(board_hdr_tmp, 2048)); > +} > + > +char *generate_board_header(char *kernel_hdr, char > *rootfs_hdr, char *boardname) { > + unsigned int board_checksum; > + char *board_hdr; > + > + board_hdr = malloc(BOARD_HEADER_LEN); > + memset(board_hdr, 0xff, BOARD_HEADER_LEN); > + > + board_checksum = generate_board_header_checksum(kernel_hdr, rootfs_hdr, boardname); > + > + memcpy(board_hdr, &board_checksum, 4); > + memcpy(board_hdr + 4, boardname, strlen(boardname)); > + board_hdr[4 + strlen(boardname)] = 0x0; > + > + return board_hdr; > +} > + > +int build_image() { > + char *rootfs_header, *kernel_header, *board_header; > + > + size_t ptr; > + > + /* Load files */ > + bufferFile(&kernel); > + bufferFile(&rootfs); > + > + /* Allocate memory for temporary ouput rootfs */ > + rootfs_out.data = malloc(rootfs_out.size); > + memset(rootfs_out.data, 0x00, rootfs_out.size); > + > + /* Prepare headers */ > + rootfs_header = generate_rootfs_header(rootfs, version_name); > + kernel_header = generate_kernel_header(kernel); > + board_header = generate_board_header(kernel_header, rootfs_header, board_name); > + > + /* Prepare output file */ > + out.size = HEADER_PARTITION_LENGTH + rootfs_out.size + kernel.size; > + out.data = malloc(out.size); > + memset(out.data, 0xFF, out.size); > + > + /* Build output image */ > + memcpy(out.data, rootfs_header, ROOTFS_HEADER_LEN); > + memcpy(out.data + ROOTFS_HEADER_LEN, board_header, BOARD_HEADER_LEN); > + memcpy(out.data + ROOTFS_HEADER_LEN + BOARD_HEADER_LEN, kernel_header, KERNEL_HEADER_LEN); > + ptr = HEADER_PARTITION_LENGTH; > + memcpy(out.data + ptr, rootfs_out.data, rootfs_out.size); > + ptr += rootfs_out.size; > + memcpy(out.data + ptr, kernel.data, kernel.size); > + > + /* Write back output image */ > + writeFile(&out); > + > + /* Free allocated memory */ > + free(kernel.data); > + free(rootfs.data); > + free(out.data); > + free(rootfs_out.data); > + > + free(rootfs_header); > + free(kernel_header); > + free(board_header); > + > + return 0; > +} > + > +int check_options() { > + if (kernel.name == 0) { > + ERR("No kernel filename supplied"); > + return -1; > + } > + > + if (rootfs.name == 0) { > + ERR("No rootfs filename supplied"); > + return -2; > + } > + > + if (out.name == 0) { > + ERR("No output filename supplied"); > + return -3; > + } > + > + if (board_name == 0) { > + ERR("No board-name supplied"); > + return -4; > + } > + > + if (version_name == 0) { > + ERR("No version supplied"); > + return -5; > + } > + > + if (rootfs_size <= 0) { > + ERR("Invalid rootfs size supplied"); > + return -6; > + } > + > + if (strlen(board_name) > 31) { > + ERR("Board name is to long"); > + return -7; > + } > + return 0; > +} > + > +int main(int argc, char *argv[]) { > + int ret; > + progname = basename(argv[0]); > + while (1) { > + int c; > + > + c = getopt(argc, argv, "b:k:o:r:s:v:h"); > + if (c == -1) > + break; > + > + switch (c) { > + case 'b': > + board_name = optarg; > + break; > + case 'h': > + usage(EXIT_SUCCESS); > + break; > + case 'k': > + kernel.name = optarg; > + break; > + case 'o': > + out.name = optarg; > + break; > + case 'r': > + rootfs.name = optarg; > + break; > + case 's': > + sscanf(optarg, "%u", &rootfs_size); > + break; > + case 'v': > + version_name = optarg; > + break; > + default: > + usage(EXIT_FAILURE); > + break; > + } > + } > + > + ret = check_options(); > + if (ret) > + usage(EXIT_FAILURE); > + > + /* As ZyXEL Web-GUI only accept images with a rootfs equal or larger than the first firmware shipped > + * for the device, we need to pad rootfs partition to this size. To perform further calculations, we > + * decide the size of this part here. In case the rootfs we want to integrate in our image is larger, > + * take it's size, otherwise the supplied size. > + * > + * Be careful! We rely on assertion of correct size to be performed beforehand. It is unknown if images > + * with a to large rootfs are accepted or not. > + */ > + rootfs_out.size = rootfs_size < rootfs.size ? rootfs.size : rootfs_size; > + return build_image(); > +}
Hello Karl, On 8/16/18 3:12 AM, Karl Palsson wrote: > 1) how bad are the portability issues that you felt > reimplementing in _C_ was the best path? 2) if it's that bad, why > keep it? How will future people knwo what to use. Either get rid > of it, or fix it. Regarding 1) The portability issue seems to be affecting older bash versions. Christian described and also fixed the issue in the forums [1]. I'm not sure about what systems correctly are affected (definitely older MacOS X and the OS of the buildbots). The C implementation originated from when i worked on the device before finding Christians tree. Honestly i also think the readability of the current script is not really that great but honestly i don't think we can substantially improve that. Regarding 2) I see that point, removing it is probably the better way. [1] https://forum.openwrt.org/t/solved-zyxel-nbg6617-tftp-flash-wont-take/17731/10 Greetings David > Cheers, > Karl P > >> >> Signed-off-by: David Bauer <mail@david-bauer.net> >> --- >> include/image-commands.mk | 13 + >> target/linux/ipq40xx/image/Makefile | 2 +- >> tools/firmware-utils/Makefile | 1 + >> tools/firmware-utils/src/mkrasimage.c | 374 ++++++++++++++++++++++++++ >> 4 files changed, 389 insertions(+), 1 deletion(-) >> create mode 100644 tools/firmware-utils/src/mkrasimage.c >> >> diff --git a/include/image-commands.mk >> b/include/image-commands.mk index 3cc5dc21e1..bb5fe46e1a 100644 >> --- a/include/image-commands.mk >> +++ b/include/image-commands.mk >> @@ -62,6 +62,19 @@ define Build/make-ras >> @mv $@.new $@ >> endef >> >> +define Build/zyxel-ras-image >> + let \ >> + newsize="$(subst k,* 1024,$(RAS_ROOTFS_SIZE))"; \ >> + $(STAGING_DIR_HOST)/bin/mkrasimage \ >> + -b $(RAS_BOARD) \ >> + -v $(RAS_VERSION) \ >> + -k $(call param_get_default,kernel,$(1),$(IMAGE_KERNEL)) \ >> + -r $@ \ >> + -s $$newsize \ >> + -o $@.new >> + @mv $@.new $@ >> +endef >> + >> define Build/mkbuffaloimg >> $(STAGING_DIR_HOST)/bin/mkbuffaloimg -B $(BOARDNAME) \ >> -R $$(($(subst k, * 1024,$(ROOTFS_SIZE)))) \ >> diff --git a/target/linux/ipq40xx/image/Makefile >> b/target/linux/ipq40xx/image/Makefile index >> d1ee1004fd..6e4125db0b 100644 >> --- a/target/linux/ipq40xx/image/Makefile >> +++ b/target/linux/ipq40xx/image/Makefile >> @@ -221,7 +221,7 @@ define Device/zyxel_nbg6617 >> # at least as large as the one of the initial firmware image (not the current >> # one on the device). This only applies to the Web-UI, the bootlaoder ignores >> # this minimum-size. However, the larger image can be flashed both ways. >> - IMAGE/factory.bin := append-rootfs | pad-rootfs | check-size $$$$(ROOTFS_SIZE) | make-ras >> + IMAGE/factory.bin := append-rootfs | pad-rootfs | check-size $$$$(ROOTFS_SIZE) | zyxel-ras-image >> IMAGE/sysupgrade.bin/squashfs := append-rootfs | pad-rootfs | check-size $$$$(ROOTFS_SIZE) | sysupgrade-tar rootfs=$$$$@ | append-metadata >> DEVICE_PACKAGES := ipq-wifi-zyxel_nbg6617 uboot-envtools >> endef >> diff --git a/tools/firmware-utils/Makefile >> b/tools/firmware-utils/Makefile index 436a43794c..00917c3417 >> 100644 >> --- a/tools/firmware-utils/Makefile >> +++ b/tools/firmware-utils/Makefile >> @@ -70,6 +70,7 @@ define Host/Compile >> $(call cc,fix-u-media-header cyg_crc32,-Wall) >> $(call cc,hcsmakeimage bcmalgo) >> $(call cc,mkporayfw, -Wall) >> + $(call cc,mkrasimage, --std=gnu99) >> $(call cc,mkhilinkfw, -lcrypto) >> $(call cc,mkdcs932, -Wall) >> $(call cc,mkheader_gemtek,-lz) >> diff --git a/tools/firmware-utils/src/mkrasimage.c >> b/tools/firmware-utils/src/mkrasimage.c new file mode 100644 >> index 0000000000..1cac7b5da9 >> --- /dev/null >> +++ b/tools/firmware-utils/src/mkrasimage.c >> @@ -0,0 +1,374 @@ >> +/* >> + * --- ZyXEL header format --- >> + * Original Version by Benjamin Berg <benjamin@sipsolutions.net> >> + * C implementation based on generation-script by Christian Lamparter <chunkeey@gmail.com> >> + * >> + * The firmware image prefixed with a header (which is written into the MTD device). >> + * The header is one erase block (~64KiB) in size, but the checksum only convers the >> + * first 2KiB. Padding is 0xff. All integers are in big-endian. >> + * >> + * The checksum is always a 16-Bit System V checksum (sum -s) stored in a 32-Bit integer. >> + * >> + * 4 bytes: checksum of the rootfs image >> + * 4 bytes: length of the contained rootfs image file (big endian) >> + * 32 bytes: Firmware Version string (NUL terminated, 0xff padded) >> + * 4 bytes: checksum over the header partition (big endian - see below) >> + * 64 bytes: Model (e.g. "NBG6617", NUL termiated, 0xff padded) >> + * 4 bytes: checksum of the kernel partition >> + * 4 bytes: length of the contained kernel image file (big endian) >> + * rest: 0xff padding (To erase block size) >> + * >> + * The checksums are calculated by adding up all bytes and if a 16bit >> + * overflow occurs, one is added and the sum is masked to 16 bit: >> + * csum = csum + databyte; if (csum > 0xffff) { csum += 1; csum &= 0xffff }; >> + * Should the file have an odd number of bytes then the byte len-0x800 is >> + * used additionally. >> + * >> + * The checksum for the header is calculated over the first 2048 bytes with >> + * the rootfs image checksum as the placeholder during calculation. >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU General Public License version 2 as published >> + * by the Free Software Foundation. >> + * >> + */ >> +#include <byteswap.h> >> +#include <getopt.h> >> +#include <libgen.h> >> +#include <stdio.h> >> +#include <string.h> >> +#include <stdlib.h> >> + >> +#define VERSION_STRING_LEN 31 >> +#define ROOTFS_HEADER_LEN 40 >> + >> +#define KERNEL_HEADER_LEN 8 >> + >> +#define BOARD_NAME_LEN 64 >> +#define BOARD_HEADER_LEN 68 >> + >> +#define HEADER_PARTITION_CALC_LENGTH 2048 >> +#define HEADER_PARTITION_LENGTH 0x10000 >> + >> +struct file_info { >> + char *name; /* name of the file */ >> + char *data; /* file content */ >> + size_t size; /* length of the file */ >> +}; >> + >> +static char *progname; >> + >> +static char *board_name = 0; >> +static char *version_name = 0; >> +static unsigned int rootfs_size = 0; >> + >> +static struct file_info kernel = {0, 0, 0}; >> +static struct file_info rootfs = {0, 0, 0}; >> +static struct file_info rootfs_out = {0, 0, 0}; >> +static struct file_info out = {0, 0, 0}; >> + >> +#define ERR(fmt, ...) do { \ >> + fflush(0); \ >> + fprintf(stderr, "[%s] *** error: " fmt "\n", \ >> + progname, ## __VA_ARGS__ ); \ >> +} while (0) >> + >> +void bufferFile(struct file_info *finfo) { >> + unsigned int fs = 0; >> + FILE *f = NULL; >> + >> + f = fopen(finfo->name, "rb"); >> + if (f == NULL) { >> + printf("Error while opening file %s.", finfo->name); >> + exit(EXIT_FAILURE); >> + } >> + >> + fseek(f, 0L, SEEK_END); >> + fs = (unsigned int) ftell(f); >> + rewind(f); >> + >> + finfo->size = fs; >> + >> + char *data = malloc(fs); >> + finfo->data = data; >> + size_t read = fread(data, fs, 1, f); >> + >> + if (read != 1) { >> + printf("Error reading file %s.", finfo->name); >> + exit(EXIT_FAILURE); >> + } >> + >> + fclose(f); >> +} >> + >> +void writeFile(struct file_info *finfo) { >> + FILE *fout = fopen(finfo->name, "w"); >> + >> + if (!fwrite(finfo->data, finfo->size, 1, fout)) { >> + printf("Wanted to write, but something went wrong.\n"); >> + exit(1); >> + } >> +} >> + >> +void usage(int status) { >> + FILE *stream = (status != EXIT_SUCCESS) ? stderr : stdout; >> + >> + fprintf(stream, "Usage: %s [OPTIONS...]\n", progname); >> + fprintf(stream, >> + "\n" >> + "Options:\n" >> + " -k <kernel> path for kernel image\n" >> + " -r <rootfs> path for rootfs image\n" >> + " -s <rfssize> size of output rootfs\n" >> + " -v <version> version string\n" >> + " -b <boardname> name of board to generate image for\n" >> + " -h show this screen\n" >> + ); >> + >> + exit(status); >> +} >> + >> +static int sysv_chksm(const unsigned char *data, int size) { >> + int r; >> + int checksum; >> + unsigned int s = 0; /* The sum of all the input bytes, modulo (UINT_MAX + 1). */ >> + >> + >> + for (int i = 0; i < size; i++) { >> + s += data[i]; >> + } >> + >> + r = (s & 0xffff) + ((s & 0xffffffff) >> 16); >> + checksum = (r & 0xffff) + (r >> 16); >> + >> + return checksum; >> +} >> + >> +char *generate_rootfs_header(struct file_info rootfs, char >> *version) { >> + size_t version_string_length; >> + unsigned int chksm, size; >> + char *rootfs_header; >> + size_t ptr = 0; >> + >> + rootfs_header = malloc(ROOTFS_HEADER_LEN); >> + memset(rootfs_header, 0xff, ROOTFS_HEADER_LEN); >> + >> + memcpy(rootfs_out.data, rootfs.data, rootfs.size); >> + >> + chksm = __bswap_32(sysv_chksm(rootfs_out.data, rootfs_out.size)); >> + size = __bswap_32(rootfs_out.size); >> + >> + memcpy(rootfs_header + ptr, &chksm, 4); >> + ptr += 4; >> + >> + memcpy(rootfs_header + ptr, &size, 4); >> + ptr += 4; >> + >> + version_string_length = strlen(version) <= VERSION_STRING_LEN ? strlen(version) : VERSION_STRING_LEN; >> + >> + memcpy(rootfs_header + ptr, version, version_string_length); >> + ptr += version_string_length; >> + >> + rootfs_header[ptr] = 0x0; >> + >> + return rootfs_header; >> +} >> + >> +char *generate_kernel_header(struct file_info kernel) { >> + unsigned int chksm, size; >> + char *kernel_header; >> + size_t ptr = 0; >> + >> + kernel_header = malloc(KERNEL_HEADER_LEN); >> + memset(kernel_header, 0xff, KERNEL_HEADER_LEN); >> + >> + chksm = __bswap_32(sysv_chksm(kernel.data, kernel.size)); >> + size = __bswap_32(kernel.size); >> + >> + memcpy(kernel_header + ptr, &chksm, 4); >> + ptr += 4; >> + >> + memcpy(kernel_header + ptr, &size, 4); >> + >> + return kernel_header; >> +} >> + >> +unsigned int generate_board_header_checksum(char *kernel_hdr, >> char *rootfs_hdr, char *boardname) { >> + char *board_hdr_tmp; >> + size_t ptr = 0; >> + >> + board_hdr_tmp = malloc(HEADER_PARTITION_CALC_LENGTH); >> + memset(board_hdr_tmp, 0xff, HEADER_PARTITION_CALC_LENGTH); >> + >> + memcpy(board_hdr_tmp, rootfs_hdr, ROOTFS_HEADER_LEN); >> + ptr += ROOTFS_HEADER_LEN; >> + >> + memcpy(board_hdr_tmp + ptr, rootfs_hdr, 4); /* Use RootFS checksum as placeholder */ >> + ptr += 4; >> + >> + memcpy(board_hdr_tmp + ptr, boardname, strlen(boardname)); /* Append boardname */ >> + ptr += strlen(boardname); >> + >> + board_hdr_tmp[ptr] = 0x0; /* Add null-terminator */ >> + ptr = ROOTFS_HEADER_LEN + 4 + BOARD_NAME_LEN; >> + >> + memcpy(board_hdr_tmp + ptr, kernel_hdr, 8); >> + >> + return __bswap_32(sysv_chksm(board_hdr_tmp, 2048)); >> +} >> + >> +char *generate_board_header(char *kernel_hdr, char >> *rootfs_hdr, char *boardname) { >> + unsigned int board_checksum; >> + char *board_hdr; >> + >> + board_hdr = malloc(BOARD_HEADER_LEN); >> + memset(board_hdr, 0xff, BOARD_HEADER_LEN); >> + >> + board_checksum = generate_board_header_checksum(kernel_hdr, rootfs_hdr, boardname); >> + >> + memcpy(board_hdr, &board_checksum, 4); >> + memcpy(board_hdr + 4, boardname, strlen(boardname)); >> + board_hdr[4 + strlen(boardname)] = 0x0; >> + >> + return board_hdr; >> +} >> + >> +int build_image() { >> + char *rootfs_header, *kernel_header, *board_header; >> + >> + size_t ptr; >> + >> + /* Load files */ >> + bufferFile(&kernel); >> + bufferFile(&rootfs); >> + >> + /* Allocate memory for temporary ouput rootfs */ >> + rootfs_out.data = malloc(rootfs_out.size); >> + memset(rootfs_out.data, 0x00, rootfs_out.size); >> + >> + /* Prepare headers */ >> + rootfs_header = generate_rootfs_header(rootfs, version_name); >> + kernel_header = generate_kernel_header(kernel); >> + board_header = generate_board_header(kernel_header, rootfs_header, board_name); >> + >> + /* Prepare output file */ >> + out.size = HEADER_PARTITION_LENGTH + rootfs_out.size + kernel.size; >> + out.data = malloc(out.size); >> + memset(out.data, 0xFF, out.size); >> + >> + /* Build output image */ >> + memcpy(out.data, rootfs_header, ROOTFS_HEADER_LEN); >> + memcpy(out.data + ROOTFS_HEADER_LEN, board_header, BOARD_HEADER_LEN); >> + memcpy(out.data + ROOTFS_HEADER_LEN + BOARD_HEADER_LEN, kernel_header, KERNEL_HEADER_LEN); >> + ptr = HEADER_PARTITION_LENGTH; >> + memcpy(out.data + ptr, rootfs_out.data, rootfs_out.size); >> + ptr += rootfs_out.size; >> + memcpy(out.data + ptr, kernel.data, kernel.size); >> + >> + /* Write back output image */ >> + writeFile(&out); >> + >> + /* Free allocated memory */ >> + free(kernel.data); >> + free(rootfs.data); >> + free(out.data); >> + free(rootfs_out.data); >> + >> + free(rootfs_header); >> + free(kernel_header); >> + free(board_header); >> + >> + return 0; >> +} >> + >> +int check_options() { >> + if (kernel.name == 0) { >> + ERR("No kernel filename supplied"); >> + return -1; >> + } >> + >> + if (rootfs.name == 0) { >> + ERR("No rootfs filename supplied"); >> + return -2; >> + } >> + >> + if (out.name == 0) { >> + ERR("No output filename supplied"); >> + return -3; >> + } >> + >> + if (board_name == 0) { >> + ERR("No board-name supplied"); >> + return -4; >> + } >> + >> + if (version_name == 0) { >> + ERR("No version supplied"); >> + return -5; >> + } >> + >> + if (rootfs_size <= 0) { >> + ERR("Invalid rootfs size supplied"); >> + return -6; >> + } >> + >> + if (strlen(board_name) > 31) { >> + ERR("Board name is to long"); >> + return -7; >> + } >> + return 0; >> +} >> + >> +int main(int argc, char *argv[]) { >> + int ret; >> + progname = basename(argv[0]); >> + while (1) { >> + int c; >> + >> + c = getopt(argc, argv, "b:k:o:r:s:v:h"); >> + if (c == -1) >> + break; >> + >> + switch (c) { >> + case 'b': >> + board_name = optarg; >> + break; >> + case 'h': >> + usage(EXIT_SUCCESS); >> + break; >> + case 'k': >> + kernel.name = optarg; >> + break; >> + case 'o': >> + out.name = optarg; >> + break; >> + case 'r': >> + rootfs.name = optarg; >> + break; >> + case 's': >> + sscanf(optarg, "%u", &rootfs_size); >> + break; >> + case 'v': >> + version_name = optarg; >> + break; >> + default: >> + usage(EXIT_FAILURE); >> + break; >> + } >> + } >> + >> + ret = check_options(); >> + if (ret) >> + usage(EXIT_FAILURE); >> + >> + /* As ZyXEL Web-GUI only accept images with a rootfs equal or larger than the first firmware shipped >> + * for the device, we need to pad rootfs partition to this size. To perform further calculations, we >> + * decide the size of this part here. In case the rootfs we want to integrate in our image is larger, >> + * take it's size, otherwise the supplied size. >> + * >> + * Be careful! We rely on assertion of correct size to be performed beforehand. It is unknown if images >> + * with a to large rootfs are accepted or not. >> + */ >> + rootfs_out.size = rootfs_size < rootfs.size ? rootfs.size : rootfs_size; >> + return build_image(); >> +} >> >> _______________________________________________ >> openwrt-devel mailing list >> openwrt-devel@lists.openwrt.org >> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On Thursday, August 16, 2018 12:31:38 PM CEST David Bauer wrote: > On 8/16/18 3:12 AM, Karl Palsson wrote: > > 1) how bad are the portability issues that you felt > > reimplementing in _C_ was the best path? 2) if it's that bad, why > > keep it? How will future people knwo what to use. Either get rid > > of it, or fix it. > > Regarding 1) > The portability issue seems to be affecting older bash versions. > Christian described and also fixed the issue in the forums [1]. I'm not > sure about what systems correctly are affected (definitely older MacOS X > and the OS of the buildbots). Oh, that "MacOS X" there was meant that I used a iBook G4 (BigEndian PowerPC) with MacOS X 10.5.8 to test and fix the portability of the make-ras.sh script. I also tested it on the ZyXEL NBG6617 itself running the busybox sh: If someone wants to look, here's the program from back in the day. <https://github.com/chunkeey/LEDE-IPQ40XX/blob/staging/target/linux/ipq40xx/image/make-ras.sh> > The C implementation originated from when i worked on the device before > finding Christians tree. Honestly i also think the readability of the > current script is not really that great but honestly i don't think we > can substantially improve that. I wrote the bash version because of issues in the past with C implementations of firmware packers for the Meraki routers (mkmerakifw.c and mkmerakifw-old.c). The issue there was that the main dev was coming from a sysadmin background and as with many OpenWrt users he had problems with C (endiannes!). As for what implementation is better. Why not let Karl Palsson (or any other commentor that wants to join ;) ) decide? The "magic" that needs to be done to create the ras.bin images is as follow: 1. They start with an 2KiB header (later aligned to 64KiB) that starts with # 4 bytes: checksum of the rootfs image (System V checksum - big endian) # 4 bytes: length of the contained rootfs image file (big endian) # 32 bytes: Firmware Version string (NUL terminated, 0xff padded) # 4 bytes: checksum over the header partition (System V checksum - big e.) # 32 bytes: Model (e.g. "NBG6617", NUL termiated, 0xff padded) # 4 bytes: checksum of the kernel partition (System V checksum - big e.) # 4 bytes: length of the contained kernel image file (big endian) # rest: 0xff padding # # The checksum for the header is calculated over the first 2048 bytes with # the rootfs image checksum as the placeholder during calculation. 2. followed by the rootfs image 3. followed by the kernel image 4. Done (pack it all in one big file) Note: The NBG6617 isn't the only device from ZyXEL that utilize the ras image. In fact, the "original" version of the notes above was written by Benjamin Berg and can still be found as a rather long comment in the NBG6616 image generation code under /target/linux/ar71xx/image/generic.mk [0]. (And there is at least one more device that can make use of ras: the NBG6817). So chances are, there that whatever "wins" will need to be updated to support those routers as well. Regards, Christian [0] <https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ar71xx/image/generic.mk;h=3b320814c5ab091c8c8ac8ff91c40e18df446f44;hb=HEAD#l1069>
Hi Christian, > I wrote the bash version because of issues in the past with C implementations > of firmware packers for the Meraki routers (mkmerakifw.c and mkmerakifw-old.c). > The issue there was that the main dev was coming from a sysadmin background > and as with many OpenWrt users he had problems with C (endiannes!). > > As for what implementation is better. Why not let Karl Palsson (or any > other commentor that wants to join ;) ) decide? I didn't want to start a race here, I was just more comfortable with finishing my C implementation as i have a problem with understanding such shell-scripts. From my understanding your linked script fixed the issues? I'm not sure why the script in OpenWRT is that old. IIRC i took it directly from your tree without further modifications. > Note: The NBG6617 isn't the only device from ZyXEL that utilize the ras image. > In fact, the "original" version of the notes above was written by Benjamin Berg > and can still be found as a rather long comment in the NBG6616 image generation > code under /target/linux/ar71xx/image/generic.mk [0]. (And there is at least > one more device that can make use of ras: the NBG6817). > > So chances are, there that whatever "wins" will need to be updated to support > those routers as well. Good point (didn't thought about that). At first glance the NBG6817 looks identical while the NBG6616 is missing the kernel-header part. I will see how i can support those two devices. There is also the NBG6716 around, but his one is using legacy build-code. So for this patch i see it as out-of-scope (although the header looks identical to the NBG6616). Best wishes David
Hi On 2018-08-17, Christian Lamparter wrote: > On Thursday, August 16, 2018 12:31:38 PM CEST David Bauer wrote: > > On 8/16/18 3:12 AM, Karl Palsson wrote: [...] > [...] (And there is at least > one more device that can make use of ras: the NBG6817). Adding support for a factory image for the NBG6817 by using make-ras is on my radar, but as that router is in use as my main router with a rather complex vlan and 4addr setup, it might take a while until I find an opportunity to look into it. Regards Stefan Lippers-Hollmann
diff --git a/include/image-commands.mk b/include/image-commands.mk index 3cc5dc21e1..bb5fe46e1a 100644 --- a/include/image-commands.mk +++ b/include/image-commands.mk @@ -62,6 +62,19 @@ define Build/make-ras @mv $@.new $@ endef +define Build/zyxel-ras-image + let \ + newsize="$(subst k,* 1024,$(RAS_ROOTFS_SIZE))"; \ + $(STAGING_DIR_HOST)/bin/mkrasimage \ + -b $(RAS_BOARD) \ + -v $(RAS_VERSION) \ + -k $(call param_get_default,kernel,$(1),$(IMAGE_KERNEL)) \ + -r $@ \ + -s $$newsize \ + -o $@.new + @mv $@.new $@ +endef + define Build/mkbuffaloimg $(STAGING_DIR_HOST)/bin/mkbuffaloimg -B $(BOARDNAME) \ -R $$(($(subst k, * 1024,$(ROOTFS_SIZE)))) \ diff --git a/target/linux/ipq40xx/image/Makefile b/target/linux/ipq40xx/image/Makefile index d1ee1004fd..6e4125db0b 100644 --- a/target/linux/ipq40xx/image/Makefile +++ b/target/linux/ipq40xx/image/Makefile @@ -221,7 +221,7 @@ define Device/zyxel_nbg6617 # at least as large as the one of the initial firmware image (not the current # one on the device). This only applies to the Web-UI, the bootlaoder ignores # this minimum-size. However, the larger image can be flashed both ways. - IMAGE/factory.bin := append-rootfs | pad-rootfs | check-size $$$$(ROOTFS_SIZE) | make-ras + IMAGE/factory.bin := append-rootfs | pad-rootfs | check-size $$$$(ROOTFS_SIZE) | zyxel-ras-image IMAGE/sysupgrade.bin/squashfs := append-rootfs | pad-rootfs | check-size $$$$(ROOTFS_SIZE) | sysupgrade-tar rootfs=$$$$@ | append-metadata DEVICE_PACKAGES := ipq-wifi-zyxel_nbg6617 uboot-envtools endef diff --git a/tools/firmware-utils/Makefile b/tools/firmware-utils/Makefile index 436a43794c..00917c3417 100644 --- a/tools/firmware-utils/Makefile +++ b/tools/firmware-utils/Makefile @@ -70,6 +70,7 @@ define Host/Compile $(call cc,fix-u-media-header cyg_crc32,-Wall) $(call cc,hcsmakeimage bcmalgo) $(call cc,mkporayfw, -Wall) + $(call cc,mkrasimage, --std=gnu99) $(call cc,mkhilinkfw, -lcrypto) $(call cc,mkdcs932, -Wall) $(call cc,mkheader_gemtek,-lz) diff --git a/tools/firmware-utils/src/mkrasimage.c b/tools/firmware-utils/src/mkrasimage.c new file mode 100644 index 0000000000..1cac7b5da9 --- /dev/null +++ b/tools/firmware-utils/src/mkrasimage.c @@ -0,0 +1,374 @@ +/* + * --- ZyXEL header format --- + * Original Version by Benjamin Berg <benjamin@sipsolutions.net> + * C implementation based on generation-script by Christian Lamparter <chunkeey@gmail.com> + * + * The firmware image prefixed with a header (which is written into the MTD device). + * The header is one erase block (~64KiB) in size, but the checksum only convers the + * first 2KiB. Padding is 0xff. All integers are in big-endian. + * + * The checksum is always a 16-Bit System V checksum (sum -s) stored in a 32-Bit integer. + * + * 4 bytes: checksum of the rootfs image + * 4 bytes: length of the contained rootfs image file (big endian) + * 32 bytes: Firmware Version string (NUL terminated, 0xff padded) + * 4 bytes: checksum over the header partition (big endian - see below) + * 64 bytes: Model (e.g. "NBG6617", NUL termiated, 0xff padded) + * 4 bytes: checksum of the kernel partition + * 4 bytes: length of the contained kernel image file (big endian) + * rest: 0xff padding (To erase block size) + * + * The checksums are calculated by adding up all bytes and if a 16bit + * overflow occurs, one is added and the sum is masked to 16 bit: + * csum = csum + databyte; if (csum > 0xffff) { csum += 1; csum &= 0xffff }; + * Should the file have an odd number of bytes then the byte len-0x800 is + * used additionally. + * + * The checksum for the header is calculated over the first 2048 bytes with + * the rootfs image checksum as the placeholder during calculation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published + * by the Free Software Foundation. + * + */ +#include <byteswap.h> +#include <getopt.h> +#include <libgen.h> +#include <stdio.h> +#include <string.h> +#include <stdlib.h> + +#define VERSION_STRING_LEN 31 +#define ROOTFS_HEADER_LEN 40 + +#define KERNEL_HEADER_LEN 8 + +#define BOARD_NAME_LEN 64 +#define BOARD_HEADER_LEN 68 + +#define HEADER_PARTITION_CALC_LENGTH 2048 +#define HEADER_PARTITION_LENGTH 0x10000 + +struct file_info { + char *name; /* name of the file */ + char *data; /* file content */ + size_t size; /* length of the file */ +}; + +static char *progname; + +static char *board_name = 0; +static char *version_name = 0; +static unsigned int rootfs_size = 0; + +static struct file_info kernel = {0, 0, 0}; +static struct file_info rootfs = {0, 0, 0}; +static struct file_info rootfs_out = {0, 0, 0}; +static struct file_info out = {0, 0, 0}; + +#define ERR(fmt, ...) do { \ + fflush(0); \ + fprintf(stderr, "[%s] *** error: " fmt "\n", \ + progname, ## __VA_ARGS__ ); \ +} while (0) + +void bufferFile(struct file_info *finfo) { + unsigned int fs = 0; + FILE *f = NULL; + + f = fopen(finfo->name, "rb"); + if (f == NULL) { + printf("Error while opening file %s.", finfo->name); + exit(EXIT_FAILURE); + } + + fseek(f, 0L, SEEK_END); + fs = (unsigned int) ftell(f); + rewind(f); + + finfo->size = fs; + + char *data = malloc(fs); + finfo->data = data; + size_t read = fread(data, fs, 1, f); + + if (read != 1) { + printf("Error reading file %s.", finfo->name); + exit(EXIT_FAILURE); + } + + fclose(f); +} + +void writeFile(struct file_info *finfo) { + FILE *fout = fopen(finfo->name, "w"); + + if (!fwrite(finfo->data, finfo->size, 1, fout)) { + printf("Wanted to write, but something went wrong.\n"); + exit(1); + } +} + +void usage(int status) { + FILE *stream = (status != EXIT_SUCCESS) ? stderr : stdout; + + fprintf(stream, "Usage: %s [OPTIONS...]\n", progname); + fprintf(stream, + "\n" + "Options:\n" + " -k <kernel> path for kernel image\n" + " -r <rootfs> path for rootfs image\n" + " -s <rfssize> size of output rootfs\n" + " -v <version> version string\n" + " -b <boardname> name of board to generate image for\n" + " -h show this screen\n" + ); + + exit(status); +} + +static int sysv_chksm(const unsigned char *data, int size) { + int r; + int checksum; + unsigned int s = 0; /* The sum of all the input bytes, modulo (UINT_MAX + 1). */ + + + for (int i = 0; i < size; i++) { + s += data[i]; + } + + r = (s & 0xffff) + ((s & 0xffffffff) >> 16); + checksum = (r & 0xffff) + (r >> 16); + + return checksum; +} + +char *generate_rootfs_header(struct file_info rootfs, char *version) { + size_t version_string_length; + unsigned int chksm, size; + char *rootfs_header; + size_t ptr = 0; + + rootfs_header = malloc(ROOTFS_HEADER_LEN); + memset(rootfs_header, 0xff, ROOTFS_HEADER_LEN); + + memcpy(rootfs_out.data, rootfs.data, rootfs.size); + + chksm = __bswap_32(sysv_chksm(rootfs_out.data, rootfs_out.size)); + size = __bswap_32(rootfs_out.size); + + memcpy(rootfs_header + ptr, &chksm, 4); + ptr += 4; + + memcpy(rootfs_header + ptr, &size, 4); + ptr += 4; + + version_string_length = strlen(version) <= VERSION_STRING_LEN ? strlen(version) : VERSION_STRING_LEN; + + memcpy(rootfs_header + ptr, version, version_string_length); + ptr += version_string_length; + + rootfs_header[ptr] = 0x0; + + return rootfs_header; +} + +char *generate_kernel_header(struct file_info kernel) { + unsigned int chksm, size; + char *kernel_header; + size_t ptr = 0; + + kernel_header = malloc(KERNEL_HEADER_LEN); + memset(kernel_header, 0xff, KERNEL_HEADER_LEN); + + chksm = __bswap_32(sysv_chksm(kernel.data, kernel.size)); + size = __bswap_32(kernel.size); + + memcpy(kernel_header + ptr, &chksm, 4); + ptr += 4; + + memcpy(kernel_header + ptr, &size, 4); + + return kernel_header; +} + +unsigned int generate_board_header_checksum(char *kernel_hdr, char *rootfs_hdr, char *boardname) { + char *board_hdr_tmp; + size_t ptr = 0; + + board_hdr_tmp = malloc(HEADER_PARTITION_CALC_LENGTH); + memset(board_hdr_tmp, 0xff, HEADER_PARTITION_CALC_LENGTH); + + memcpy(board_hdr_tmp, rootfs_hdr, ROOTFS_HEADER_LEN); + ptr += ROOTFS_HEADER_LEN; + + memcpy(board_hdr_tmp + ptr, rootfs_hdr, 4); /* Use RootFS checksum as placeholder */ + ptr += 4; + + memcpy(board_hdr_tmp + ptr, boardname, strlen(boardname)); /* Append boardname */ + ptr += strlen(boardname); + + board_hdr_tmp[ptr] = 0x0; /* Add null-terminator */ + ptr = ROOTFS_HEADER_LEN + 4 + BOARD_NAME_LEN; + + memcpy(board_hdr_tmp + ptr, kernel_hdr, 8); + + return __bswap_32(sysv_chksm(board_hdr_tmp, 2048)); +} + +char *generate_board_header(char *kernel_hdr, char *rootfs_hdr, char *boardname) { + unsigned int board_checksum; + char *board_hdr; + + board_hdr = malloc(BOARD_HEADER_LEN); + memset(board_hdr, 0xff, BOARD_HEADER_LEN); + + board_checksum = generate_board_header_checksum(kernel_hdr, rootfs_hdr, boardname); + + memcpy(board_hdr, &board_checksum, 4); + memcpy(board_hdr + 4, boardname, strlen(boardname)); + board_hdr[4 + strlen(boardname)] = 0x0; + + return board_hdr; +} + +int build_image() { + char *rootfs_header, *kernel_header, *board_header; + + size_t ptr; + + /* Load files */ + bufferFile(&kernel); + bufferFile(&rootfs); + + /* Allocate memory for temporary ouput rootfs */ + rootfs_out.data = malloc(rootfs_out.size); + memset(rootfs_out.data, 0x00, rootfs_out.size); + + /* Prepare headers */ + rootfs_header = generate_rootfs_header(rootfs, version_name); + kernel_header = generate_kernel_header(kernel); + board_header = generate_board_header(kernel_header, rootfs_header, board_name); + + /* Prepare output file */ + out.size = HEADER_PARTITION_LENGTH + rootfs_out.size + kernel.size; + out.data = malloc(out.size); + memset(out.data, 0xFF, out.size); + + /* Build output image */ + memcpy(out.data, rootfs_header, ROOTFS_HEADER_LEN); + memcpy(out.data + ROOTFS_HEADER_LEN, board_header, BOARD_HEADER_LEN); + memcpy(out.data + ROOTFS_HEADER_LEN + BOARD_HEADER_LEN, kernel_header, KERNEL_HEADER_LEN); + ptr = HEADER_PARTITION_LENGTH; + memcpy(out.data + ptr, rootfs_out.data, rootfs_out.size); + ptr += rootfs_out.size; + memcpy(out.data + ptr, kernel.data, kernel.size); + + /* Write back output image */ + writeFile(&out); + + /* Free allocated memory */ + free(kernel.data); + free(rootfs.data); + free(out.data); + free(rootfs_out.data); + + free(rootfs_header); + free(kernel_header); + free(board_header); + + return 0; +} + +int check_options() { + if (kernel.name == 0) { + ERR("No kernel filename supplied"); + return -1; + } + + if (rootfs.name == 0) { + ERR("No rootfs filename supplied"); + return -2; + } + + if (out.name == 0) { + ERR("No output filename supplied"); + return -3; + } + + if (board_name == 0) { + ERR("No board-name supplied"); + return -4; + } + + if (version_name == 0) { + ERR("No version supplied"); + return -5; + } + + if (rootfs_size <= 0) { + ERR("Invalid rootfs size supplied"); + return -6; + } + + if (strlen(board_name) > 31) { + ERR("Board name is to long"); + return -7; + } + return 0; +} + +int main(int argc, char *argv[]) { + int ret; + progname = basename(argv[0]); + while (1) { + int c; + + c = getopt(argc, argv, "b:k:o:r:s:v:h"); + if (c == -1) + break; + + switch (c) { + case 'b': + board_name = optarg; + break; + case 'h': + usage(EXIT_SUCCESS); + break; + case 'k': + kernel.name = optarg; + break; + case 'o': + out.name = optarg; + break; + case 'r': + rootfs.name = optarg; + break; + case 's': + sscanf(optarg, "%u", &rootfs_size); + break; + case 'v': + version_name = optarg; + break; + default: + usage(EXIT_FAILURE); + break; + } + } + + ret = check_options(); + if (ret) + usage(EXIT_FAILURE); + + /* As ZyXEL Web-GUI only accept images with a rootfs equal or larger than the first firmware shipped + * for the device, we need to pad rootfs partition to this size. To perform further calculations, we + * decide the size of this part here. In case the rootfs we want to integrate in our image is larger, + * take it's size, otherwise the supplied size. + * + * Be careful! We rely on assertion of correct size to be performed beforehand. It is unknown if images + * with a to large rootfs are accepted or not. + */ + rootfs_out.size = rootfs_size < rootfs.size ? rootfs.size : rootfs_size; + return build_image(); +}
The current make-ras.sh image generation script for the ZyXEL NBG6617 has portability issues with bash. Because of this, factory images are currently not built correctly by the OpenWRT buildbots. This commit replaces the make-ras.sh by C-written mkrasimage. The old script is still kept but can be deleted in the future. Signed-off-by: David Bauer <mail@david-bauer.net> --- include/image-commands.mk | 13 + target/linux/ipq40xx/image/Makefile | 2 +- tools/firmware-utils/Makefile | 1 + tools/firmware-utils/src/mkrasimage.c | 374 ++++++++++++++++++++++++++ 4 files changed, 389 insertions(+), 1 deletion(-) create mode 100644 tools/firmware-utils/src/mkrasimage.c