diff mbox

acpi/nfit: Fix endless loop on broken NFIT tables

Message ID 1470831292-29649-1-git-send-email-prarit@redhat.com
State Accepted
Headers show

Commit Message

Prarit Bhargava Aug. 10, 2016, 12:14 p.m. UTC
When running 'fwts nfit' on a system with an incorrect subtable length of
zero, the nfit test will loop endlessly.

This results.log contains many entries of

NFIT NVDIMM Firmware Interface Table:
  Reserved:                 0x00000000

  NFIT Subtable:
    Type:                                   0x0000
    Length:                                 0x0000
    SPA Range Structure Index:              0x0000
    Flags:                                  0x0000
    Reserved:                               0x00000000
    Proximity Domain:                       0x00000000
    Address Range Type GUID:                00000000-0000-0000-0000-000000000000
    System Physical Address Range Base:     0x0000000000000000
    System Physical Address Range Length:   0x0000000000000000
    Address Range Memory Mapping Attribute: 0x0000000000000000
FAILED [HIGH] NFITBadRangeIndexZero: Test 1, NFIT SPA Range Structure Index must
not be zero

This occurs because the test assumes a valid table length.  While the ACPI
specification is not explicit in indicating that a zero length is invalid,
it certainly is implied that it cannot be zero.

This patch adds a check and aborts the NFIT test on a zero subtable length.

As a result the output of the test is now

NFIT NVDIMM Firmware Interface Table:
  Reserved:                 0x00000000

  NFIT Subtable:
    Type:                                   0x0000
    Length:                                 0x0000
FAILED [HIGH] NFITLengthZero: Test 1, NFIT Subtable (offset 0x28) length cannot
be 0

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Alex Hung <alex.hung@canonical.com>
---
 src/acpi/nfit/nfit.c |    8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Alex Hung Aug. 11, 2016, 8:06 a.m. UTC | #1
