[U-Boot] [PATCH 4/8] cmd: avb2.0: avb command for performing verification

[Simon Glass]

Hi Igor,

On 16 May 2018 at 01:44, Igor Opaniuk <igor.opaniuk at linaro.org> wrote:
> Hi Simon,
>
> I've dug into DriverModel documentation and even created a PoC for
> existing avb commands. The problem is that (maybe I missed out some
> key concepts) I'm still
> not sure if it makes sense to follow it driver mode in the context of
> AVB 2.0 feature and
> what kind of extra devices can be used within the same uclass in this case?
> Could you please explain in detail.
> Thanks

avb_ops_alloc() is allocating a struct and then assigning operations
to its members.

This is what driver model is designed to do. It handles the
allocations and the operations become part of the uclass interface.

At present it looks like you only have one driver. I'm not sure if it
would make sense to have a second one.

As a counter example see cros_ec uclass, which does things
differently. It models the EC interface (I2C, SPI, LPC) using DM, with
just a single impl on top.

Also can you please drop the CamelCase in the patches? We don't use
that in U-Boot.

>
> Hi Sam,
> Thanks, will fix!
>
>
> On 3 May 2018 at 05:31, Simon Glass <sjg at chromium.org> wrote:
>> Hi Igor,
>>
>> On 25 April 2018 at 07:18, Igor Opaniuk <igor.opaniuk at linaro.org> wrote:
>>> Enable a "avb" command to execute Android Verified
>>> Boot 2.0 operations. It includes such subcommands:
>>>   avb init - initialize avb2 subsystem
>>>   avb read_rb - read rollback index
>>>   avb write_rb - write rollback index
>>>   avb is_unlocked - check device lock state
>>>   avb get_uuid - read and print uuid of a partition
>>>   avb read_part - read data from partition
>>>   avb read_part_hex - read data from partition and output to stdout
>>>   avb write_part - write data to partition
>>>   avb verify - run full verification chain
>>>
>>> Signed-off-by: Igor Opaniuk <igor.opaniuk at linaro.org>
>>> ---
>>>  cmd/Kconfig  |  15 +++
>>>  cmd/Makefile |   3 +
>>>  cmd/avb.c    | 351 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 369 insertions(+)
>>>  create mode 100644 cmd/avb.c
>>>
>>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>>> index bc1d2f3..96695ff 100644
>>> --- a/cmd/Kconfig
>>> +++ b/cmd/Kconfig
>>> @@ -1675,6 +1675,21 @@ config CMD_TRACE
>>>           for analsys (e.g. using bootchart). See doc/README.trace for full
>>>           details.
>>>
>>> +config CMD_AVB
>>> +       bool "avb - Android Verified Boot 2.0 operations"
>>> +       depends on LIBAVB_AB
>>> +       help
>>> +         Enables a "avb" command to perform verification of partitions using
>>> +         Android Verified Boot 2.0 functionality. It includes such subcommands:
>>> +           avb init - initialize avb2 subsystem
>>> +           avb read_rb - read rollback index
>>> +           avb write_rb - write rollback index
>>> +           avb is_unlocked - check device lock state
>>> +           avb get_uuid - read and print uuid of a partition
>>> +           avb read_part - read data from partition
>>> +           avb read_part_hex - read data from partition and output to stdout
>>> +           avb write_part - write data to partition
>>> +           avb verify - run full verification chain
>>>  endmenu
>>>
>>>  config CMD_UBI
>>> diff --git a/cmd/Makefile b/cmd/Makefile
>>> index c4269ac..bbf6c2a 100644
>>> --- a/cmd/Makefile
>>> +++ b/cmd/Makefile
>>> @@ -151,6 +151,9 @@ obj-$(CONFIG_CMD_REGULATOR) += regulator.o
>>>
>>>  obj-$(CONFIG_CMD_BLOB) += blob.o
>>>
>>> +# Android Verified Boot 2.0
>>> +obj-$(CONFIG_CMD_AVB) += avb.o
>>> +
>>>  obj-$(CONFIG_X86) += x86/
>>>  endif # !CONFIG_SPL_BUILD
>>>
>>> diff --git a/cmd/avb.c b/cmd/avb.c
>>> new file mode 100644
>>> index 0000000..d040906
>>> --- /dev/null
>>> +++ b/cmd/avb.c
>>> @@ -0,0 +1,351 @@
>>> +
>>> +/*
>>> + * (C) Copyright 2018, Linaro Limited
>>> + *
>>> + * SPDX-License-Identifier:    GPL-2.0+
>>> + */
>>> +
>>> +#include <avb_verify.h>
>>> +#include <command.h>
>>> +#include <image.h>
>>> +#include <malloc.h>
>>> +#include <mmc.h>
>>> +
>>> +#define AVB_BOOTARGS   "avb_bootargs"
>>> +static struct AvbOps *avb_ops;
>>> +
>>> +static const char * const requested_partitions[] = {"boot",
>>> +                                            "system",
>>> +                                            "vendor",
>>> +                                            NULL};
>>> +
>>> +int do_avb_init(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>> +{
>>> +       unsigned long mmc_dev;
>>> +
>>> +       if (argc != 2)
>>> +               return CMD_RET_USAGE;
>>> +
>>> +       mmc_dev = simple_strtoul(argv[1], NULL, 16);
>>> +
>>> +       if (avb_ops)
>>> +               avb_ops_free(avb_ops);
>>> +
>>> +       avb_ops = avb_ops_alloc(mmc_dev);
>>> +       if (avb_ops)
>>> +               return CMD_RET_SUCCESS;
>>> +
>>> +       return CMD_RET_FAILURE;
>>> +}
>>> +
>>> +int do_avb_read_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>> +{
>>> +       const char *part;
>>> +       s64 offset;
>>> +       size_t bytes, bytes_read = 0;
>>> +       void *buffer;
>>> +
>>> +       if (!avb_ops) {
>>> +               printf("AVB 2.0 is not initialized, please run 'avb init'\n");
>>> +               return CMD_RET_USAGE;
>>> +       }
>>> +
>>> +       if (argc != 5)
>>> +               return CMD_RET_USAGE;
>>> +
>>> +       part = argv[1];
>>> +       offset = simple_strtoul(argv[2], NULL, 16);
>>> +       bytes = simple_strtoul(argv[3], NULL, 16);
>>> +       buffer = (void *)simple_strtoul(argv[4], NULL, 16);
>>> +
>>> +       if (avb_ops->read_from_partition(avb_ops, part, offset, bytes,
>>> +                                        buffer, &bytes_read) ==
>>> +                                        AVB_IO_RESULT_OK) {
>>
>> Please can you make sure this uses driver model, and put wrappers for
>> these function calls in the uclass?

Regards,
Simon