[U-Boot] [PATCH] cmd: avb: Fix requested partitions list
The requested_partitions[] array should contain only boot partitions. Usually it's only 'boot' partition, as can be seen in [1]. Also, seems like the requested_partitions[] are only used when there is no 'vbmeta' partition [2], which is not a regular use-case.
Make requested_partitions[] contain only 'boot' partition as it was supposed to be, and also make that array to be a local in do_avb_verify_part() function, as nobody else needs that.
[1] https://android.googlesource.com/platform/external/avb/+/master/test/avb_slo... [2] https://android.googlesource.com/platform/external/avb/+/master/libavb/avb_s...
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org --- cmd/avb.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/cmd/avb.c b/cmd/avb.c index d1942d6605..8f2bb85fce 100644 --- a/cmd/avb.c +++ b/cmd/avb.c @@ -14,11 +14,6 @@ #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; @@ -231,6 +226,7 @@ int do_avb_get_uuid(cmd_tbl_t *cmdtp, int flag, int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) { + const char * const requested_partitions[] = {"boot", NULL}; AvbSlotVerifyResult slot_result; AvbSlotVerifyData *out_data; char *cmdline;
Hi Sam,
On Fri, Aug 9, 2019 at 7:16 PM Sam Protsenko semen.protsenko@linaro.org wrote:
The requested_partitions[] array should contain only boot partitions. Usually it's only 'boot' partition, as can be seen in [1]. Also, seems like the requested_partitions[] are only used when there is no 'vbmeta' partition [2], which is not a regular use-case.
Make requested_partitions[] contain only 'boot' partition as it was supposed to be, and also make that array to be a local in do_avb_verify_part() function, as nobody else needs that.
[1] https://android.googlesource.com/platform/external/avb/+/master/test/avb_slo... [2] https://android.googlesource.com/platform/external/avb/+/master/libavb/avb_s...
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
cmd/avb.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/cmd/avb.c b/cmd/avb.c index d1942d6605..8f2bb85fce 100644 --- a/cmd/avb.c +++ b/cmd/avb.c @@ -14,11 +14,6 @@ #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; @@ -231,6 +226,7 @@ int do_avb_get_uuid(cmd_tbl_t *cmdtp, int flag, int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) {
const char * const requested_partitions[] = {"boot", NULL}; AvbSlotVerifyResult slot_result; AvbSlotVerifyData *out_data; char *cmdline;-- 2.20.1
Thanks for the patch.
Current snapshot of libavb in U-boot is a bit out-dated (~2 years) and before introducing patches that can leverage new features from the mainline libavb(taking into account that you're referring to Jul 30, 2019 patch 36d41d9223 ("Add AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION flag")) it makes sense to update libavb in U-boot first:
The best approach I see here (just to summarize what we've already discussed): 1. Update libavb to the latest stable version 2. Check if `avb verify` still behaves as expected on non-AB setups. 3. Introduce support of AB slots and different fixes like this one
Thanks
Hi all,
On Mon, Aug 12, 2019 at 02:13:55PM +0300, Igor Opaniuk wrote: [..]
Current snapshot of libavb in U-boot is a bit out-dated (~2 years) and before introducing patches that can leverage new features from the mainline libavb(taking into account that you're referring to Jul 30, 2019 patch 36d41d9223 ("Add AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION flag")) it makes sense to update libavb in U-boot first:
The best approach I see here (just to summarize what we've already discussed):
- Update libavb to the latest stable version
FWIW, this sounds good. jFYI, U-Boot seems to contain the android-o-mr1-iot-preview-8 version of libavb (heuristics applied since commit [1] didn't include any information about the version).
FWIW, in case of an alignment to AOSP, the list of libavb commits to be backported is shown in [2]. The diff stats is visible in [3].
FWIW w.r.t. integration strategy, we better use 'git cherry-pick --strategy=subtree -Xsubtree=lib/' instead of manual copying the libavb contents from https://android.googlesource.com/platform/external/avb/, since the latter would overwrite local U-Boot fixes like commit [4].
[1] https://gitlab.denx.de/u-boot/u-boot/commit/d8f9d2af96b38f ("avb2.0: add Android Verified Boot 2.0 library")
[2] avb (master) git log --oneline --no-merges --no-decorate \ android-o-mr1-iot-preview-8..5fbb42a189aa -- libavb 36d41d922380 Add AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION flag. 3deffc556cfa Fix the identification of vbmeta partition 2ea684dbd612 Adds avb_strncmp helper 31f5f91352e6 Libavb: Check rollback index location against 0. 0a4e345d73f9 Fix a bug that may cause inifinite loop. d91e04e2c163 Fix a bug that would cause OoB read. 18b535905bab Fix a stack-use-after-scope bug. 5786fa2f2cfa Fix a memory leak bug. a6c9ad41f779 Fix a bug that would cause OoB memory read. 9ba3b6613b4e Fix AvbSlotVerifyData->cmdline might be NULL 5abd6bc25789 Allow system partition to be absent 8127dae6e4f5 Only automatically initialize new persistent digest values when locked 4f137c38f838 libavb: Use size_t instead of uint32_t for lengths in avb_sha*() functions. 818cf5674077 libavb: Only query partition GUIDs when the cmdline needs them. 49936b4c0109 libavb: Support vbmeta blobs in beginning of partition. 6f4de3181429 libavb: Add ERROR_MODE_MANAGED_RESTART_AND_EIO for managing dm-verity state. 2367b46287bd Implement initialization of new persistent digest values
[3] avb (master) git diff --shortstat \ android-o-mr1-iot-preview-8..5fbb42a189aa -- libavb 12 files changed, 704 insertions(+), 208 deletions(-)
[4] https://gitlab.denx.de/u-boot/u-boot/commit/ecc6f6bea6a2 ("libavb: Handle wrong hashtree_error_mode in avb_append_options()")
Hi Eugeniu,
On Mon, Aug 12, 2019 at 4:57 PM Eugeniu Rosca erosca@de.adit-jv.com wrote:
Hi all,
On Mon, Aug 12, 2019 at 02:13:55PM +0300, Igor Opaniuk wrote: [..]
Current snapshot of libavb in U-boot is a bit out-dated (~2 years) and before introducing patches that can leverage new features from the mainline libavb(taking into account that you're referring to Jul 30, 2019 patch 36d41d9223 ("Add AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION flag")) it makes sense to update libavb in U-boot first:
The best approach I see here (just to summarize what we've already discussed):
- Update libavb to the latest stable version
FWIW, this sounds good. jFYI, U-Boot seems to contain the android-o-mr1-iot-preview-8 version of libavb (heuristics applied since commit [1] didn't include any information about the version).
FWIW, in case of an alignment to AOSP, the list of libavb commits to be backported is shown in [2]. The diff stats is visible in [3].
FWIW w.r.t. integration strategy, we better use 'git cherry-pick --strategy=subtree -Xsubtree=lib/' instead of manual copying the libavb contents from https://android.googlesource.com/platform/external/avb/, since the latter would overwrite local U-Boot fixes like commit [4].
Wow, that's some real jedi git skills here :) Thanks, Eugeniu, will use that when porting most recent libavb to U-Boot.
[1] https://gitlab.denx.de/u-boot/u-boot/commit/d8f9d2af96b38f ("avb2.0: add Android Verified Boot 2.0 library")
[2] avb (master) git log --oneline --no-merges --no-decorate \ android-o-mr1-iot-preview-8..5fbb42a189aa -- libavb 36d41d922380 Add AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION flag. 3deffc556cfa Fix the identification of vbmeta partition 2ea684dbd612 Adds avb_strncmp helper 31f5f91352e6 Libavb: Check rollback index location against 0. 0a4e345d73f9 Fix a bug that may cause inifinite loop. d91e04e2c163 Fix a bug that would cause OoB read. 18b535905bab Fix a stack-use-after-scope bug. 5786fa2f2cfa Fix a memory leak bug. a6c9ad41f779 Fix a bug that would cause OoB memory read. 9ba3b6613b4e Fix AvbSlotVerifyData->cmdline might be NULL 5abd6bc25789 Allow system partition to be absent 8127dae6e4f5 Only automatically initialize new persistent digest values when locked 4f137c38f838 libavb: Use size_t instead of uint32_t for lengths in avb_sha*() functions. 818cf5674077 libavb: Only query partition GUIDs when the cmdline needs them. 49936b4c0109 libavb: Support vbmeta blobs in beginning of partition. 6f4de3181429 libavb: Add ERROR_MODE_MANAGED_RESTART_AND_EIO for managing dm-verity state. 2367b46287bd Implement initialization of new persistent digest values
[3] avb (master) git diff --shortstat \ android-o-mr1-iot-preview-8..5fbb42a189aa -- libavb 12 files changed, 704 insertions(+), 208 deletions(-)
[4] https://gitlab.denx.de/u-boot/u-boot/commit/ecc6f6bea6a2 ("libavb: Handle wrong hashtree_error_mode in avb_append_options()")
-- Best Regards, Eugeniu.
Hi Sam,
On Fri, Aug 09, 2019 at 07:16:03PM +0300, Sam Protsenko wrote:
The requested_partitions[] array should contain only boot partitions. Usually it's only 'boot' partition, as can be seen in [1]. Also, seems like the requested_partitions[] are only used when there is no 'vbmeta' partition [2], which is not a regular use-case.
Make requested_partitions[] contain only 'boot' partition as it was supposed to be, and also make that array to be a local in do_avb_verify_part() function, as nobody else needs that.
[1] https://android.googlesource.com/platform/external/avb/+/master/test/avb_slo... [2] https://android.googlesource.com/platform/external/avb/+/master/libavb/avb_s...
The patches are much appreciated. Could we agree to avoid volatile references in the links, since those will point out to wrong lines after a couple of weeks? I think it's safer to either use the latest available commit id or tag, e.g.:
[1] https://android.googlesource.com/platform/external/avb/+/5fbb42a189aa/test/a... [2] https://android.googlesource.com/platform/external/avb/+/5fbb42a189aa/libavb...
Thank you.
Hi Eugeniu,
On Tue, Aug 13, 2019 at 7:59 PM Eugeniu Rosca erosca@de.adit-jv.com wrote:
Hi Sam,
On Fri, Aug 09, 2019 at 07:16:03PM +0300, Sam Protsenko wrote:
The requested_partitions[] array should contain only boot partitions. Usually it's only 'boot' partition, as can be seen in [1]. Also, seems like the requested_partitions[] are only used when there is no 'vbmeta' partition [2], which is not a regular use-case.
Make requested_partitions[] contain only 'boot' partition as it was supposed to be, and also make that array to be a local in do_avb_verify_part() function, as nobody else needs that.
[1] https://android.googlesource.com/platform/external/avb/+/master/test/avb_slo... [2] https://android.googlesource.com/platform/external/avb/+/master/libavb/avb_s...
The patches are much appreciated. Could we agree to avoid volatile references in the links, since those will point out to wrong lines after a couple of weeks? I think it's safer to either use the latest available commit id or tag, e.g.:
[1] https://android.googlesource.com/platform/external/avb/+/5fbb42a189aa/test/a... [2] https://android.googlesource.com/platform/external/avb/+/5fbb42a189aa/libavb...
Sure, will send v2 soon. Can you please review this patch [1] please? Without this AVB doesn't work (at least on X15 board, but I presume it might affect more platforms, as code I'm fixing in that patch is common). Also I send [2] for slots support in AVB. Will appreciate your review.
Thanks!
[1] https://patchwork.ozlabs.org/patch/1147191/ [2] https://patchwork.ozlabs.org/patch/1144646/
Thank you.
-- Best Regards, Eugeniu.
On Thu, Aug 15, 2019 at 8:55 PM Sam Protsenko semen.protsenko@linaro.org wrote:
Hi Eugeniu,
On Tue, Aug 13, 2019 at 7:59 PM Eugeniu Rosca erosca@de.adit-jv.com wrote:
Hi Sam,
On Fri, Aug 09, 2019 at 07:16:03PM +0300, Sam Protsenko wrote:
The requested_partitions[] array should contain only boot partitions. Usually it's only 'boot' partition, as can be seen in [1]. Also, seems like the requested_partitions[] are only used when there is no 'vbmeta' partition [2], which is not a regular use-case.
Make requested_partitions[] contain only 'boot' partition as it was supposed to be, and also make that array to be a local in do_avb_verify_part() function, as nobody else needs that.
[1] https://android.googlesource.com/platform/external/avb/+/master/test/avb_slo... [2] https://android.googlesource.com/platform/external/avb/+/master/libavb/avb_s...
The patches are much appreciated. Could we agree to avoid volatile references in the links, since those will point out to wrong lines after a couple of weeks? I think it's safer to either use the latest available commit id or tag, e.g.:
[1] https://android.googlesource.com/platform/external/avb/+/5fbb42a189aa/test/a... [2] https://android.googlesource.com/platform/external/avb/+/5fbb42a189aa/libavb...
Sure, will send v2 soon. Can you please review this patch [1] please? Without this AVB doesn't work (at least on X15 board, but I presume it might affect more platforms, as code I'm fixing in that patch is common). Also I send [2] for slots support in AVB. Will appreciate your review.
Thanks!
[1] https://patchwork.ozlabs.org/patch/1147191/ [2] https://patchwork.ozlabs.org/patch/1144646/
Thank you.
-- Best Regards, Eugeniu.
Acked-by: Igor Opaniuk igor.opaniuk@gmail.com
participants (3)
-
Eugeniu Rosca -
Igor Opaniuk -
Sam Protsenko