Reviewed-by: Igor Opaniuk igor.opaniuk@linaro.org

On 14 August 2018 at 03:43, Eugeniu Rosca roscaeugeniu@gmail.com wrote:

Fix below compiler [1] warning:

common/avb_verify.c: In function ‘avb_find_dm_args’: common/avb_verify.c:179:30: warning: left-hand operand of comma expression has no effect [-Wunused-value] for (i = 0; i < AVB_MAX_ARGS, args[i]; ++i) {

[1] aarch64-linux-gnu-gcc (Linaro GCC 7.2-2017.11)

Signed-off-by: Eugeniu Rosca erosca@de.adit-jv.com

common/avb_verify.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/avb_verify.c b/common/avb_verify.c index 20e35ade3029..e6f3f207ff6f 100644 --- a/common/avb_verify.c +++ b/common/avb_verify.c @@ -176,7 +176,7 @@ static int avb_find_dm_args(char **args, char *str) if (!str) return -1;

•   for (i = 0; i < AVB_MAX_ARGS, args[i]; ++i) {
  

•   for (i = 0; i < AVB_MAX_ARGS && args[i]; ++i) {
            if (strstr(args[i], str))
                    return i;
    }
  

-- 2.18.0

Hi Eugeniu, thanks for fixing this!

BTW, I plan to replace this workaround with hardcoded public RSA key for verification of vbmeta image signature, and an option to store it in RPMB using AVB TA/OP-TEE instead, just after Jens's Wiklander patches (that he sent last Friday) are merged.

Reviewed-by: Igor Opaniuk igor.opaniuk@linaro.org

On 14 August 2018 at 03:43, Eugeniu Rosca roscaeugeniu@gmail.com wrote:

Fix sparse complaint:

common/avb_verify.c:14:21: warning: \ symbol 'avb_root_pub' was not declared. Should it be static?

Signed-off-by: Eugeniu Rosca erosca@de.adit-jv.com

common/avb_verify.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/avb_verify.c b/common/avb_verify.c index e6f3f207ff6f..9c90e1b4ae5c 100644 --- a/common/avb_verify.c +++ b/common/avb_verify.c @@ -11,7 +11,7 @@ #include <malloc.h> #include <part.h>

-const unsigned char avb_root_pub[1032] = { +static const unsigned char avb_root_pub[1032] = { 0x0, 0x0, 0x10, 0x0, 0x55, 0xd9, 0x4, 0xad, 0xd8, 0x4, 0xaf, 0xe3, 0xd3, 0x84, 0x6c, 0x7e, 0xd, 0x89, 0x3d, 0xc2, 0x8c, 0xd3, 0x12, 0x55, 0xe9, 0x62, 0xc9, 0xf1, 0xf, 0x5e, -- 2.18.0

Reviewed-by: Igor Opaniuk igor.opaniuk@linaro.org

On 14 August 2018 at 03:43, Eugeniu Rosca roscaeugeniu@gmail.com wrote:

Cppcheck (v1.85) reports w/o this patch:

[common/avb_verify.c:351]: (error) Memory leak: part [common/avb_verify.c:356]: (error) Memory leak: part [common/avb_verify.c:361]: (error) Memory leak: part [common/avb_verify.c:366]: (error) Memory leak: part

Signed-off-by: Eugeniu Rosca erosca@de.adit-jv.com

common/avb_verify.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/common/avb_verify.c b/common/avb_verify.c index 9c90e1b4ae5c..58cfa1aa7de8 100644 --- a/common/avb_verify.c +++ b/common/avb_verify.c @@ -348,34 +348,37 @@ static struct mmc_part *get_partition(AvbOps *ops, const char *partition) part->mmc = find_mmc_device(dev_num); if (!part->mmc) { printf("No MMC device at slot %x\n", dev_num);

•           return NULL;
  

•           goto err;
    }
  
    if (mmc_init(part->mmc)) {
            printf("MMC initialization failed\n");
  

•           return NULL;
  

•           goto err;
    }
  
    ret = mmc_switch_part(part->mmc, part_num);
    if (ret)
  

•           return NULL;
  

•           goto err;
  
    mmc_blk = mmc_get_blk_desc(part->mmc);
    if (!mmc_blk) {
            printf("Error - failed to obtain block descriptor\n");
  

•           return NULL;
  

•           goto err;
    }
  
    ret = part_get_info_by_name(mmc_blk, partition, &part->info);
    if (!ret) {
            printf("Can't find partition '%s'\n", partition);
  

•           return NULL;
  

•           goto err;
    }
  
    part->dev_num = dev_num;
    part->mmc_blk = mmc_blk;
  
    return part;
  

+err:

•   free(part);
  

•   return NULL;
  

}

static AvbIOResult mmc_byte_io(AvbOps *ops,

2.18.0

Thanks for fixing this!

Reviewed-by: Igor Opaniuk igor.opaniuk@linaro.org

On 14 August 2018 at 03:43, Eugeniu Rosca roscaeugeniu@gmail.com wrote:

Compiling U-Boot with ubsan/asan libraries and running it in sandbox may lead to below backtrace:

=> avb init 0 => avb verify ## Android Verified Boot 2.0 version 1.1.0 read_is_device_unlocked not supported yet common/avb_verify.c:407:31: runtime error: division by zero AddressSanitizer:DEADLYSIGNAL ================================================================= ==9388==ERROR: AddressSanitizer: FPE on unknown address 0x0000004b467f \ (pc 0x0000004b467f bp 0x000000000000 sp 0x7ffd899fe150 T0) #0 0x4b467e in mmc_byte_io common/avb_verify.c:407 #1 0x4b4c47 in mmc_byte_io common/avb_verify.c:532 #2 0x4b4c47 in read_from_partition common/avb_verify.c:533 #3 0x69dc0d in load_and_verify_vbmeta lib/libavb/avb_slot_verify.c:560 #4 0x6a1ee6 in avb_slot_verify lib/libavb/avb_slot_verify.c:1139 #5 0x45dabd in do_avb_verify_part cmd/avb.c:245 #6 0x4af77c in cmd_call common/command.c:499 #7 0x4af77c in cmd_process common/command.c:538 #8 0x46bafc in run_pipe_real common/cli_hush.c:1677 #9 0x46bafc in run_list_real common/cli_hush.c:1875 #10 0x46c780 in run_list common/cli_hush.c:2024 #11 0x46c780 in parse_stream_outer common/cli_hush.c:3216 #12 0x46d34b in parse_file_outer common/cli_hush.c:3299 #13 0x4ad609 in cli_loop common/cli.c:217 #14 0x4625ae in main_loop common/main.c:65 #15 0x46f2d1 in run_main_loop common/board_r.c:648 #16 0x640253 in initcall_run_list lib/initcall.c:30 #17 0x46f9d0 in board_init_r common/board_r.c:879 #18 0x40539b in main arch/sandbox/cpu/start.c:321 #19 0x7fa94925f82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f) #20 0x408908 in _start (/srv/R/u-boot-master/u-boot+0x408908)

AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: FPE common/avb_verify.c:407 in mmc_byte_io ==9388==ABORTING

Signed-off-by: Eugeniu Rosca erosca@de.adit-jv.com

common/avb_verify.c | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/common/avb_verify.c b/common/avb_verify.c index 3d2b4cbad92d..759df7bd25c0 100644 --- a/common/avb_verify.c +++ b/common/avb_verify.c @@ -402,6 +402,9 @@ static AvbIOResult mmc_byte_io(AvbOps *ops, if (!part) return AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION;

•   if (!part->info.blksz)
  

•           return AVB_IO_RESULT_ERROR_IO;
  

•   start_offset = calc_offset(part, offset);
    while (num_bytes) {
            start_sector = start_offset / part->info.blksz;
  

-- 2.18.0