On 2016-08-10 08:14 PM, Prarit Bhargava wrote:
> When running 'fwts nfit' on a system with an incorrect subtable length of
> zero, the nfit test will loop endlessly.
>
> This results.log contains many entries of
>
> NFIT NVDIMM Firmware Interface Table:
>   Reserved:                 0x00000000
>
>   NFIT Subtable:
>     Type:                                   0x0000
>     Length:                                 0x0000
>     SPA Range Structure Index:              0x0000
>     Flags:                                  0x0000
>     Reserved:                               0x00000000
>     Proximity Domain:                       0x00000000
>     Address Range Type GUID:                00000000-0000-0000-0000-000000000000
>     System Physical Address Range Base:     0x0000000000000000
>     System Physical Address Range Length:   0x0000000000000000
>     Address Range Memory Mapping Attribute: 0x0000000000000000
> FAILED [HIGH] NFITBadRangeIndexZero: Test 1, NFIT SPA Range Structure Index must
> not be zero
>
> This occurs because the test assumes a valid table length.  While the ACPI
> specification is not explicit in indicating that a zero length is invalid,
> it certainly is implied that it cannot be zero.
>
> This patch adds a check and aborts the NFIT test on a zero subtable length.
>
> As a result the output of the test is now
>
> NFIT NVDIMM Firmware Interface Table:
>   Reserved:                 0x00000000
>
>   NFIT Subtable:
>     Type:                                   0x0000
>     Length:                                 0x0000
> FAILED [HIGH] NFITLengthZero: Test 1, NFIT Subtable (offset 0x28) length cannot
> be 0
>
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: Alex Hung <alex.hung@canonical.com>
> ---
>  src/acpi/nfit/nfit.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/src/acpi/nfit/nfit.c b/src/acpi/nfit/nfit.c
> index 3738a5037f82..253070eb8bbd 100644
> --- a/src/acpi/nfit/nfit.c
> +++ b/src/acpi/nfit/nfit.c
> @@ -76,6 +76,14 @@ static int nfit_test1(fwts_framework *fw)
>  		fwts_log_info_verbatim(fw, "    Type:                                   0x%4.4" PRIx16, entry->type);
>  		fwts_log_info_verbatim(fw, "    Length:                                 0x%4.4" PRIx16, entry->length);
>
> +		if (entry->length == 0) {
> +			passed = false;
> +			fwts_failed(fw, LOG_LEVEL_HIGH, "NFITLengthZero",
> +				    "NFIT Subtable (offset 0x%x) length "
> +				    "cannot be 0", (int)offset);
> +			break;
> +		}
> +
>  		if (entry->type == FWTS_ACPI_NFIT_TYPE_SYSTEM_ADDRESS) {
>  			fwts_acpi_table_nfit_system_memory *nfit_struct = (fwts_acpi_table_nfit_system_memory *) entry;
>  			char guid_str[37];
>

Acked-by: Alex Hung <alex.hung@canonical.com>
Colin Ian King Aug. 11, 2016, 11:12 a.m. UTC | #2
On 10/08/16 13:14, Prarit Bhargava wrote:
> When running 'fwts nfit' on a system with an incorrect subtable length of
> zero, the nfit test will loop endlessly.
> 
> This results.log contains many entries of
> 
> NFIT NVDIMM Firmware Interface Table:
>   Reserved:                 0x00000000
> 
>   NFIT Subtable:
>     Type:                                   0x0000
>     Length:                                 0x0000
>     SPA Range Structure Index:              0x0000
>     Flags:                                  0x0000
>     Reserved:                               0x00000000
>     Proximity Domain:                       0x00000000
>     Address Range Type GUID:                00000000-0000-0000-0000-000000000000
>     System Physical Address Range Base:     0x0000000000000000
>     System Physical Address Range Length:   0x0000000000000000
>     Address Range Memory Mapping Attribute: 0x0000000000000000
> FAILED [HIGH] NFITBadRangeIndexZero: Test 1, NFIT SPA Range Structure Index must
> not be zero
> 
> This occurs because the test assumes a valid table length.  While the ACPI
> specification is not explicit in indicating that a zero length is invalid,
> it certainly is implied that it cannot be zero.
> 
> This patch adds a check and aborts the NFIT test on a zero subtable length.
> 
> As a result the output of the test is now
> 
> NFIT NVDIMM Firmware Interface Table:
>   Reserved:                 0x00000000
> 
>   NFIT Subtable:
>     Type:                                   0x0000
>     Length:                                 0x0000
> FAILED [HIGH] NFITLengthZero: Test 1, NFIT Subtable (offset 0x28) length cannot
> be 0
> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: Alex Hung <alex.hung@canonical.com>
> ---
>  src/acpi/nfit/nfit.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/acpi/nfit/nfit.c b/src/acpi/nfit/nfit.c
> index 3738a5037f82..253070eb8bbd 100644
> --- a/src/acpi/nfit/nfit.c
> +++ b/src/acpi/nfit/nfit.c
> @@ -76,6 +76,14 @@ static int nfit_test1(fwts_framework *fw)
>  		fwts_log_info_verbatim(fw, "    Type:                                   0x%4.4" PRIx16, entry->type);
>  		fwts_log_info_verbatim(fw, "    Length:                                 0x%4.4" PRIx16, entry->length);
>  
> +		if (entry->length == 0) {
> +			passed = false;
> +			fwts_failed(fw, LOG_LEVEL_HIGH, "NFITLengthZero",
> +				    "NFIT Subtable (offset 0x%x) length "
> +				    "cannot be 0", (int)offset);
> +			break;
> +		}
> +
>  		if (entry->type == FWTS_ACPI_NFIT_TYPE_SYSTEM_ADDRESS) {
>  			fwts_acpi_table_nfit_system_memory *nfit_struct = (fwts_acpi_table_nfit_system_memory *) entry;
>  			char guid_str[37];
> 
Thanks Prarit, nice catch!

Acked-by: Colin Ian King <colin.king@canonical.com>
diff mbox

Patch

diff --git a/src/acpi/nfit/nfit.c b/src/acpi/nfit/nfit.c
index 3738a5037f82..253070eb8bbd 100644
--- a/src/acpi/nfit/nfit.c
+++ b/src/acpi/nfit/nfit.c
@@ -76,6 +76,14 @@  static int nfit_test1(fwts_framework *fw)
 		fwts_log_info_verbatim(fw, "    Type:                                   0x%4.4" PRIx16, entry->type);
 		fwts_log_info_verbatim(fw, "    Length:                                 0x%4.4" PRIx16, entry->length);
 
+		if (entry->length == 0) {
+			passed = false;
+			fwts_failed(fw, LOG_LEVEL_HIGH, "NFITLengthZero",
+				    "NFIT Subtable (offset 0x%x) length "
+				    "cannot be 0", (int)offset);
+			break;
+		}
+
 		if (entry->type == FWTS_ACPI_NFIT_TYPE_SYSTEM_ADDRESS) {
 			fwts_acpi_table_nfit_system_memory *nfit_struct = (fwts_acpi_table_nfit_system_memory *) entry;
 			char guid_str[37];