Re: [PATCH v3 2/2] fastboot: bail if update handler couldn't be found for bbu-partition
On Fri, Jun 10, 2022 at 08:36:35AM +0200, Michael Olbrich wrote: > On Thu, Jun 09, 2022 at 04:12:39PM +0200, Ahmad Fatoum wrote: > > On 09.06.22 16:10, Sascha Hauer wrote: > > > On Thu, Jun 09, 2022 at 03:09:36PM +0200, Ahmad Fatoum wrote: > > >> Fastboot would fall back to a raw copy even for bbu- partitions if > > >> no barebox_update handler was found. Prevent this by bailing out > > >> with an error code. > > >> > > >> Signed-off-by: Ahmad Fatoum > > >> --- > > >> v2 -> v3: > > >> - bail out instead of only printing message and falling back > > >> to raw copy (Sascha) > > >> v1 -> v2: > > >> - print message in case barebox_update handler is not found > > >> --- > > >> common/fastboot.c | 9 +++-- > > >> 1 file changed, 7 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/common/fastboot.c b/common/fastboot.c > > >> index 330a06f5a32f..a5cf04b39ecd 100644 > > >> --- a/common/fastboot.c > > >> +++ b/common/fastboot.c > > >> @@ -683,8 +683,13 @@ static void cb_flash(struct fastboot *fb, const > > >> char *cmd) > > >> }; > > >> > > >> handler = bbu_find_handler_by_device(data.devicefile); > > >> -if (!handler) > > >> -goto copy; > > >> +if (!handler) { > > >> +fastboot_tx_print(fb, FASTBOOT_MSG_FAIL, > > >> + "No barebox update handler > > >> registered for %s", > > >> + data.devicefile); > > >> +ret = -ENOENT; > > >> +goto out; > > >> +} > > > > > > I didn't verify that, but I believe a full raw bootable disk image > > > generated for i.MX will be detected as barebox image. With this patch we > > > wouldn't be able to flash that anymore. > > > > Can we just apply v2 and see how often we see the message? > > Or maybe the 'is a barebox image' detection is not strict enough? We know > the image size, right? Is there a way to determine the size of the actual > barebox image? If the two differ then it's not a barebox image after all. filetype_is_barebox_image() looks like this: bool filetype_is_barebox_image(enum filetype ft) { switch (ft) { case filetype_arm_barebox: case filetype_mips_barebox: case filetype_ch_image: case filetype_ch_image_be: case filetype_layerscape_image: case filetype_layerscape_qspi_image: case filetype_stm32_image_fsbl_v1: case filetype_fip: return true; default: return false; } } There's likely a way to determine the size for some of the images, but probably not for all. We could check the size of the image. If it's too big for a barebox image then it is none. I don't have a very good feeling about such a heuristic though. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 2/2] fastboot: bail if update handler couldn't be found for bbu-partition
On Thu, Jun 09, 2022 at 04:12:39PM +0200, Ahmad Fatoum wrote: > On 09.06.22 16:10, Sascha Hauer wrote: > > On Thu, Jun 09, 2022 at 03:09:36PM +0200, Ahmad Fatoum wrote: > >> Fastboot would fall back to a raw copy even for bbu- partitions if > >> no barebox_update handler was found. Prevent this by bailing out > >> with an error code. > >> > >> Signed-off-by: Ahmad Fatoum > >> --- > >> v2 -> v3: > >> - bail out instead of only printing message and falling back > >> to raw copy (Sascha) > >> v1 -> v2: > >> - print message in case barebox_update handler is not found > >> --- > >> common/fastboot.c | 9 +++-- > >> 1 file changed, 7 insertions(+), 2 deletions(-) > >> > >> diff --git a/common/fastboot.c b/common/fastboot.c > >> index 330a06f5a32f..a5cf04b39ecd 100644 > >> --- a/common/fastboot.c > >> +++ b/common/fastboot.c > >> @@ -683,8 +683,13 @@ static void cb_flash(struct fastboot *fb, const char > >> *cmd) > >>}; > >> > >>handler = bbu_find_handler_by_device(data.devicefile); > >> - if (!handler) > >> - goto copy; > >> + if (!handler) { > >> + fastboot_tx_print(fb, FASTBOOT_MSG_FAIL, > >> +"No barebox update handler registered > >> for %s", > >> +data.devicefile); > >> + ret = -ENOENT; > >> + goto out; > >> + } > > > > I didn't verify that, but I believe a full raw bootable disk image > > generated for i.MX will be detected as barebox image. With this patch we > > wouldn't be able to flash that anymore. > > Can we just apply v2 and see how often we see the message? Or maybe the 'is a barebox image' detection is not strict enough? We know the image size, right? Is there a way to determine the size of the actual barebox image? If the two differ then it's not a barebox image after all. Michael -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 2/2] fastboot: bail if update handler couldn't be found for bbu-partition
On 09.06.22 16:10, Sascha Hauer wrote: > On Thu, Jun 09, 2022 at 03:09:36PM +0200, Ahmad Fatoum wrote: >> Fastboot would fall back to a raw copy even for bbu- partitions if >> no barebox_update handler was found. Prevent this by bailing out >> with an error code. >> >> Signed-off-by: Ahmad Fatoum >> --- >> v2 -> v3: >> - bail out instead of only printing message and falling back >> to raw copy (Sascha) >> v1 -> v2: >> - print message in case barebox_update handler is not found >> --- >> common/fastboot.c | 9 +++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/common/fastboot.c b/common/fastboot.c >> index 330a06f5a32f..a5cf04b39ecd 100644 >> --- a/common/fastboot.c >> +++ b/common/fastboot.c >> @@ -683,8 +683,13 @@ static void cb_flash(struct fastboot *fb, const char >> *cmd) >> }; >> >> handler = bbu_find_handler_by_device(data.devicefile); >> -if (!handler) >> -goto copy; >> +if (!handler) { >> +fastboot_tx_print(fb, FASTBOOT_MSG_FAIL, >> + "No barebox update handler registered >> for %s", >> + data.devicefile); >> +ret = -ENOENT; >> +goto out; >> +} > > I didn't verify that, but I believe a full raw bootable disk image > generated for i.MX will be detected as barebox image. With this patch we > wouldn't be able to flash that anymore. Can we just apply v2 and see how often we see the message? > > Sascha > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 2/2] fastboot: bail if update handler couldn't be found for bbu-partition
On Thu, Jun 09, 2022 at 03:09:36PM +0200, Ahmad Fatoum wrote: > Fastboot would fall back to a raw copy even for bbu- partitions if > no barebox_update handler was found. Prevent this by bailing out > with an error code. > > Signed-off-by: Ahmad Fatoum > --- > v2 -> v3: > - bail out instead of only printing message and falling back > to raw copy (Sascha) > v1 -> v2: > - print message in case barebox_update handler is not found > --- > common/fastboot.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/common/fastboot.c b/common/fastboot.c > index 330a06f5a32f..a5cf04b39ecd 100644 > --- a/common/fastboot.c > +++ b/common/fastboot.c > @@ -683,8 +683,13 @@ static void cb_flash(struct fastboot *fb, const char > *cmd) > }; > > handler = bbu_find_handler_by_device(data.devicefile); > - if (!handler) > - goto copy; > + if (!handler) { > + fastboot_tx_print(fb, FASTBOOT_MSG_FAIL, > + "No barebox update handler registered > for %s", > + data.devicefile); > + ret = -ENOENT; > + goto out; > + } I didn't verify that, but I believe a full raw bootable disk image generated for i.MX will be detected as barebox image. With this patch we wouldn't be able to flash that anymore. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH v3 2/2] fastboot: bail if update handler couldn't be found for bbu-partition
Fastboot would fall back to a raw copy even for bbu- partitions if no barebox_update handler was found. Prevent this by bailing out with an error code. Signed-off-by: Ahmad Fatoum --- v2 -> v3: - bail out instead of only printing message and falling back to raw copy (Sascha) v1 -> v2: - print message in case barebox_update handler is not found --- common/fastboot.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/common/fastboot.c b/common/fastboot.c index 330a06f5a32f..a5cf04b39ecd 100644 --- a/common/fastboot.c +++ b/common/fastboot.c @@ -683,8 +683,13 @@ static void cb_flash(struct fastboot *fb, const char *cmd) }; handler = bbu_find_handler_by_device(data.devicefile); - if (!handler) - goto copy; + if (!handler) { + fastboot_tx_print(fb, FASTBOOT_MSG_FAIL, + "No barebox update handler registered for %s", + data.devicefile); + ret = -ENOENT; + goto out; + } fastboot_tx_print(fb, FASTBOOT_MSG_INFO, "This is a barebox image..."); -- 2.30.2 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox