From 02f45f51a7e80bf7e62c3fa0926c3e30ccd7124d Mon Sep 17 00:00:00 2001 From: Joel Kitching Date: Mon, 16 Mar 2020 16:20:02 +0800 Subject: vboot: stop using StrnAppend and Uint64ToString Use snprintf instead. Remove utility_string library. Also, prepare VbDisplayDebugInfo to handle 64-byte nvdata. BUG=b:124141368, chromium:968464 TEST=make clean && make runtests TEST=boot with 16-byte nvdata, check output (one line) TEST=boot with 64-byte nvdata, check output (five lines) BRANCH=none Change-Id: If6c4b3a4e9fa7b71cb2d8ca7ccd37e4f36b97fd6 Signed-off-by: Joel Kitching Cq-Depend: chromium:2122061 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2104880 Tested-by: Joel Kitching Commit-Queue: Joel Kitching Reviewed-by: Julius Werner --- firmware/2lib/include/2sysincludes.h | 1 + firmware/include/vboot_api.h | 2 +- firmware/lib/cgptlib/cgptlib.c | 1 - firmware/lib/cgptlib/cgptlib_internal.c | 1 - firmware/lib/gpt_misc.c | 1 - firmware/lib/include/utility.h | 39 ------------ firmware/lib/tpm2_lite/marshaling.c | 1 - firmware/lib/tpm2_lite/tlcl.c | 2 +- firmware/lib/tpm_lite/mocked_tlcl.c | 1 - firmware/lib/tpm_lite/tlcl.c | 1 - firmware/lib/utility_string.c | 72 ---------------------- firmware/lib/vboot_api_kernel.c | 1 - firmware/lib/vboot_audio.c | 1 - firmware/lib/vboot_display.c | 102 +++++++++++-------------------- firmware/lib/vboot_kernel.c | 1 - firmware/lib/vboot_ui_legacy_clamshell.c | 1 - firmware/lib/vboot_ui_legacy_menu.c | 1 - firmware/stub/tpm_lite_stub.c | 1 - 18 files changed, 37 insertions(+), 193 deletions(-) delete mode 100644 firmware/lib/include/utility.h delete mode 100644 firmware/lib/utility_string.c (limited to 'firmware') diff --git a/firmware/2lib/include/2sysincludes.h b/firmware/2lib/include/2sysincludes.h index c23054a1..06717724 100644 --- a/firmware/2lib/include/2sysincludes.h +++ b/firmware/2lib/include/2sysincludes.h @@ -17,6 +17,7 @@ #include /* For PRIu64 */ #include #include +#include #include #include diff --git a/firmware/include/vboot_api.h b/firmware/include/vboot_api.h index 7e141348..d3e05817 100644 --- a/firmware/include/vboot_api.h +++ b/firmware/include/vboot_api.h @@ -85,7 +85,7 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx, VbSelectAndLoadKernelParams *kparams); /*****************************************************************************/ -/* Timer and delay (first two from utility.h) */ +/* Timer and delay */ #define VB_USEC_PER_MSEC 1000ULL #define VB_MSEC_PER_SEC VB_USEC_PER_MSEC diff --git a/firmware/lib/cgptlib/cgptlib.c b/firmware/lib/cgptlib/cgptlib.c index db704b54..2cc77b4a 100644 --- a/firmware/lib/cgptlib/cgptlib.c +++ b/firmware/lib/cgptlib/cgptlib.c @@ -9,7 +9,6 @@ #include "cgptlib_internal.h" #include "crc32.h" #include "gpt.h" -#include "utility.h" #include "vboot_api.h" int GptInit(GptData *gpt) diff --git a/firmware/lib/cgptlib/cgptlib_internal.c b/firmware/lib/cgptlib/cgptlib_internal.c index 7d214a91..593b1e2c 100644 --- a/firmware/lib/cgptlib/cgptlib_internal.c +++ b/firmware/lib/cgptlib/cgptlib_internal.c @@ -9,7 +9,6 @@ #include "crc32.h" #include "gpt.h" #include "gpt_misc.h" -#include "utility.h" const static int MIN_SECTOR_SIZE = 512; diff --git a/firmware/lib/gpt_misc.c b/firmware/lib/gpt_misc.c index 6d91ec15..ae7f8a23 100644 --- a/firmware/lib/gpt_misc.c +++ b/firmware/lib/gpt_misc.c @@ -9,7 +9,6 @@ #include "cgptlib_internal.h" #include "crc32.h" #include "gpt.h" -#include "utility.h" #include "vboot_api.h" /** diff --git a/firmware/lib/include/utility.h b/firmware/lib/include/utility.h deleted file mode 100644 index bca49828..00000000 --- a/firmware/lib/include/utility.h +++ /dev/null @@ -1,39 +0,0 @@ -/* Copyright (c) 2013 The Chromium OS Authors. All rights reserved. - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - * - * Helper functions/wrappers for memory allocations, manipulation and - * comparison. - */ - -#ifndef VBOOT_REFERENCE_UTILITY_H_ -#define VBOOT_REFERENCE_UTILITY_H_ - -#include "2common.h" -#include "2sysincludes.h" -#include "vboot_api.h" - -/* - * Buffer size required to hold the longest possible output of Uint64ToString() - * - that is, Uint64ToString(~0, 2). - */ -#define UINT64_TO_STRING_MAX 65 - -/** - * Convert a value to a string in the specified radix (2=binary, 10=decimal, - * 16=hex) and store it in , which is chars long. If - * , left-pads the string to at least that width with '0'. - * Returns the length of the stored string, not counting the terminating null. - */ -uint32_t Uint64ToString(char *buf, uint32_t bufsize, uint64_t value, - uint32_t radix, uint32_t zero_pad_width); - -/** - * Concatenate onto , which has space for characters - * including the terminating null. Note that will always be - * null-terminated if > 0. Returns the number of characters used in - * , not counting the terminating null. - */ -uint32_t StrnAppend(char *dest, const char *src, uint32_t destlen); - -#endif /* VBOOT_REFERENCE_UTILITY_H_ */ diff --git a/firmware/lib/tpm2_lite/marshaling.c b/firmware/lib/tpm2_lite/marshaling.c index 90ce04ca..ef3606b9 100644 --- a/firmware/lib/tpm2_lite/marshaling.c +++ b/firmware/lib/tpm2_lite/marshaling.c @@ -6,7 +6,6 @@ #include "2common.h" #include "2sysincludes.h" #include "tpm2_marshaling.h" -#include "utility.h" static uint16_t tpm_tag; /* Depends on the command type. */ static int ph_disabled; /* Platform hierarchy disabled. */ diff --git a/firmware/lib/tpm2_lite/tlcl.c b/firmware/lib/tpm2_lite/tlcl.c index dca8facc..9bacbb6b 100644 --- a/firmware/lib/tpm2_lite/tlcl.c +++ b/firmware/lib/tpm2_lite/tlcl.c @@ -10,7 +10,7 @@ #include "2sysincludes.h" #include "tlcl.h" #include "tpm2_marshaling.h" -#include "utility.h" +#include "vboot_api.h" /* * TODO(chromium:1032930): Originally accessed by including secdata_tpm.h. diff --git a/firmware/lib/tpm_lite/mocked_tlcl.c b/firmware/lib/tpm_lite/mocked_tlcl.c index d0f61201..838b5e7e 100644 --- a/firmware/lib/tpm_lite/mocked_tlcl.c +++ b/firmware/lib/tpm_lite/mocked_tlcl.c @@ -6,7 +6,6 @@ #include "2sysincludes.h" #include "tlcl.h" #include "tlcl_internal.h" -#include "utility.h" #include "vboot_api.h" uint32_t TlclLibInit(void) diff --git a/firmware/lib/tpm_lite/tlcl.c b/firmware/lib/tpm_lite/tlcl.c index b6685bf8..28c0bb63 100644 --- a/firmware/lib/tpm_lite/tlcl.c +++ b/firmware/lib/tpm_lite/tlcl.c @@ -21,7 +21,6 @@ #include "tlcl.h" #include "tlcl_internal.h" #include "tlcl_structures.h" -#include "utility.h" #include "vboot_api.h" /* Sets the size field of a TPM command. */ diff --git a/firmware/lib/utility_string.c b/firmware/lib/utility_string.c deleted file mode 100644 index 299bbaa1..00000000 --- a/firmware/lib/utility_string.c +++ /dev/null @@ -1,72 +0,0 @@ -/* Copyright (c) 2013 The Chromium OS Authors. All rights reserved. - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - * - * String utility functions that need to be built as part of the firmware. - */ - -#include "2sysincludes.h" -#include "utility.h" - -uint32_t Uint64ToString(char *buf, uint32_t bufsize, uint64_t value, - uint32_t radix, uint32_t zero_pad_width) -{ - char ibuf[UINT64_TO_STRING_MAX]; - char *s; - uint32_t usedsize = 1; - - if (!buf) - return 0; - - /* Clear output buffer in case of error */ - *buf = '\0'; - - /* Sanity-check input args */ - if (radix < 2 || radix > 36 || zero_pad_width >= UINT64_TO_STRING_MAX) - return 0; - - /* Start at end of string and work backwards */ - s = ibuf + UINT64_TO_STRING_MAX - 1; - *(s) = '\0'; - do { - int v = value % radix; - value /= radix; - - *(--s) = (char)(v < 10 ? v + '0' : v + 'a' - 10); - if (++usedsize > bufsize) - return 0; /* Result won't fit in buffer */ - } while (value); - - /* Zero-pad if necessary */ - while (usedsize <= zero_pad_width) { - *(--s) = '0'; - if (++usedsize > bufsize) - return 0; /* Result won't fit in buffer */ - } - - /* Now copy the string back to the input buffer. */ - memcpy(buf, s, usedsize); - - /* Don't count the terminating null in the bytes used */ - return usedsize - 1; -} - -uint32_t StrnAppend(char *dest, const char *src, uint32_t destlen) -{ - uint32_t used = 0; - - if (!dest || !src || !destlen) - return 0; - - /* Skip past existing string in destination.*/ - while (dest[used] && used < destlen - 1) - used++; - - /* Now copy source */ - while (*src && used < destlen - 1) - dest[used++] = *src++; - - /* Terminate destination and return count of non-null characters */ - dest[used] = 0; - return used; -} diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c index 4f86046a..6111eee0 100644 --- a/firmware/lib/vboot_api_kernel.c +++ b/firmware/lib/vboot_api_kernel.c @@ -15,7 +15,6 @@ #include "2sysincludes.h" #include "2ui.h" #include "load_kernel_fw.h" -#include "utility.h" #include "vb2_common.h" #include "vboot_api.h" #include "vboot_kernel.h" diff --git a/firmware/lib/vboot_audio.c b/firmware/lib/vboot_audio.c index 20b14376..5e44606d 100644 --- a/firmware/lib/vboot_audio.c +++ b/firmware/lib/vboot_audio.c @@ -8,7 +8,6 @@ #include "2common.h" #include "2misc.h" #include "2sysincludes.h" -#include "utility.h" #include "vboot_api.h" #include "vboot_audio.h" diff --git a/firmware/lib/vboot_display.c b/firmware/lib/vboot_display.c index 258619fb..07e1da0a 100644 --- a/firmware/lib/vboot_display.c +++ b/firmware/lib/vboot_display.c @@ -10,7 +10,6 @@ #include "2nvstorage.h" #include "2sha.h" #include "2sysincludes.h" -#include "utility.h" #include "vboot_api.h" #include "vboot_display.h" #include "vboot_kernel.h" @@ -216,7 +215,12 @@ const char *RecoveryReasonString(uint8_t code) return "Unknown or deprecated error code"; } -#define DEBUG_INFO_SIZE 512 +#define DEBUG_INFO_SIZE 1024 +#define DEBUG_INFO_APPEND(format, args...) do { \ + if (used < DEBUG_INFO_SIZE) \ + used += snprintf(buf + used, DEBUG_INFO_SIZE - used, format, \ + ## args); \ +} while (0) vb2_error_t VbDisplayDebugInfo(struct vb2_context *ctx) { @@ -225,7 +229,7 @@ vb2_error_t VbDisplayDebugInfo(struct vb2_context *ctx) struct vb2_workbuf wb; char buf[DEBUG_INFO_SIZE] = ""; char sha1sum[VB2_SHA1_DIGEST_SIZE * 2 + 1]; - uint32_t used = 0; + int32_t used = 0; vb2_error_t ret; uint32_t i; @@ -238,83 +242,53 @@ vb2_error_t VbDisplayDebugInfo(struct vb2_context *ctx) ret = vb2api_gbb_read_hwid(ctx, hwid, &size); if (ret) strcpy(hwid, "{INVALID}"); - used += StrnAppend(buf + used, "HWID: ", - DEBUG_INFO_SIZE - used); - used += StrnAppend(buf + used, hwid, DEBUG_INFO_SIZE - used); + DEBUG_INFO_APPEND("HWID: %s", hwid); } /* Add recovery reason and subcode */ i = vb2_nv_get(ctx, VB2_NV_RECOVERY_SUBCODE); - used += StrnAppend(buf + used, - "\nrecovery_reason: 0x", DEBUG_INFO_SIZE - used); - used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, - sd->recovery_reason, 16, 2); - used += StrnAppend(buf + used, " / 0x", DEBUG_INFO_SIZE - used); - used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, i, 16, 2); - used += StrnAppend(buf + used, " ", DEBUG_INFO_SIZE - used); - used += StrnAppend(buf + used, - RecoveryReasonString(sd->recovery_reason), - DEBUG_INFO_SIZE - used); + DEBUG_INFO_APPEND("\nrecovery_reason: %#.2x / %#.2x %s", + sd->recovery_reason, i, + RecoveryReasonString(sd->recovery_reason)); /* Add vb2_context and vb2_shared_data flags */ - used += StrnAppend(buf + used, "\ncontext.flags: 0x", - DEBUG_INFO_SIZE - used); - used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, - ctx->flags, 16, 16); - used += StrnAppend(buf + used, "\nshared_data.flags: 0x", - DEBUG_INFO_SIZE - used); - used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, - sd->flags, 16, 8); - used += StrnAppend(buf + used, "\nshared_data.status: 0x", - DEBUG_INFO_SIZE - used); - used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, - sd->status, 16, 8); - - /* Add raw contents of VbNvStorage */ - used += StrnAppend(buf + used, "\nVbNv.raw:", DEBUG_INFO_SIZE - used); + DEBUG_INFO_APPEND("\ncontext.flags: %#.16" PRIx64, ctx->flags); + DEBUG_INFO_APPEND("\nshared_data.flags: %#.8x", sd->flags); + DEBUG_INFO_APPEND("\nshared_data.status: %#.8x", sd->status); + + /* Add raw contents of nvdata */ + DEBUG_INFO_APPEND("\nnvdata:"); + if (vb2_nv_get_size(ctx) > 16) /* Multi-line starts on next line */ + DEBUG_INFO_APPEND("\n "); for (i = 0; i < vb2_nv_get_size(ctx); i++) { - used += StrnAppend(buf + used, " ", DEBUG_INFO_SIZE - used); - used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, - ctx->nvdata[i], 16, 2); + /* Split into 16-byte blocks */ + if (i > 0 && i % 16 == 0) + DEBUG_INFO_APPEND("\n "); + DEBUG_INFO_APPEND(" %02x", ctx->nvdata[i]); } /* Add dev_boot_usb flag */ i = vb2_nv_get(ctx, VB2_NV_DEV_BOOT_USB); - used += StrnAppend(buf + used, "\ndev_boot_usb: ", DEBUG_INFO_SIZE - used); - used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, i, 10, 0); + DEBUG_INFO_APPEND("\ndev_boot_usb: %d", i); /* Add dev_boot_legacy flag */ i = vb2_nv_get(ctx, VB2_NV_DEV_BOOT_LEGACY); - used += StrnAppend(buf + used, - "\ndev_boot_legacy: ", DEBUG_INFO_SIZE - used); - used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, i, 10, 0); + DEBUG_INFO_APPEND("\ndev_boot_legacy: %d", i); /* Add dev_default_boot flag */ i = vb2_nv_get(ctx, VB2_NV_DEV_DEFAULT_BOOT); - used += StrnAppend(buf + used, - "\ndev_default_boot: ", DEBUG_INFO_SIZE - used); - used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, i, 10, 0); + DEBUG_INFO_APPEND("\ndev_default_boot: %d", i); /* Add dev_boot_signed_only flag */ i = vb2_nv_get(ctx, VB2_NV_DEV_BOOT_SIGNED_ONLY); - used += StrnAppend(buf + used, "\ndev_boot_signed_only: ", - DEBUG_INFO_SIZE - used); - used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, i, 10, 0); + DEBUG_INFO_APPEND("\ndev_boot_signed_only: %d", i); /* Add TPM versions */ - used += StrnAppend(buf + used, - "\nTPM: fwver=0x", DEBUG_INFO_SIZE - used); - used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, - sd->fw_version_secdata, 16, 8); - used += StrnAppend(buf + used, " kernver=0x", DEBUG_INFO_SIZE - used); - used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, - sd->kernel_version_secdata, 16, 8); + DEBUG_INFO_APPEND("\nTPM: fwver=%#.8x kernver=%#.8x", + sd->fw_version_secdata, sd->kernel_version_secdata); /* Add GBB flags */ - used += StrnAppend(buf + used, - "\ngbb.flags: 0x", DEBUG_INFO_SIZE - used); - used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, - gbb->flags, 16, 8); + DEBUG_INFO_APPEND("\ngbb.flags: %#.8x", gbb->flags); /* Add sha1sum for Root & Recovery keys */ { @@ -323,10 +297,7 @@ vb2_error_t VbDisplayDebugInfo(struct vb2_context *ctx) ret = vb2_gbb_read_root_key(ctx, &key, NULL, &wblocal); if (!ret) { FillInSha1Sum(sha1sum, key); - used += StrnAppend(buf + used, "\ngbb.rootkey: ", - DEBUG_INFO_SIZE - used); - used += StrnAppend(buf + used, sha1sum, - DEBUG_INFO_SIZE - used); + DEBUG_INFO_APPEND("\ngbb.rootkey: %s", sha1sum); } } @@ -336,10 +307,7 @@ vb2_error_t VbDisplayDebugInfo(struct vb2_context *ctx) ret = vb2_gbb_read_recovery_key(ctx, &key, NULL, &wblocal); if (!ret) { FillInSha1Sum(sha1sum, key); - used += StrnAppend(buf + used, "\ngbb.recovery_key: ", - DEBUG_INFO_SIZE - used); - used += StrnAppend(buf + used, sha1sum, - DEBUG_INFO_SIZE - used); + DEBUG_INFO_APPEND("\ngbb.recovery_key: %s", sha1sum); } } @@ -349,13 +317,11 @@ vb2_error_t VbDisplayDebugInfo(struct vb2_context *ctx) struct vb2_packed_key *key = vb2_member_of(sd, sd->kernel_key_offset); FillInSha1Sum(sha1sum, key); - used += StrnAppend(buf + used, - "\nkernel_subkey: ", DEBUG_INFO_SIZE - used); - used += StrnAppend(buf + used, sha1sum, DEBUG_INFO_SIZE - used); + DEBUG_INFO_APPEND("\nkernel_subkey: %s", sha1sum); } /* Make sure we finish with a newline */ - used += StrnAppend(buf + used, "\n", DEBUG_INFO_SIZE - used); + DEBUG_INFO_APPEND("\n"); /* TODO: add more interesting data: * - Information on current disks */ diff --git a/firmware/lib/vboot_kernel.c b/firmware/lib/vboot_kernel.c index 8118599d..7420a697 100644 --- a/firmware/lib/vboot_kernel.c +++ b/firmware/lib/vboot_kernel.c @@ -17,7 +17,6 @@ #include "cgptlib_internal.h" #include "gpt_misc.h" #include "load_kernel_fw.h" -#include "utility.h" #include "vb2_common.h" #include "vboot_api.h" #include "vboot_kernel.h" diff --git a/firmware/lib/vboot_ui_legacy_clamshell.c b/firmware/lib/vboot_ui_legacy_clamshell.c index 959981dd..c510d58f 100644 --- a/firmware/lib/vboot_ui_legacy_clamshell.c +++ b/firmware/lib/vboot_ui_legacy_clamshell.c @@ -13,7 +13,6 @@ #include "2sysincludes.h" #include "load_kernel_fw.h" #include "tlcl.h" -#include "utility.h" #include "vb2_common.h" #include "vboot_api.h" #include "vboot_audio.h" diff --git a/firmware/lib/vboot_ui_legacy_menu.c b/firmware/lib/vboot_ui_legacy_menu.c index b0cfb43b..48eaabe9 100644 --- a/firmware/lib/vboot_ui_legacy_menu.c +++ b/firmware/lib/vboot_ui_legacy_menu.c @@ -12,7 +12,6 @@ #include "2secdata.h" #include "2sysincludes.h" #include "load_kernel_fw.h" -#include "utility.h" #include "vb2_common.h" #include "vboot_api.h" #include "vboot_audio.h" diff --git a/firmware/stub/tpm_lite_stub.c b/firmware/stub/tpm_lite_stub.c index 5fd673f2..54accb56 100644 --- a/firmware/stub/tpm_lite_stub.c +++ b/firmware/stub/tpm_lite_stub.c @@ -23,7 +23,6 @@ #include "2sysincludes.h" #include "tlcl.h" #include "tlcl_internal.h" -#include "utility.h" #include "vboot_api.h" #define TPM_DEVICE_PATH "/dev/tpm0" -- cgit v1.2.1