[PATCH v1] fastboot: introduce 'oem board' subcommand

[Alexey Romanov]

Hi,

On Tue, Jan 09, 2024 at 10:45:46AM -0500, Sean Anderson wrote:
> On 1/9/24 05:27, Alexey Romanov wrote:
> > Hello Sean!
> > 
> > Thanks for you reply.
> > 
> > On Thu, Dec 28, 2023 at 11:45:04AM -0500, Sean Anderson wrote:
> >> On 12/28/23 10:25, Alexey Romanov wrote:
> >> > Currently, fastboot protocol in U-Boot has no opportunity
> >> > to execute vendor custom code with verifed boot.
> >> 
> >> Well, I would say the most conventional way to do this would be something like
> >> 
> >> => fastboot 0
> >> => source \# CONFIG_FASTBOOT_BUF_ADDR
> >> 
> >> and on your host machine,
> >> 
> >> $ fastboot stage my_script.itb
> >> 
> >> where my_script.its looks like
> >> 
> >> /dts-v1/;
> >> 
> >> / {
> >>     description = "my script";
> >>     #address-cells = <1>;
> >> 
> >>     images {
> >>         my-script {
> >>             data = /incbin/("my_script.scr");
> >>             type = "script";
> >>             arch = "arm64";
> >>             compression = "none";
> >>             hash-1 {
> >>                 algo = "sha256";
> >>             };
> >>         };
> >>     };
> >> 
> >>     configurations {
> >>         default = "conf";
> >>         conf {
> >>             description = "Load my script";
> >>             script = "my-script";
> >>             signature {
> >>                 algo = "sha256,rsa2048";
> >>                 key-name-hint = "vboot";
> >>                 sign-images = "script";
> >>             };
> >>         };
> >>     };
> >> };
> >> 
> >> This method is especially useful to pass complex parameters to your command.
> >> This method of course requires commit bcc85b96b5f ("cmd: source: Support
> >> specifying config name").
> >> 
> >> Would it be possible to use the above method for your use case?
> > 
> > This method sounds good for some cases, but we have encountered some
> > problems that prevent us from applying it:
> > 
> > 1. Looks like this method requires access to UART (for typing 'source'
> > command). Open the UART is unsafe at devices with verified boot.
> 
> Generally the idea is that you run source after you run fastboot. E.g. you set
> your altbootcmd (or whatever) to something like
> 
> while true; fastboot 0; source \# || boot; done
> 
> so you try to source whatever gets staged, and boot it otherwise.

If I understand right, U-Boot always will try fastboot mode?
Yes, it seems like it will work. But the code for complex
fastboot scripts will be complex and difficult to read.

I think my version looks more elegant and simple.

> 
> > 2. We use automation scripts to flash our devices using fastboot
> > protocol. A typical example of flashing scripts (device is already in
> > fastboot mode):
> > 
> >   $ fastboot erase bootloader
> >   $ fastboot stage bootloader.img
> >   $ fastboot oem board:write_bootloader
> > 
> >   $ fastboot reboot-bootloader
> > 
> >   $ fastboot erase super
> >   $ fastboot flash super super.bin
> > 
> >   $ fastboot reboot
> > 
> > This method doesn't assume what something typing additional command in
> > U-Boot shell, even if we have access to UART.
> 
> I'm curious what you actual usage for this is? That is, what do you need
> a custom command for.

Our SoC vendor use custom scheme for flashing bootloader partition.
So we can't just use fastbooot flash command - we have to use custom
flashing logic. We also don't want to use hacks in generic fastboot
code, for example something like this in _fb_nand_write():

  ...

  if (!strcmp(part->name, "bootloader"))
     write_bootloader_custom_logic(...)
  else
    nand_write_skip_bad(...)

  ...

> 
> --Sean
> 
> >> 
> >> --Sean
> >> 
> >> > This patch
> >> > introduce new fastboot subcommand fastboot oem board:<cmd>,
> >> > which allow to run custom oem_board function.
> >> > =
> >> > Default implementation is __weak. Vendor must redefine it in
> >> > board/ folder with his own logic.
> >> > 
> >> > For example, some vendors have their custom nand/emmc partition
> >> > flashing or erasing. Here some typical command for such use cases:
> >> > 
> >> > - flashing:
> >> > 
> >> >   $ fastboot stage bootloader.img
> >> >   $ fastboot oem board:write_bootloader
> >> > 
> >> > - erasing:
> >> > 
> >> >   $ fastboot oem board:erase_env
> >> > 
> >> > Signed-off-by: Alexey Romanov <avromanov at salutedevices.com>
> >> > ---
> >> >  drivers/fastboot/Kconfig      |  7 +++++++
> >> >  drivers/fastboot/fb_command.c | 15 +++++++++++++++
> >> >  include/fastboot.h            |  1 +
> >> >  3 files changed, 23 insertions(+)
> >> > 
> >> > diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> >> > index 3cfeea4837..4c955cabab 100644
> >> > --- a/drivers/fastboot/Kconfig
> >> > +++ b/drivers/fastboot/Kconfig
> >> > @@ -241,6 +241,13 @@ config FASTBOOT_OEM_RUN
> >> >  	  this feature if you are using verified boot, as it will allow an
> >> >  	  attacker to bypass any restrictions you have in place.
> >> >  
> >> > +config FASTBOOT_OEM_BOARD
> >> > +	bool "Enable the 'oem board' command"
> >> > +	help
> >> > +	  This extends the fastboot protocol with an "oem board" command. This
> >> > +	  command allows running vendor custom code defined in board/ files.
> >> > +	  Otherwise, it will do nothing and send fastboot fail.
> >> > +
> >> >  endif # FASTBOOT
> >> >  
> >> >  endmenu
> >> > diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c
> >> > index 71cfaec6e9..4d2b451f46 100644
> >> > --- a/drivers/fastboot/fb_command.c
> >> > +++ b/drivers/fastboot/fb_command.c
> >> > @@ -39,6 +39,7 @@ static void reboot_recovery(char *, char *);
> >> >  static void oem_format(char *, char *);
> >> >  static void oem_partconf(char *, char *);
> >> >  static void oem_bootbus(char *, char *);
> >> > +static void oem_board(char *, char *);
> >> >  static void run_ucmd(char *, char *);
> >> >  static void run_acmd(char *, char *);
> >> >  
> >> > @@ -106,6 +107,10 @@ static const struct {
> >> >  		.command = "oem run",
> >> >  		.dispatch = CONFIG_IS_ENABLED(FASTBOOT_OEM_RUN, (run_ucmd), (NULL))
> >> >  	},
> >> > +	[FASTBOOT_COMMAND_OEM_BOARD] = {
> >> > +		.command = "oem board",
> >> > +		.dispatch = CONFIG_IS_ENABLED(FASTBOOT_OEM_BOARD, (oem_board), (NULL))
> >> > +	},
> >> >  	[FASTBOOT_COMMAND_UCMD] = {
> >> >  		.command = "UCmd",
> >> >  		.dispatch = CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (run_ucmd), (NULL))
> >> > @@ -489,3 +494,13 @@ static void __maybe_unused oem_bootbus(char *cmd_parameter, char *response)
> >> >  	else
> >> >  		fastboot_okay(NULL, response);
> >> >  }
> >> > +
> >> > +void __weak fastboot_oem_board(char *cmd_parameter, void *data, u32 size, char *response)
> >> > +{
> >> > +	fastboot_fail("oem board function not defined", response);
> >> > +}
> >> > +
> >> > +static void __maybe_unused oem_board(char *cmd_parameter, char *response)
> >> > +{
> >> > +	fastboot_oem_board(cmd_parameter, fastboot_buf_addr, image_size, response);
> >> > +}
> >> > diff --git a/include/fastboot.h b/include/fastboot.h
> >> > index 296451f89d..06c1f26b6c 100644
> >> > --- a/include/fastboot.h
> >> > +++ b/include/fastboot.h
> >> > @@ -37,6 +37,7 @@ enum {
> >> >  	FASTBOOT_COMMAND_OEM_PARTCONF,
> >> >  	FASTBOOT_COMMAND_OEM_BOOTBUS,
> >> >  	FASTBOOT_COMMAND_OEM_RUN,
> >> > +	FASTBOOT_COMMAND_OEM_BOARD,
> >> >  	FASTBOOT_COMMAND_ACMD,
> >> >  	FASTBOOT_COMMAND_UCMD,
> >> >  	FASTBOOT_COMMAND_COUNT
> >> 
> > 
> 

-- 
Thank you,
Alexey