[U-Boot] [RFC PATCH] cmd: avb: Support A/B slots

Igor Opaniuk igor.opaniuk at gmail.com
Wed Aug 7 09:38:33 UTC 2019


+ added also David Zeuthen to CC (I hope he doesn't mind :) ),
who actually introduced all these changes in libavb.

On Tue, Aug 6, 2019 at 4:38 PM Sam Protsenko <semen.protsenko at linaro.org> wrote:
>
> Hi Igor,
>
> On Tue, Aug 6, 2019 at 4:07 PM Igor Opaniuk <igor.opaniuk at gmail.com> wrote:
> >
> > Hi Sam,
> >
> > Sorry for the late reply,
> >
> > On Fri, Aug 2, 2019 at 9:57 PM Sam Protsenko <semen.protsenko at linaro.org> wrote:
> > >
> > > Hi Igor, Jens,
> > >
> > > Can you please comments on next topics:
> > >   1. With enabled A/B partitions, we have boot_a/boot_b,
> > >      system_a/system_b, vendor_a/vendor_b partitions. Therefore
> > >      requested_partitions[] should be slotted (which is done in this RFC
> > >      patch). But this patch doesn' handle item (2) below.
> > >   2. With dynamic partitions enabled, we don't have system/vendor
> > >      anymore; instead we have single "super" partitions. Therefore
> > >      requested_partitions[] table contains wrong partitions list for
> > >      that particular case.
> >
> > This case can be handled in the latest libavb by
> > 49936b4c010(libavb: Support vbmeta blobs in beginning of partition) [1].
> > Anyway, this will require to pull the latest libavb sources into U-boot.
> >
> > >
> > > Question: can we allow user to select which partition to verify, instead
> > > of trying to verify hard-coded partitions from requested_partitions[]
> > > table? This would solve both (1) and (2) items. But I'm not sure about
> > > next possible issues:
> > >   a. Wouldn't it break chain of trust somehow?
> >
> > It wont. If the user can obtain access to U-boot shell or edit U-boot
> > env, the chain of trust is already broken (he can just wipe off
> > `avb_verify` cmd invocation and that's it). But anyway, at first,
> > check this solution [1].
> >
>
> Thanks for your reply. So as I understand we need to:
>   1. Pull new libavb from AOSP to U-Boot
Right

>   2. Provide a parameter to 'avb verify' command, to pass partition to verify
>   3. Rework AM57x boot procedure, and instead of just one 'avb verify'
> call we should call 'avb verify' several times,one time per partition
In the latest libavb you can even provide empty requested_partitions[] array,
as a list of partitions to verify can be obtained from vbmeta image
(this is done implicitly
in avb_slot_verify() here [2]). I assume this was also possible at the
time when I added
initial snapshot of libavb to U-boot, and I hard-coded "vendor"/"system" in
requested_partitions[] by mistake (as it wasn't obvious for me either).

Regarding the additional param, you'll definitely need to provide a slot suffix,
which now can be extracted from android a/b meta partition by `ab_select` cmd.
Example about how avb_slot_verify() is called with proper slot suffix
is here [3].

>
> Is that correct or you have another idea how to improve it?
>
> > >   b. Is it ok to run avb_slot_verify() several times (one time per one
> > >      partition?
> > >
> > > If (a) or (b) is of any concern, then maybe we can provide a way for the
> > > user to pass any number of arguments to 'avb verify', like this:
> > >
> > >     => avb verify boot_a super_a dtbo_a
> > >
> > > so help synopsis for 'avb verify' can be like this:
> > >
> > >     avb verify <partition> ...
> > >
> > > What do you think about this? Which would be the best course of action
> > > to fix both issues (1) and (2)?
> > >
> > > Thanks.
> > >
> > > Signed-off-by: Sam Protsenko <semen.protsenko at linaro.org>
> > > ---
> > >  cmd/avb.c | 13 +++++++++----
> > >  1 file changed, 9 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/cmd/avb.c b/cmd/avb.c
> > > index 3f6fd763a0..d1942d6605 100644
> > > --- a/cmd/avb.c
> > > +++ b/cmd/avb.c
> > > @@ -235,6 +235,7 @@ int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag,
> > >         AvbSlotVerifyData *out_data;
> > >         char *cmdline;
> > >         char *extra_args;
> > > +       char *slot_suffix = "";
> > >
> > >         bool unlocked = false;
> > >         int res = CMD_RET_FAILURE;
> > > @@ -244,9 +245,12 @@ int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag,
> > >                 return CMD_RET_FAILURE;
> > >         }
> > >
> > > -       if (argc != 1)
> > > +       if (argc < 1 || argc > 2)
> > >                 return CMD_RET_USAGE;
> > >
> > > +       if (argc == 2)
> > > +               slot_suffix = argv[1];
> > > +
> > >         printf("## Android Verified Boot 2.0 version %s\n",
> > >                avb_version_string());
> > >
> > > @@ -259,7 +263,7 @@ int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag,
> > >         slot_result =
> > >                 avb_slot_verify(avb_ops,
> > >                                 requested_partitions,
> > > -                               "",
> > > +                               slot_suffix,
> > >                                 unlocked,
> > >                                 AVB_HASHTREE_ERROR_MODE_RESTART_AND_INVALIDATE,
> > >                                 &out_data);
> > > @@ -419,7 +423,7 @@ static cmd_tbl_t cmd_avb[] = {
> > >         U_BOOT_CMD_MKENT(read_part, 5, 0, do_avb_read_part, "", ""),
> > >         U_BOOT_CMD_MKENT(read_part_hex, 4, 0, do_avb_read_part_hex, "", ""),
> > >         U_BOOT_CMD_MKENT(write_part, 5, 0, do_avb_write_part, "", ""),
> > > -       U_BOOT_CMD_MKENT(verify, 1, 0, do_avb_verify_part, "", ""),
> > > +       U_BOOT_CMD_MKENT(verify, 2, 0, do_avb_verify_part, "", ""),
> > >  #ifdef CONFIG_OPTEE_TA_AVB
> > >         U_BOOT_CMD_MKENT(read_pvalue, 3, 0, do_avb_read_pvalue, "", ""),
> > >         U_BOOT_CMD_MKENT(write_pvalue, 3, 0, do_avb_write_pvalue, "", ""),
> > > @@ -462,6 +466,7 @@ U_BOOT_CMD(
> > >         "avb read_pvalue <name> <bytes> - read a persistent value <name>\n"
> > >         "avb write_pvalue <name> <value> - write a persistent value <name>\n"
> > >  #endif
> > > -       "avb verify - run verification process using hash data\n"
> > > +       "avb verify [slot_suffix] - run verification process using hash data\n"
> > >         "    from vbmeta structure\n"
> > > +       "    [slot_suffix] - _a, _b, etc (if vbmeta partition is slotted)\n"
> > >         );
> > > --
> > > 2.20.1
> > >
> >
> > Thanks
> >
> > [1] https://android.googlesource.com/platform/external/avb/+/49936b4c0109411fdd38bd4ba3a32a01c40439a9
> >
> > --
> > Best regards - Freundliche Grüsse - Meilleures salutations
> >
> > Igor Opaniuk
> >
> > mailto: igor.opaniuk at gmail.com
> > skype: igor.opanyuk
> > +380 (93) 836 40 67
> > http://ua.linkedin.com/in/iopaniuk

[2] https://android.googlesource.com/platform/external/avb/+/master/libavb/avb_slot_verify.c#1461
[3] https://android.googlesource.com/platform/external/avb/+/master/test/avb_slot_verify_unittest.cc#110

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk at gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk


More information about the U-Boot mailing list