[prev in list] [next in list] [prev in thread] [next in thread]
List: fwts-devel
Subject: ACK: [PATCH] acpi: refactor all table length checks to fwts_acpi_table_length_check
From: Colin Ian King <colin.king () canonical ! com>
Date: 2019-02-26 19:05:21
Message-ID: 5c321bca-73d4-c146-89bb-d2a0d59512bb () canonical ! com
[Download RAW message or body]
On 26/02/2019 19:03, Alex Hung wrote:
> Signed-off-by: Alex Hung <alex.hung@canonical.com>
> ---
> src/acpi/aspt/aspt.c | 7 +------
> src/acpi/boot/boot.c | 7 +------
> src/acpi/dbg2/dbg2.c | 7 +------
> src/acpi/dbgp/dbgp.c | 7 +------
> src/acpi/fpdt/fpdt.c | 8 +-------
> src/acpi/hest/hest.c | 8 +-------
> src/acpi/mchi/mchi.c | 7 +------
> src/acpi/msdm/msdm.c | 7 +------
> src/acpi/slic/slic.c | 7 +------
> src/acpi/slit/slit.c | 7 +------
> src/acpi/spmi/spmi.c | 7 +------
> src/acpi/uefi/uefi.c | 7 +------
> src/acpi/waet/waet.c | 7 +------
> src/lib/include/fwts_acpi_tables.h | 1 +
> src/lib/src/fwts_acpi_tables.c | 23 +++++++++++++++++++++++
> 15 files changed, 37 insertions(+), 80 deletions(-)
>
> diff --git a/src/acpi/aspt/aspt.c b/src/acpi/aspt/aspt.c
> index a2a438b1..7e64e06a 100644
> --- a/src/acpi/aspt/aspt.c
> +++ b/src/acpi/aspt/aspt.c
> @@ -52,13 +52,8 @@ static int aspt_test1(fwts_framework *fw)
> bool passed = true;
> fwts_acpi_table_aspt *aspt = (fwts_acpi_table_aspt *)table->data;
>
> - if (table->length < sizeof(fwts_acpi_table_aspt)) {
> + if (!fwts_acpi_table_length_check(fw, "ASPT", table->length, \
> sizeof(fwts_acpi_table_aspt))) { passed = false;
> - fwts_failed(fw, LOG_LEVEL_HIGH,
> - "ASPTTooShort",
> - "ASPT table too short, expecting %zu bytes, "
> - "instead got %zu bytes",
> - sizeof(fwts_acpi_table_aspt), table->length);
> goto done;
> }
>
> diff --git a/src/acpi/boot/boot.c b/src/acpi/boot/boot.c
> index 1fb9d0e2..081b8b37 100644
> --- a/src/acpi/boot/boot.c
> +++ b/src/acpi/boot/boot.c
> @@ -52,13 +52,8 @@ static int boot_test1(fwts_framework *fw)
> bool passed = true;
> fwts_acpi_table_boot *boot = (fwts_acpi_table_boot *)table->data;
>
> - if (table->length < sizeof(fwts_acpi_table_boot)) {
> + if (!fwts_acpi_table_length_check(fw, "BOOT", table->length, \
> sizeof(fwts_acpi_table_boot))) { passed = false;
> - fwts_failed(fw, LOG_LEVEL_HIGH,
> - "BOOTTooShort",
> - "BOOT table too short, expecting %zu bytes, "
> - "instead got %zu bytes",
> - sizeof(fwts_acpi_table_boot), table->length);
> goto done;
> }
>
> diff --git a/src/acpi/dbg2/dbg2.c b/src/acpi/dbg2/dbg2.c
> index 45eb96a8..48e82601 100644
> --- a/src/acpi/dbg2/dbg2.c
> +++ b/src/acpi/dbg2/dbg2.c
> @@ -148,13 +148,8 @@ static int dbg2_test1(fwts_framework *fw)
> size_t total_size;
>
> /* Enough length for the initial dbg2 header? */
> - if (table->length < sizeof(fwts_acpi_table_dbg2)) {
> + if (!fwts_acpi_table_length_check(fw, "DBG2", table->length, \
> sizeof(fwts_acpi_table_dbg2))) { passed = false;
> - fwts_failed(fw, LOG_LEVEL_HIGH,
> - "DBG2TooShort",
> - "DBG2 table too short, expecting %zu bytes, "
> - "instead got %zu bytes",
> - sizeof(fwts_acpi_table_dbg2), table->length);
> goto done;
> }
>
> diff --git a/src/acpi/dbgp/dbgp.c b/src/acpi/dbgp/dbgp.c
> index 7e85e99b..41e6bb2b 100644
> --- a/src/acpi/dbgp/dbgp.c
> +++ b/src/acpi/dbgp/dbgp.c
> @@ -53,13 +53,8 @@ static int dbgp_test1(fwts_framework *fw)
> char *interface_type;
> fwts_acpi_table_dbgp *dbgp = (fwts_acpi_table_dbgp *)table->data;
>
> - if (table->length < sizeof(fwts_acpi_table_dbgp)) {
> + if (!fwts_acpi_table_length_check(fw, "DBGP", table->length, \
> sizeof(fwts_acpi_table_dbgp))) { passed = false;
> - fwts_failed(fw, LOG_LEVEL_HIGH,
> - "DBGPTooShort",
> - "DBGP table too short, expecting %zu bytes, "
> - "instead got %zu bytes",
> - sizeof(fwts_acpi_table_dbgp), table->length);
> goto done;
> }
>
> diff --git a/src/acpi/fpdt/fpdt.c b/src/acpi/fpdt/fpdt.c
> index e15d019b..a4924e3a 100644
> --- a/src/acpi/fpdt/fpdt.c
> +++ b/src/acpi/fpdt/fpdt.c
> @@ -80,14 +80,8 @@ static int fpdt_test1(fwts_framework *fw)
> const size_t fpdt_hdr_len = sizeof(fwts_acpi_table_fpdt_header);
>
> /* Size sanity, got enough table to get initial header */
> - if (table->length < sizeof(fwts_acpi_table_header)) {
> + if (!fwts_acpi_table_length_check(fw, "FPDT", table->length, \
> sizeof(fwts_acpi_table_header))) { passed = false;
> - fwts_failed(fw, LOG_LEVEL_HIGH,
> - "FPDTTooShort",
> - "FPDT table too short, must be at least %zu bytes, "
> - "instead got %zu bytes",
> - sizeof(fwts_acpi_table_header),
> - table->length);
> goto done;
> }
>
> diff --git a/src/acpi/hest/hest.c b/src/acpi/hest/hest.c
> index 0a3b47f5..c6da557d 100644
> --- a/src/acpi/hest/hest.c
> +++ b/src/acpi/hest/hest.c
> @@ -801,14 +801,8 @@ static int hest_test1(fwts_framework *fw)
> hest_type_02_count = 0,
> hest_type_11_count = 0;
>
> -
> - if (table->length < sizeof(fwts_acpi_table_hest)) {
> + if (!fwts_acpi_table_length_check(fw, "HEST", table->length, \
> sizeof(fwts_acpi_table_hest))) { passed = false;
> - fwts_failed(fw, LOG_LEVEL_HIGH,
> - "HESTTooShort",
> - "HEST table too short, expecting %zu bytes, "
> - "instead got %zu bytes",
> - sizeof(fwts_acpi_table_hest), table->length);
> goto done;
> }
>
> diff --git a/src/acpi/mchi/mchi.c b/src/acpi/mchi/mchi.c
> index d38899c9..120c6f4f 100644
> --- a/src/acpi/mchi/mchi.c
> +++ b/src/acpi/mchi/mchi.c
> @@ -54,13 +54,8 @@ static int mchi_test1(fwts_framework *fw)
> bool passed = true;
> fwts_acpi_table_mchi *mchi = (fwts_acpi_table_mchi *)table->data;
>
> - if (table->length < sizeof(fwts_acpi_table_mchi)) {
> + if (!fwts_acpi_table_length_check(fw, "MCHI", table->length, \
> sizeof(fwts_acpi_table_mchi))) { passed = false;
> - fwts_failed(fw, LOG_LEVEL_HIGH,
> - "MCHITooShort",
> - "MCHI table too short, expecting %zu bytes, "
> - "instead got %zu bytes",
> - sizeof(fwts_acpi_table_mchi), table->length);
> goto done;
> }
>
> diff --git a/src/acpi/msdm/msdm.c b/src/acpi/msdm/msdm.c
> index ed476d3c..fb7bb771 100644
> --- a/src/acpi/msdm/msdm.c
> +++ b/src/acpi/msdm/msdm.c
> @@ -55,13 +55,8 @@ static int msdm_test1(fwts_framework *fw)
> bool passed = true;
>
> /* Size sanity check #1, got enough table to get initial header */
> - if (table->length < sizeof(fwts_acpi_table_msdm)) {
> + if (!fwts_acpi_table_length_check(fw, "MSDM", table->length, \
> sizeof(fwts_acpi_table_msdm))) { passed = false;
> - fwts_failed(fw, LOG_LEVEL_HIGH,
> - "MSDMTooShort",
> - "MSDM table too short, must be at least %zu bytes, "
> - "instead got %zu bytes",
> - sizeof(fwts_acpi_table_msdm), table->length);
> goto done;
> }
> fwts_log_info_verbatim(fw, " Reserved: 0x%8.8" PRIx32, \
> msdm->reserved);
> diff --git a/src/acpi/slic/slic.c b/src/acpi/slic/slic.c
> index 21bd7976..4c947694 100644
> --- a/src/acpi/slic/slic.c
> +++ b/src/acpi/slic/slic.c
> @@ -59,13 +59,8 @@ static int slic_test1(fwts_framework *fw)
> size_t length = slic_min_hdr_size;
>
> /* Size sanity check #1, got enough table to get initial header */
> - if (table->length < slic_min_hdr_size) {
> + if (!fwts_acpi_table_length_check(fw, "SLIC", table->length, slic_min_hdr_size)) \
> { passed = false;
> - fwts_failed(fw, LOG_LEVEL_HIGH,
> - "SLICTooShort",
> - "SLIC table too short, must be at least %zu bytes, "
> - "instead got %zu bytes",
> - slic_min_hdr_size, table->length);
> goto done;
> }
> ptr = (uint8_t *)table->data + sizeof(fwts_acpi_table_header);
> diff --git a/src/acpi/slit/slit.c b/src/acpi/slit/slit.c
> index a365cba1..ef84a807 100644
> --- a/src/acpi/slit/slit.c
> +++ b/src/acpi/slit/slit.c
> @@ -57,13 +57,8 @@ static int slit_test1(fwts_framework *fw)
> fwts_acpi_table_slit *slit = (fwts_acpi_table_slit *)table->data;
>
> /* Size sanity check #1, got enough table to at least get matrix size */
> - if (table->length < sizeof(fwts_acpi_table_slit)) {
> + if (!fwts_acpi_table_length_check(fw, "SLIT", table->length, \
> sizeof(fwts_acpi_table_slit))) { passed = false;
> - fwts_failed(fw, LOG_LEVEL_HIGH,
> - "SLITTooShort",
> - "SLIT table too short, must be at least %zu bytes, "
> - "instead got %zu bytes",
> - sizeof(fwts_acpi_table_spmi), table->length);
> goto done;
> }
>
> diff --git a/src/acpi/spmi/spmi.c b/src/acpi/spmi/spmi.c
> index cac580a5..5ee2d2ac 100644
> --- a/src/acpi/spmi/spmi.c
> +++ b/src/acpi/spmi/spmi.c
> @@ -55,13 +55,8 @@ static int spmi_test1(fwts_framework *fw)
> bool passed = true;
> fwts_acpi_table_spmi *spmi = (fwts_acpi_table_spmi *)table->data;
>
> - if (table->length < sizeof(fwts_acpi_table_spmi)) {
> + if (!fwts_acpi_table_length_check(fw, "SPMI", table->length, \
> sizeof(fwts_acpi_table_spmi))) { passed = false;
> - fwts_failed(fw, LOG_LEVEL_HIGH,
> - "SPMITooShort",
> - "SPMI table too short, expecting %zu bytes, "
> - "instead got %zu bytes",
> - sizeof(fwts_acpi_table_spmi), table->length);
> goto done;
> }
>
> diff --git a/src/acpi/uefi/uefi.c b/src/acpi/uefi/uefi.c
> index 17779d0a..0569af3f 100644
> --- a/src/acpi/uefi/uefi.c
> +++ b/src/acpi/uefi/uefi.c
> @@ -61,13 +61,8 @@ static int uefi_test1(fwts_framework *fw)
> 0x9d, 0x94, 0xdb, 0x65, 0xac, 0xc5, 0xc3, 0x32 };
>
> /* Enough length for the uefi table? */
> - if (table->length < sizeof(fwts_acpi_table_uefi)) {
> + if (!fwts_acpi_table_length_check(fw, "UEFI", table->length, \
> sizeof(fwts_acpi_table_uefi))) { passed = false;
> - fwts_failed(fw, LOG_LEVEL_HIGH,
> - "UEFITooShort",
> - "UEFI table too short, expecting %zu bytes, "
> - "instead got %zu bytes",
> - sizeof(fwts_acpi_table_uefi), table->length);
> goto done;
> }
>
> diff --git a/src/acpi/waet/waet.c b/src/acpi/waet/waet.c
> index 14481f4f..81002e6a 100644
> --- a/src/acpi/waet/waet.c
> +++ b/src/acpi/waet/waet.c
> @@ -55,13 +55,8 @@ static int waet_test1(fwts_framework *fw)
> fwts_acpi_table_waet *waet = (fwts_acpi_table_waet *)table->data;
>
> /* Enough length for the initial waet header? */
> - if (table->length < sizeof(fwts_acpi_table_waet)) {
> + if (!fwts_acpi_table_length_check(fw, "WAET", table->length, \
> sizeof(fwts_acpi_table_waet))) { passed = false;
> - fwts_failed(fw, LOG_LEVEL_HIGH,
> - "WAETTooShort",
> - "WAET table too short, expecting %zu bytes, "
> - "instead got %zu bytes",
> - sizeof(fwts_acpi_table_waet), table->length);
> goto done;
> }
>
> diff --git a/src/lib/include/fwts_acpi_tables.h \
> b/src/lib/include/fwts_acpi_tables.h index 8151f76d..de51a4bc 100644
> --- a/src/lib/include/fwts_acpi_tables.h
> +++ b/src/lib/include/fwts_acpi_tables.h
> @@ -57,6 +57,7 @@ void fwts_acpi_reserved_zero_check(fwts_framework *fw, const char \
> *table, const void fwts_acpi_reserved_zero_array_check(fwts_framework *fw, const \
> char *table, const char *field, uint8_t* data, uint8_t length, bool *passed); void \
> fwts_acpi_reserved_bits_check(fwts_framework *fw, const char *table, const char \
> *field, uint64_t value, uint8_t size, uint8_t min, uint8_t max, bool *passed); void \
> fwts_acpi_reserved_type_check(fwts_framework *fw, const char *table, uint8_t value, \
> uint8_t min, uint8_t reserved, bool *passed); +bool \
> fwts_acpi_table_length_check(fwts_framework *fw, const char *table, uint32_t \
> length, uint32_t size); bool fwts_acpi_structure_length_check(fwts_framework *fw, \
> const char *table, uint8_t subtable_type, uint32_t subtable_length, uint32_t size); \
> uint32_t fwts_get_acpi_version(fwts_framework *fw);
> diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c
> index 4ee7c783..88f484b7 100644
> --- a/src/lib/src/fwts_acpi_tables.c
> +++ b/src/lib/src/fwts_acpi_tables.c
> @@ -1546,6 +1546,29 @@ void fwts_acpi_reserved_type_check(
> }
> }
>
> +/*
> + * fwts_acpi_table_length_check()
> + * verify whether table length is sane
> + */
> +bool fwts_acpi_table_length_check(
> + fwts_framework *fw,
> + const char *table,
> + uint32_t length,
> + uint32_t size)
> +{
> + if (length < size) {
> + char label[30];
> + strncpy(label, table, 4); /* ACPI name is 4 char long */
> + strncpy(label + 4, "TooShort", sizeof(label) - 4);
> + fwts_failed(fw, LOG_LEVEL_HIGH, label,
> + "%4.4s table too short, expecting %2.2" PRIu8
> + " bytes, instead got %2.2" PRIu8 " bytes",
> + table, size, length);
> + return false;
> + }
> + return true;
> +}
> +
> /*
> * fwts_acpi_structure_length_check()
> * verify whether sub structure length is sane
>
Thanks Alex
Acked-by: Colin Ian King <colin.king@canonical.com>
--
fwts-devel mailing list
fwts-devel@lists.ubuntu.com
Modify settings or unsubscribe at: \
https://lists.ubuntu.com/mailman/listinfo/fwts-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic