[U-Boot] [PATCH 4/8] cmd: avb2.0: avb command for performing verification
Igor Opaniuk
igor.opaniuk at linaro.org
Tue May 15 17:31:46 UTC 2018
On 15 May 2018 at 19:26, Simon Glass <sjg at chromium.org> wrote:
> 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.
Right, I do understand what DriverModel is and why it should
be used in the case of real-device drivers. But regarding Android Verified
Boot 2.0 feature, which introduces verification capabilities
and leverages only device-independent APIs,
I see no reason why it should be used here.
Could you please check [1] and confirm that this set of commands should
really follow this model.
> Also can you please drop the CamelCase in the patches? We don't use
> that in U-Boot.
Frankly, I don't like it also, bit all CamelCase instances in
the code relate to libavb/libavb_ab library ([2]), which is planned to be
deviated from AOSP upstream in the future.
That's why a decision was made to not introduce any changes to simplify this
process, as Google intensively introduces new changes to it.
(I've left a notice in the cover letter that checkpatch
will fail on the commit, which introduces libavb; also, there is the ongoing
discussion there regarding why libavb/libavb_ab should be
kept as it's. Please join us, if you don't mind).
Thanks!
Best regards,
Igor
[1] https://android.googlesource.com/platform/external/avb/+/master/README.md
[2] https://android.googlesource.com/platform/external/avb/
>>
>> 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
--
Regards,
Igor Opaniuk
More information about the U-Boot
mailing list