| Message ID | 20230122142028.452861-1-hauke@hauke-m.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | tools: mkimage: Allow changing U-Boot image magic | expand |
Hi, On Sun, 22 Jan 2023 at 07:20, Hauke Mehrtens <hauke@hauke-m.de> wrote: > > Extend mkimage with a new optional option -M to specify a special U-Boot > image magic number. OpenWrt ships images for about 30 different boards > with vendor boot loaders which are expecting the mkimage format, but > with a different image magic. > > OpenWrt includes this patch for mkimage more than 10 years. > It was added by Gabor Juhos in this commit: > https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=f3d2056b81b7a92d28402c22736534d84fe23cfe > > Cc: Gabor Juhos <j4g8y7@gmail.com> > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > --- > tools/default_image.c | 4 ++-- > tools/imagetool.h | 1 + > tools/mkimage.c | 14 ++++++++++++-- > 3 files changed, 15 insertions(+), 4 deletions(-) Strange...what is the different magic for? > > diff --git a/tools/default_image.c b/tools/default_image.c > index 0ac3382003..514edf05a5 100644 > --- a/tools/default_image.c > +++ b/tools/default_image.c > @@ -57,7 +57,7 @@ static int image_verify_header(unsigned char *ptr, int image_size, > */ > memcpy(hdr, ptr, sizeof(struct legacy_img_hdr)); > > - if (be32_to_cpu(hdr->ih_magic) != IH_MAGIC) { > + if (be32_to_cpu(hdr->ih_magic) != params->magic) { > debug("%s: Bad Magic Number: \"%s\" is no valid image\n", > params->cmdname, params->imagefile); > return -FDT_ERR_BADMAGIC; > @@ -126,7 +126,7 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd, > } > > /* Build new header */ > - image_set_magic(hdr, IH_MAGIC); > + image_set_magic(hdr, params->magic); > image_set_time(hdr, time); > image_set_size(hdr, imagesize); > image_set_load(hdr, addr); > diff --git a/tools/imagetool.h b/tools/imagetool.h > index ca7c2e48ba..5e61228a5f 100644 > --- a/tools/imagetool.h > +++ b/tools/imagetool.h > @@ -59,6 +59,7 @@ struct image_tool_params { > int arch; > int type; > int comp; > + unsigned int magic; > char *dtc; > unsigned int addr; > unsigned int ep; > diff --git a/tools/mkimage.c b/tools/mkimage.c > index 8306861ce5..957abe115c 100644 > --- a/tools/mkimage.c > +++ b/tools/mkimage.c > @@ -26,6 +26,7 @@ static struct image_tool_params params = { > .arch = IH_ARCH_PPC, > .type = IH_TYPE_KERNEL, > .comp = IH_COMP_GZIP, > + .magic = IH_MAGIC, > .dtc = MKIMAGE_DEFAULT_DTC_OPTIONS, > .imagename = "", > .imagename2 = "", > @@ -89,11 +90,12 @@ static void usage(const char *msg) > " -q ==> quiet\n", > params.cmdname); > fprintf(stderr, > - " %s [-x] -A arch -O os -T type -C comp -a addr -e ep -n name -d data_file[:data_file...] image\n" > + " %s [-x] -A arch -O os -T type -C comp -M magic -a addr -e ep -n name -d data_file[:data_file...] image\n" > " -A ==> set architecture to 'arch'\n" > " -O ==> set operating system to 'os'\n" > " -T ==> set image type to 'type'\n" > " -C ==> set compression type 'comp'\n" > + " -M ==> set image magic to 'magic' (hex)\n" > " -a ==> set load address to 'addr' (hex)\n" > " -e ==> set entry point to 'ep' (hex)\n" > " -n ==> set image name to 'name'\n" > @@ -159,7 +161,7 @@ static int add_content(int type, const char *fname) > } > > static const char optstring[] = > - "a:A:b:B:c:C:d:D:e:Ef:Fg:G:i:k:K:ln:N:o:O:p:qrR:stT:vVx"; > + "a:A:b:B:c:C:d:D:e:Ef:Fg:G:i:k:K:lM:n:N:o:O:p:qrR:stT:vVx"; > > static const struct option longopts[] = { > { "load-address", required_argument, NULL, 'a' }, > @@ -298,6 +300,14 @@ static void process_args(int argc, char **argv) > case 'l': > params.lflag = 1; > break; > + case 'M': > + params.magic = strtoull(optarg, &ptr, 16); > + if (*ptr) { > + fprintf(stderr, "%s: invalid magic %s\n", > + params.cmdname, optarg); > + exit(EXIT_FAILURE); > + } > + break; > case 'n': > params.imagename = optarg; > break; > -- > 2.39.0 > REgards, Simon
On 1/23/23 19:49, Simon Glass wrote: > Hi, > > On Sun, 22 Jan 2023 at 07:20, Hauke Mehrtens <hauke@hauke-m.de> wrote: >> >> Extend mkimage with a new optional option -M to specify a special U-Boot >> image magic number. OpenWrt ships images for about 30 different boards >> with vendor boot loaders which are expecting the mkimage format, but >> with a different image magic. >> >> OpenWrt includes this patch for mkimage more than 10 years. >> It was added by Gabor Juhos in this commit: >> https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=f3d2056b81b7a92d28402c22736534d84fe23cfe >> >> Cc: Gabor Juhos <j4g8y7@gmail.com> >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> >> --- >> tools/default_image.c | 4 ++-- >> tools/imagetool.h | 1 + >> tools/mkimage.c | 14 ++++++++++++-- >> 3 files changed, 15 insertions(+), 4 deletions(-) > > Strange...what is the different magic for? I do not know really know why some vendors do this. With Openwrt we create images which user can install on their devices. We normally do not change the vendor bootloader and create an image which is compatible with the vendor boot loader. I assume the vendors want to prevent users from installing "unsupported" images, but still want to use something close to the U-Boot image. With a different magic their boot loader (often vendor modified U-Boot) can prevent the user from installing "wrong" images. I saw this on many devices sold by NetGear for example these devices: NETGEAR WNDR4300 NETGEAR WNDR4500 NETGEAR WNR1000 NETGEAR WNR612 v2 I have also seen it from other vendors, probably some ODM or OEM does this. Hauke
Hi Hauke, On Mon, 23 Jan 2023 at 15:45, Hauke Mehrtens <hauke@hauke-m.de> wrote: > > On 1/23/23 19:49, Simon Glass wrote: > > Hi, > > > > On Sun, 22 Jan 2023 at 07:20, Hauke Mehrtens <hauke@hauke-m.de> wrote: > >> > >> Extend mkimage with a new optional option -M to specify a special U-Boot > >> image magic number. OpenWrt ships images for about 30 different boards > >> with vendor boot loaders which are expecting the mkimage format, but > >> with a different image magic. > >> > >> OpenWrt includes this patch for mkimage more than 10 years. > >> It was added by Gabor Juhos in this commit: > >> https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=f3d2056b81b7a92d28402c22736534d84fe23cfe > >> > >> Cc: Gabor Juhos <j4g8y7@gmail.com> > >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > >> --- > >> tools/default_image.c | 4 ++-- > >> tools/imagetool.h | 1 + > >> tools/mkimage.c | 14 ++++++++++++-- > >> 3 files changed, 15 insertions(+), 4 deletions(-) > > > > Strange...what is the different magic for? > > I do not know really know why some vendors do this. With Openwrt we > create images which user can install on their devices. We normally do > not change the vendor bootloader and create an image which is compatible > with the vendor boot loader. > > I assume the vendors want to prevent users from installing "unsupported" > images, but still want to use something close to the U-Boot image. With > a different magic their boot loader (often vendor modified U-Boot) can > prevent the user from installing "wrong" images. > > I saw this on many devices sold by NetGear for example these devices: > NETGEAR WNDR4300 > NETGEAR WNDR4500 > NETGEAR WNR1000 > NETGEAR WNR612 v2 > > I have also seen it from other vendors, probably some ODM or OEM does this. > OK, but I think it would be better to use a FIT, thus allowing the vendor to add more info into the file. Are the vendor bootloaders upstreamed? Anyway, I don't see a big problem with doing this. Can you please update the mkimage man page? Do we need to update dumpimage also? Regards, Simon
diff --git a/tools/default_image.c b/tools/default_image.c index 0ac3382003..514edf05a5 100644 --- a/tools/default_image.c +++ b/tools/default_image.c @@ -57,7 +57,7 @@ static int image_verify_header(unsigned char *ptr, int image_size, */ memcpy(hdr, ptr, sizeof(struct legacy_img_hdr)); - if (be32_to_cpu(hdr->ih_magic) != IH_MAGIC) { + if (be32_to_cpu(hdr->ih_magic) != params->magic) { debug("%s: Bad Magic Number: \"%s\" is no valid image\n", params->cmdname, params->imagefile); return -FDT_ERR_BADMAGIC; @@ -126,7 +126,7 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd, } /* Build new header */ - image_set_magic(hdr, IH_MAGIC); + image_set_magic(hdr, params->magic); image_set_time(hdr, time); image_set_size(hdr, imagesize); image_set_load(hdr, addr); diff --git a/tools/imagetool.h b/tools/imagetool.h index ca7c2e48ba..5e61228a5f 100644 --- a/tools/imagetool.h +++ b/tools/imagetool.h @@ -59,6 +59,7 @@ struct image_tool_params { int arch; int type; int comp; + unsigned int magic; char *dtc; unsigned int addr; unsigned int ep; diff --git a/tools/mkimage.c b/tools/mkimage.c index 8306861ce5..957abe115c 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -26,6 +26,7 @@ static struct image_tool_params params = { .arch = IH_ARCH_PPC, .type = IH_TYPE_KERNEL, .comp = IH_COMP_GZIP, + .magic = IH_MAGIC, .dtc = MKIMAGE_DEFAULT_DTC_OPTIONS, .imagename = "", .imagename2 = "", @@ -89,11 +90,12 @@ static void usage(const char *msg) " -q ==> quiet\n", params.cmdname); fprintf(stderr, - " %s [-x] -A arch -O os -T type -C comp -a addr -e ep -n name -d data_file[:data_file...] image\n" + " %s [-x] -A arch -O os -T type -C comp -M magic -a addr -e ep -n name -d data_file[:data_file...] image\n" " -A ==> set architecture to 'arch'\n" " -O ==> set operating system to 'os'\n" " -T ==> set image type to 'type'\n" " -C ==> set compression type 'comp'\n" + " -M ==> set image magic to 'magic' (hex)\n" " -a ==> set load address to 'addr' (hex)\n" " -e ==> set entry point to 'ep' (hex)\n" " -n ==> set image name to 'name'\n" @@ -159,7 +161,7 @@ static int add_content(int type, const char *fname) } static const char optstring[] = - "a:A:b:B:c:C:d:D:e:Ef:Fg:G:i:k:K:ln:N:o:O:p:qrR:stT:vVx"; + "a:A:b:B:c:C:d:D:e:Ef:Fg:G:i:k:K:lM:n:N:o:O:p:qrR:stT:vVx"; static const struct option longopts[] = { { "load-address", required_argument, NULL, 'a' }, @@ -298,6 +300,14 @@ static void process_args(int argc, char **argv) case 'l': params.lflag = 1; break; + case 'M': + params.magic = strtoull(optarg, &ptr, 16); + if (*ptr) { + fprintf(stderr, "%s: invalid magic %s\n", + params.cmdname, optarg); + exit(EXIT_FAILURE); + } + break; case 'n': params.imagename = optarg; break;
Extend mkimage with a new optional option -M to specify a special U-Boot image magic number. OpenWrt ships images for about 30 different boards with vendor boot loaders which are expecting the mkimage format, but with a different image magic. OpenWrt includes this patch for mkimage more than 10 years. It was added by Gabor Juhos in this commit: https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=f3d2056b81b7a92d28402c22736534d84fe23cfe Cc: Gabor Juhos <j4g8y7@gmail.com> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> --- tools/default_image.c | 4 ++-- tools/imagetool.h | 1 + tools/mkimage.c | 14 ++++++++++++-- 3 files changed, 15 insertions(+), 4 deletions(-)