diff mbox series

[1/2] nfit: improve length handling for ACPI NFIT structures

Message ID 20180706144255.56986-1-elliott@hpe.com
State Accepted
Headers show
Series [1/2] nfit: improve length handling for ACPI NFIT structures | expand

Commit Message

Elliott, Robert (Servers) July 6, 2018, 2:42 p.m. UTC
Check that each ACPI NFIT structure length is long enough before decoding
values in its fields.

For example, ACPI defines that the Control Region structure is either
32 or 80 bytes; if 32, then none of the fields after the Size of Block
Control Window field is valid.  However, fwts was accessing those fields,
interpreting whatever was in the next table entry.

Example complaining that the reserved fields in bytes 74-79 are
non-zero (because it's re reading bits from the next structure):

  NFIT Subtable:
    Type:                                   0x0004
    Length:                                 0x0020
    NVDIMM Control Region Structure Index:  0x0001
...
    Size of Status Register:                0x0000000100300001
    NVDIMM Control Region Flag:             0x0025
    Reserved:                               0x0000000100010000
FAILED [HIGH] NFITReservedBitsNonZero: Test 1, NFIT NVDIMM Control Region Flags
Bits [15..1] must be zero, got 0x0025 instead
FAILED [MEDIUM] NFITReservedNonZero: Test 1, NFIT Reserved field must be zero,
got 0x0000000100010000 instead

1. Check that each structure is the minimum size defined in ACPI.

2. Check that the Interleave and Flush Hint Address structures are the minimum
size defined by their fields indicating the number of entries.

3. Move field accesses after the appropriate length checks.

Signed-off-by: Robert Elliott <elliott@hpe.com>
---
 src/acpi/nfit/nfit.c        | 127 ++++++++++++++++++++++++++++++++++++++------
 src/lib/include/fwts_acpi.h |  18 +++++++
 2 files changed, 130 insertions(+), 15 deletions(-)

Comments

Alex Hung July 9, 2018, 8:13 p.m. UTC | #1
On 2018-07-06 07:42 AM, Robert Elliott wrote:
> Check that each ACPI NFIT structure length is long enough before decoding
> values in its fields.
> 
> For example, ACPI defines that the Control Region structure is either
> 32 or 80 bytes; if 32, then none of the fields after the Size of Block
> Control Window field is valid.  However, fwts was accessing those fields,
> interpreting whatever was in the next table entry.
> 
> Example complaining that the reserved fields in bytes 74-79 are
> non-zero (because it's re reading bits from the next structure):
> 
>    NFIT Subtable:
>      Type:                                   0x0004
>      Length:                                 0x0020
>      NVDIMM Control Region Structure Index:  0x0001
> ...
>      Size of Status Register:                0x0000000100300001
>      NVDIMM Control Region Flag:             0x0025
>      Reserved:                               0x0000000100010000
> FAILED [HIGH] NFITReservedBitsNonZero: Test 1, NFIT NVDIMM Control Region Flags
> Bits [15..1] must be zero, got 0x0025 instead
> FAILED [MEDIUM] NFITReservedNonZero: Test 1, NFIT Reserved field must be zero,
> got 0x0000000100010000 instead
> 
> 1. Check that each structure is the minimum size defined in ACPI.
> 
> 2. Check that the Interleave and Flush Hint Address structures are the minimum
> size defined by their fields indicating the number of entries.
> 
> 3. Move field accesses after the appropriate length checks.
> 
> Signed-off-by: Robert Elliott <elliott@hpe.com>
> ---
>   src/acpi/nfit/nfit.c        | 127 ++++++++++++++++++++++++++++++++++++++------
>   src/lib/include/fwts_acpi.h |  18 +++++++
>   2 files changed, 130 insertions(+), 15 deletions(-)
> 
> diff --git a/src/acpi/nfit/nfit.c b/src/acpi/nfit/nfit.c
> index 216593d3..99231501 100644
> --- a/src/acpi/nfit/nfit.c
> +++ b/src/acpi/nfit/nfit.c
> @@ -40,6 +40,16 @@ static const uint8_t guid_virtual_device[4][16] = {
>   
>   static fwts_acpi_table_info *table;
>   
> +static bool check_length(fwts_framework *fw, int actual, int min, const char *name) {
> +	if (actual < min) {
> +		fwts_failed(fw, LOG_LEVEL_HIGH, "NFITSubtableLength",
> +			    "NFIT Subtable %s length %d bytes is too short, expected >= %d bytes",
> +			    name, actual, min);
> +		return false;
> +	}
> +	return true;
> +}
> +
>   static int nfit_init(fwts_framework *fw)
>   {
>   	if (fwts_acpi_find_table(fw, "NFIT", 0, &table) != FWTS_OK) {
> @@ -93,6 +103,14 @@ static int nfit_test1(fwts_framework *fw)
>   			bool guid_skip = false;
>   			size_t i;
>   
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_SYSTEM_ADDRESS,
> +					FWTS_ACPI_NFIT_NAME_SYSTEM_ADDRESS);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>   			fwts_guid_buf_to_str(nfit_struct->range_guid, guid_str, sizeof(guid_str));
>   
>   			fwts_log_info_verbatim(fw, "    SPA Range Structure Index:              0x%4.4" PRIx16, nfit_struct->range_index);
> @@ -180,6 +198,14 @@ static int nfit_test1(fwts_framework *fw)
>   		} else if (entry->type == FWTS_ACPI_NFIT_TYPE_MEMORY_MAP) {
>   			fwts_acpi_table_nfit_memory_map *nfit_struct = (fwts_acpi_table_nfit_memory_map *) entry;
>   
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_MEMORY_MAP,
> +					FWTS_ACPI_NFIT_NAME_MEMORY_MAP);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>   			fwts_log_info_verbatim(fw, "    NFIT Device Handle:                     0x%8.8" PRIx32, nfit_struct->device_handle);
>   			fwts_log_info_verbatim(fw, "    NVDIMM Physical ID:                     0x%4.4" PRIx16, nfit_struct->physical_id);
>   			fwts_log_info_verbatim(fw, "    NVDIMM Region ID:                       0x%4.4" PRIx16, nfit_struct->region_id);
> @@ -202,11 +228,28 @@ static int nfit_test1(fwts_framework *fw)
>   			fwts_acpi_table_nfit_interleave *nfit_struct = (fwts_acpi_table_nfit_interleave *) entry;
>   			uint32_t i;
>   
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_INTERLEAVE,
> +					FWTS_ACPI_NFIT_NAME_INTERLEAVE);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>   			fwts_log_info_verbatim(fw, "    Interleave Structure Index:             0x%4.4" PRIx16, nfit_struct->interleave_index);
>   			fwts_log_info_verbatim(fw, "    Reserved:                               0x%4.4" PRIx16, nfit_struct->reserved);
>   			fwts_log_info_verbatim(fw, "    Number of Lines Described:              0x%8.8" PRIx32, nfit_struct->line_count);
>   			fwts_log_info_verbatim(fw, "    Line Size:                              0x%8.8" PRIx32, nfit_struct->line_size);
>   
> +			ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_INTERLEAVE +
> +						nfit_struct->line_count * sizeof nfit_struct->line_offset[0],
> +					FWTS_ACPI_NFIT_NAME_INTERLEAVE);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>   			for (i = 0; i < nfit_struct->line_count; i++) {
>   				fwts_log_info_verbatim(fw, "    Line Offset:                            0x%8.8"  PRIx32, nfit_struct->line_offset[i]);
>   
> @@ -230,20 +273,28 @@ static int nfit_test1(fwts_framework *fw)
>   		} else if (entry->type == FWTS_ACPI_NFIT_TYPE_SMBIOS) {
>   			fwts_acpi_table_nfit_smbios *nfit_struct = (fwts_acpi_table_nfit_smbios *) entry;
>   
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_SMBIOS,
> +					FWTS_ACPI_NFIT_NAME_SMBIOS);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>   			fwts_log_info_verbatim(fw, "    Reserved:                               0x%8.8" PRIx32, nfit_struct->reserved);
>   
>   		} else if (entry->type == FWTS_ACPI_NFIT_TYPE_CONTROL_REGION) {
>   			fwts_acpi_table_nfit_control_range *nfit_struct = (fwts_acpi_table_nfit_control_range *) entry;
>   			uint64_t reserved1;
>   
> -			reserved1 = (uint64_t) nfit_struct->reserved1[0] + ((uint64_t) nfit_struct->reserved1[1] << 8) +
> -				   ((uint64_t) nfit_struct->reserved1[2] << 16) + ((uint64_t) nfit_struct->reserved1[3] << 24) +
> -				   ((uint64_t) nfit_struct->reserved1[4] << 32) + ((uint64_t) nfit_struct->reserved1[5] << 40);
> -
> -			if (reserved1 != 0)
> -				reserved_passed = reserved1;
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_CONTROL_REGION,
> +					FWTS_ACPI_NFIT_NAME_CONTROL_REGION);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
>   
> -			fwts_log_info_verbatim(fw, "    NVDIMM Control Region Structure Index:  0x%4.4" PRIx16, nfit_struct->region_index);
>   			fwts_log_info_verbatim(fw, "    Vendor ID:                              0x%4.4" PRIx16, nfit_struct->vendor_id);
>   			fwts_log_info_verbatim(fw, "    Device ID:                              0x%4.4" PRIx16, nfit_struct->device_id);
>   			fwts_log_info_verbatim(fw, "    Revision ID:                            0x%4.4" PRIx16, nfit_struct->revision_id);
> @@ -257,13 +308,6 @@ static int nfit_test1(fwts_framework *fw)
>   			fwts_log_info_verbatim(fw, "    Serial Number:                          0x%8.8" PRIx32, nfit_struct->serial_number);
>   			fwts_log_info_verbatim(fw, "    Region Format Interface Code:           0x%4.4" PRIx16, nfit_struct->interface_code);
>   			fwts_log_info_verbatim(fw, "    Number of Block Control Windows:        0x%4.4" PRIx16, nfit_struct->windows_num);
> -			fwts_log_info_verbatim(fw, "    Size of Block Control Window:           0x%16.16" PRIx64, nfit_struct->window_size);
> -			fwts_log_info_verbatim(fw, "    Command Register Offset:                0x%16.16" PRIx64, nfit_struct->command_offset);
> -			fwts_log_info_verbatim(fw, "    Size of Command Register:               0x%16.16" PRIx64, nfit_struct->command_size);
> -			fwts_log_info_verbatim(fw, "    Status RegisterOffset:                  0x%16.16" PRIx64, nfit_struct->status_offset);
> -			fwts_log_info_verbatim(fw, "    Size of Status Register:                0x%16.16" PRIx64, nfit_struct->status_size);
> -			fwts_log_info_verbatim(fw, "    NVDIMM Control Region Flag:             0x%4.4" PRIx16, nfit_struct->flags);
> -			fwts_log_info_verbatim(fw, "    Reserved:                               0x%16.16" PRIx64, reserved1);
>   
>   			if (nfit_struct->revision_id & 0xFF00) {
>   				passed = false;
> @@ -285,11 +329,38 @@ static int nfit_test1(fwts_framework *fw)
>   				reserved_passed = nfit_struct->reserved;
>   
>   			fwts_acpi_reserved_bits_check(fw, "NFIT", "Valid", nfit_struct->valid_fields, sizeof(nfit_struct->valid_fields), 1, 7, &passed);
> -			fwts_acpi_reserved_bits_check(fw, "NFIT", "NVDIMM Control Region Flags", nfit_struct->flags, sizeof(nfit_struct->flags), 1, 15, &passed);
> +
> +			if (entry->length >= sizeof(*nfit_struct)) {
> +				reserved1 = (uint64_t) nfit_struct->reserved1[0] + ((uint64_t) nfit_struct->reserved1[1] << 8) +
> +					   ((uint64_t) nfit_struct->reserved1[2] << 16) + ((uint64_t) nfit_struct->reserved1[3] << 24) +
> +					   ((uint64_t) nfit_struct->reserved1[4] << 32) + ((uint64_t) nfit_struct->reserved1[5] << 40);
> +
> +				if (reserved1 != 0)
> +					reserved_passed = reserved1;
> +
> +				fwts_log_info_verbatim(fw, "    Size of Block Control Window:           0x%16.16" PRIx64, nfit_struct->window_size);
> +				fwts_log_info_verbatim(fw, "    Command Register Offset:                0x%16.16" PRIx64, nfit_struct->command_offset);
> +				fwts_log_info_verbatim(fw, "    Size of Command Register:               0x%16.16" PRIx64, nfit_struct->command_size);
> +				fwts_log_info_verbatim(fw, "    Status RegisterOffset:                  0x%16.16" PRIx64, nfit_struct->status_offset);
> +				fwts_log_info_verbatim(fw, "    Size of Status Register:                0x%16.16" PRIx64, nfit_struct->status_size);
> +				fwts_log_info_verbatim(fw, "    NVDIMM Control Region Flag:             0x%4.4" PRIx16, nfit_struct->flags);
> +				fwts_log_info_verbatim(fw, "    Reserved:                               0x%16.16" PRIx64, reserved1);
> +
> +				fwts_acpi_reserved_bits_check(fw, "NFIT", "NVDIMM Control Region Flags", nfit_struct->flags, sizeof(nfit_struct->flags), 1, 15, &passed);
> +				fwts_log_info_verbatim(fw, "    NVDIMM Control Region Structure Index:  0x%4.4" PRIx16, nfit_struct->region_index);
> +			}
>   
>   		} else if (entry->type == FWTS_ACPI_NFIT_TYPE_DATA_REGION) {
>   			fwts_acpi_table_nfit_data_range *nfit_struct = (fwts_acpi_table_nfit_data_range *) entry;
>   
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_DATA_REGION,
> +					FWTS_ACPI_NFIT_NAME_DATA_REGION);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>   			fwts_log_info_verbatim(fw, "    NVDIMM Control Region Structure Index:  0x%4.4" PRIx16, nfit_struct->region_index);
>   			fwts_log_info_verbatim(fw, "    Number of Block Data Windows:           0x%4.4" PRIx16, nfit_struct->window_num);
>   			fwts_log_info_verbatim(fw, "    Block Data Window Start Offset:         0x%16.16" PRIx64, nfit_struct->window_offset);
> @@ -309,6 +380,14 @@ static int nfit_test1(fwts_framework *fw)
>   			uint64_t reserved;
>   			uint16_t i;
>   
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_FLUSH_ADDRESS,
> +					FWTS_ACPI_NFIT_NAME_FLUSH_ADDRESS);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>   			reserved = (uint64_t) nfit_struct->reserved[0] + ((uint64_t) nfit_struct->reserved[1] << 8) +
>   				   ((uint64_t) nfit_struct->reserved[2] << 16) + ((uint64_t) nfit_struct->reserved[3] << 24) +
>   				   ((uint64_t) nfit_struct->reserved[4] << 32) + ((uint64_t) nfit_struct->reserved[5] << 40);
> @@ -316,6 +395,16 @@ static int nfit_test1(fwts_framework *fw)
>   			fwts_log_info_verbatim(fw, "    NFIT Device Handle:                     0x%8.8" PRIx32, nfit_struct->device_handle);
>   			fwts_log_info_verbatim(fw, "    Number of Flush Hint Addresses:         0x%4.4" PRIx16, nfit_struct->hint_count);
>   			fwts_log_info_verbatim(fw, "    Reserved:                               0x%16.16" PRIx64, reserved);
> +
> +			ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_FLUSH_ADDRESS +
> +						nfit_struct->hint_count * sizeof nfit_struct->hint_address[0],
> +					FWTS_ACPI_NFIT_NAME_FLUSH_ADDRESS);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>   			for (i = 0; i < nfit_struct->hint_count; i++)
>   				fwts_log_info_verbatim(fw, "    Flush Hint Address:                     0x%16.16" PRIx64, nfit_struct->hint_address[i]);
>   
> @@ -326,6 +415,14 @@ static int nfit_test1(fwts_framework *fw)
>   			fwts_acpi_table_nfit_platform_cap *nfit_struct = (fwts_acpi_table_nfit_platform_cap *) entry;
>   			uint32_t reserved1;
>   
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_PLATFORM_CAPABILITY,
> +					FWTS_ACPI_NFIT_NAME_PLATFORM_CAPABILITY);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>   			reserved1 = (uint32_t) nfit_struct->reserved1[0] + ((uint32_t) nfit_struct->reserved1[1] << 8) +
>   				   ((uint32_t) nfit_struct->reserved1[2] << 16);
>   
> diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h
> index e550cd76..10dedb6a 100644
> --- a/src/lib/include/fwts_acpi.h
> +++ b/src/lib/include/fwts_acpi.h
> @@ -1181,6 +1181,24 @@ typedef enum {
>   	FWTS_ACPI_NFIT_TYPE_RESERVED             = 8     /* >= 8 are reserved */
>   } fwts_acpi_nfit_type;
>   
> +#define FWTS_ACPI_NFIT_NAME_SYSTEM_ADDRESS      "SPA Range structure"
> +#define FWTS_ACPI_NFIT_NAME_MEMORY_MAP          "NVDIMM Region Mapping structure"
> +#define FWTS_ACPI_NFIT_NAME_INTERLEAVE          "Interleave structure"
> +#define FWTS_ACPI_NFIT_NAME_SMBIOS              "SMBIOS Management Information structure"
> +#define FWTS_ACPI_NFIT_NAME_CONTROL_REGION      "NVDIMM Control Region structure"
> +#define FWTS_ACPI_NFIT_NAME_DATA_REGION         "NVDIMM Block Data Window Region structure"
> +#define FWTS_ACPI_NFIT_NAME_FLUSH_ADDRESS       "Flush Hint Address structure"
> +#define FWTS_ACPI_NFIT_NAME_PLATFORM_CAPABILITY "Platform Capabilities structure"
> +
> +#define FWTS_ACPI_NFIT_MINLEN_SYSTEM_ADDRESS      56
> +#define FWTS_ACPI_NFIT_MINLEN_MEMORY_MAP          48
> +#define FWTS_ACPI_NFIT_MINLEN_INTERLEAVE          16
> +#define FWTS_ACPI_NFIT_MINLEN_SMBIOS              8
> +#define FWTS_ACPI_NFIT_MINLEN_CONTROL_REGION      32
> +#define FWTS_ACPI_NFIT_MINLEN_DATA_REGION         32
> +#define FWTS_ACPI_NFIT_MINLEN_FLUSH_ADDRESS       16
> +#define FWTS_ACPI_NFIT_MINLEN_PLATFORM_CAPABILITY 16
> +
>   typedef struct {
>   	fwts_acpi_table_nfit_struct_header	header;
>   	uint16_t	range_index;
> 

Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu July 16, 2018, 9:58 a.m. UTC | #2
On 07/06/2018 10:42 PM, Robert Elliott wrote:
> Check that each ACPI NFIT structure length is long enough before decoding
> values in its fields.
>
> For example, ACPI defines that the Control Region structure is either
> 32 or 80 bytes; if 32, then none of the fields after the Size of Block
> Control Window field is valid.  However, fwts was accessing those fields,
> interpreting whatever was in the next table entry.
>
> Example complaining that the reserved fields in bytes 74-79 are
> non-zero (because it's re reading bits from the next structure):
>
>   NFIT Subtable:
>     Type:                                   0x0004
>     Length:                                 0x0020
>     NVDIMM Control Region Structure Index:  0x0001
> ...
>     Size of Status Register:                0x0000000100300001
>     NVDIMM Control Region Flag:             0x0025
>     Reserved:                               0x0000000100010000
> FAILED [HIGH] NFITReservedBitsNonZero: Test 1, NFIT NVDIMM Control Region Flags
> Bits [15..1] must be zero, got 0x0025 instead
> FAILED [MEDIUM] NFITReservedNonZero: Test 1, NFIT Reserved field must be zero,
> got 0x0000000100010000 instead
>
> 1. Check that each structure is the minimum size defined in ACPI.
>
> 2. Check that the Interleave and Flush Hint Address structures are the minimum
> size defined by their fields indicating the number of entries.
>
> 3. Move field accesses after the appropriate length checks.
>
> Signed-off-by: Robert Elliott <elliott@hpe.com>
> ---
>  src/acpi/nfit/nfit.c        | 127 ++++++++++++++++++++++++++++++++++++++------
>  src/lib/include/fwts_acpi.h |  18 +++++++
>  2 files changed, 130 insertions(+), 15 deletions(-)
>
> diff --git a/src/acpi/nfit/nfit.c b/src/acpi/nfit/nfit.c
> index 216593d3..99231501 100644
> --- a/src/acpi/nfit/nfit.c
> +++ b/src/acpi/nfit/nfit.c
> @@ -40,6 +40,16 @@ static const uint8_t guid_virtual_device[4][16] = {
>  
>  static fwts_acpi_table_info *table;
>  
> +static bool check_length(fwts_framework *fw, int actual, int min, const char *name) {
> +	if (actual < min) {
> +		fwts_failed(fw, LOG_LEVEL_HIGH, "NFITSubtableLength",
> +			    "NFIT Subtable %s length %d bytes is too short, expected >= %d bytes",
> +			    name, actual, min);
> +		return false;
> +	}
> +	return true;
> +}
> +
>  static int nfit_init(fwts_framework *fw)
>  {
>  	if (fwts_acpi_find_table(fw, "NFIT", 0, &table) != FWTS_OK) {
> @@ -93,6 +103,14 @@ static int nfit_test1(fwts_framework *fw)
>  			bool guid_skip = false;
>  			size_t i;
>  
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_SYSTEM_ADDRESS,
> +					FWTS_ACPI_NFIT_NAME_SYSTEM_ADDRESS);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>  			fwts_guid_buf_to_str(nfit_struct->range_guid, guid_str, sizeof(guid_str));
>  
>  			fwts_log_info_verbatim(fw, "    SPA Range Structure Index:              0x%4.4" PRIx16, nfit_struct->range_index);
> @@ -180,6 +198,14 @@ static int nfit_test1(fwts_framework *fw)
>  		} else if (entry->type == FWTS_ACPI_NFIT_TYPE_MEMORY_MAP) {
>  			fwts_acpi_table_nfit_memory_map *nfit_struct = (fwts_acpi_table_nfit_memory_map *) entry;
>  
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_MEMORY_MAP,
> +					FWTS_ACPI_NFIT_NAME_MEMORY_MAP);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>  			fwts_log_info_verbatim(fw, "    NFIT Device Handle:                     0x%8.8" PRIx32, nfit_struct->device_handle);
>  			fwts_log_info_verbatim(fw, "    NVDIMM Physical ID:                     0x%4.4" PRIx16, nfit_struct->physical_id);
>  			fwts_log_info_verbatim(fw, "    NVDIMM Region ID:                       0x%4.4" PRIx16, nfit_struct->region_id);
> @@ -202,11 +228,28 @@ static int nfit_test1(fwts_framework *fw)
>  			fwts_acpi_table_nfit_interleave *nfit_struct = (fwts_acpi_table_nfit_interleave *) entry;
>  			uint32_t i;
>  
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_INTERLEAVE,
> +					FWTS_ACPI_NFIT_NAME_INTERLEAVE);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>  			fwts_log_info_verbatim(fw, "    Interleave Structure Index:             0x%4.4" PRIx16, nfit_struct->interleave_index);
>  			fwts_log_info_verbatim(fw, "    Reserved:                               0x%4.4" PRIx16, nfit_struct->reserved);
>  			fwts_log_info_verbatim(fw, "    Number of Lines Described:              0x%8.8" PRIx32, nfit_struct->line_count);
>  			fwts_log_info_verbatim(fw, "    Line Size:                              0x%8.8" PRIx32, nfit_struct->line_size);
>  
> +			ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_INTERLEAVE +
> +						nfit_struct->line_count * sizeof nfit_struct->line_offset[0],
> +					FWTS_ACPI_NFIT_NAME_INTERLEAVE);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>  			for (i = 0; i < nfit_struct->line_count; i++) {
>  				fwts_log_info_verbatim(fw, "    Line Offset:                            0x%8.8"  PRIx32, nfit_struct->line_offset[i]);
>  
> @@ -230,20 +273,28 @@ static int nfit_test1(fwts_framework *fw)
>  		} else if (entry->type == FWTS_ACPI_NFIT_TYPE_SMBIOS) {
>  			fwts_acpi_table_nfit_smbios *nfit_struct = (fwts_acpi_table_nfit_smbios *) entry;
>  
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_SMBIOS,
> +					FWTS_ACPI_NFIT_NAME_SMBIOS);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>  			fwts_log_info_verbatim(fw, "    Reserved:                               0x%8.8" PRIx32, nfit_struct->reserved);
>  
>  		} else if (entry->type == FWTS_ACPI_NFIT_TYPE_CONTROL_REGION) {
>  			fwts_acpi_table_nfit_control_range *nfit_struct = (fwts_acpi_table_nfit_control_range *) entry;
>  			uint64_t reserved1;
>  
> -			reserved1 = (uint64_t) nfit_struct->reserved1[0] + ((uint64_t) nfit_struct->reserved1[1] << 8) +
> -				   ((uint64_t) nfit_struct->reserved1[2] << 16) + ((uint64_t) nfit_struct->reserved1[3] << 24) +
> -				   ((uint64_t) nfit_struct->reserved1[4] << 32) + ((uint64_t) nfit_struct->reserved1[5] << 40);
> -
> -			if (reserved1 != 0)
> -				reserved_passed = reserved1;
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_CONTROL_REGION,
> +					FWTS_ACPI_NFIT_NAME_CONTROL_REGION);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
>  
> -			fwts_log_info_verbatim(fw, "    NVDIMM Control Region Structure Index:  0x%4.4" PRIx16, nfit_struct->region_index);
>  			fwts_log_info_verbatim(fw, "    Vendor ID:                              0x%4.4" PRIx16, nfit_struct->vendor_id);
>  			fwts_log_info_verbatim(fw, "    Device ID:                              0x%4.4" PRIx16, nfit_struct->device_id);
>  			fwts_log_info_verbatim(fw, "    Revision ID:                            0x%4.4" PRIx16, nfit_struct->revision_id);
> @@ -257,13 +308,6 @@ static int nfit_test1(fwts_framework *fw)
>  			fwts_log_info_verbatim(fw, "    Serial Number:                          0x%8.8" PRIx32, nfit_struct->serial_number);
>  			fwts_log_info_verbatim(fw, "    Region Format Interface Code:           0x%4.4" PRIx16, nfit_struct->interface_code);
>  			fwts_log_info_verbatim(fw, "    Number of Block Control Windows:        0x%4.4" PRIx16, nfit_struct->windows_num);
> -			fwts_log_info_verbatim(fw, "    Size of Block Control Window:           0x%16.16" PRIx64, nfit_struct->window_size);
> -			fwts_log_info_verbatim(fw, "    Command Register Offset:                0x%16.16" PRIx64, nfit_struct->command_offset);
> -			fwts_log_info_verbatim(fw, "    Size of Command Register:               0x%16.16" PRIx64, nfit_struct->command_size);
> -			fwts_log_info_verbatim(fw, "    Status RegisterOffset:                  0x%16.16" PRIx64, nfit_struct->status_offset);
> -			fwts_log_info_verbatim(fw, "    Size of Status Register:                0x%16.16" PRIx64, nfit_struct->status_size);
> -			fwts_log_info_verbatim(fw, "    NVDIMM Control Region Flag:             0x%4.4" PRIx16, nfit_struct->flags);
> -			fwts_log_info_verbatim(fw, "    Reserved:                               0x%16.16" PRIx64, reserved1);
>  
>  			if (nfit_struct->revision_id & 0xFF00) {
>  				passed = false;
> @@ -285,11 +329,38 @@ static int nfit_test1(fwts_framework *fw)
>  				reserved_passed = nfit_struct->reserved;
>  
>  			fwts_acpi_reserved_bits_check(fw, "NFIT", "Valid", nfit_struct->valid_fields, sizeof(nfit_struct->valid_fields), 1, 7, &passed);
> -			fwts_acpi_reserved_bits_check(fw, "NFIT", "NVDIMM Control Region Flags", nfit_struct->flags, sizeof(nfit_struct->flags), 1, 15, &passed);
> +
> +			if (entry->length >= sizeof(*nfit_struct)) {
> +				reserved1 = (uint64_t) nfit_struct->reserved1[0] + ((uint64_t) nfit_struct->reserved1[1] << 8) +
> +					   ((uint64_t) nfit_struct->reserved1[2] << 16) + ((uint64_t) nfit_struct->reserved1[3] << 24) +
> +					   ((uint64_t) nfit_struct->reserved1[4] << 32) + ((uint64_t) nfit_struct->reserved1[5] << 40);
> +
> +				if (reserved1 != 0)
> +					reserved_passed = reserved1;
> +
> +				fwts_log_info_verbatim(fw, "    Size of Block Control Window:           0x%16.16" PRIx64, nfit_struct->window_size);
> +				fwts_log_info_verbatim(fw, "    Command Register Offset:                0x%16.16" PRIx64, nfit_struct->command_offset);
> +				fwts_log_info_verbatim(fw, "    Size of Command Register:               0x%16.16" PRIx64, nfit_struct->command_size);
> +				fwts_log_info_verbatim(fw, "    Status RegisterOffset:                  0x%16.16" PRIx64, nfit_struct->status_offset);
> +				fwts_log_info_verbatim(fw, "    Size of Status Register:                0x%16.16" PRIx64, nfit_struct->status_size);
> +				fwts_log_info_verbatim(fw, "    NVDIMM Control Region Flag:             0x%4.4" PRIx16, nfit_struct->flags);
> +				fwts_log_info_verbatim(fw, "    Reserved:                               0x%16.16" PRIx64, reserved1);
> +
> +				fwts_acpi_reserved_bits_check(fw, "NFIT", "NVDIMM Control Region Flags", nfit_struct->flags, sizeof(nfit_struct->flags), 1, 15, &passed);
> +				fwts_log_info_verbatim(fw, "    NVDIMM Control Region Structure Index:  0x%4.4" PRIx16, nfit_struct->region_index);
> +			}
>  
>  		} else if (entry->type == FWTS_ACPI_NFIT_TYPE_DATA_REGION) {
>  			fwts_acpi_table_nfit_data_range *nfit_struct = (fwts_acpi_table_nfit_data_range *) entry;
>  
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_DATA_REGION,
> +					FWTS_ACPI_NFIT_NAME_DATA_REGION);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>  			fwts_log_info_verbatim(fw, "    NVDIMM Control Region Structure Index:  0x%4.4" PRIx16, nfit_struct->region_index);
>  			fwts_log_info_verbatim(fw, "    Number of Block Data Windows:           0x%4.4" PRIx16, nfit_struct->window_num);
>  			fwts_log_info_verbatim(fw, "    Block Data Window Start Offset:         0x%16.16" PRIx64, nfit_struct->window_offset);
> @@ -309,6 +380,14 @@ static int nfit_test1(fwts_framework *fw)
>  			uint64_t reserved;
>  			uint16_t i;
>  
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_FLUSH_ADDRESS,
> +					FWTS_ACPI_NFIT_NAME_FLUSH_ADDRESS);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>  			reserved = (uint64_t) nfit_struct->reserved[0] + ((uint64_t) nfit_struct->reserved[1] << 8) +
>  				   ((uint64_t) nfit_struct->reserved[2] << 16) + ((uint64_t) nfit_struct->reserved[3] << 24) +
>  				   ((uint64_t) nfit_struct->reserved[4] << 32) + ((uint64_t) nfit_struct->reserved[5] << 40);
> @@ -316,6 +395,16 @@ static int nfit_test1(fwts_framework *fw)
>  			fwts_log_info_verbatim(fw, "    NFIT Device Handle:                     0x%8.8" PRIx32, nfit_struct->device_handle);
>  			fwts_log_info_verbatim(fw, "    Number of Flush Hint Addresses:         0x%4.4" PRIx16, nfit_struct->hint_count);
>  			fwts_log_info_verbatim(fw, "    Reserved:                               0x%16.16" PRIx64, reserved);
> +
> +			ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_FLUSH_ADDRESS +
> +						nfit_struct->hint_count * sizeof nfit_struct->hint_address[0],
> +					FWTS_ACPI_NFIT_NAME_FLUSH_ADDRESS);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>  			for (i = 0; i < nfit_struct->hint_count; i++)
>  				fwts_log_info_verbatim(fw, "    Flush Hint Address:                     0x%16.16" PRIx64, nfit_struct->hint_address[i]);
>  
> @@ -326,6 +415,14 @@ static int nfit_test1(fwts_framework *fw)
>  			fwts_acpi_table_nfit_platform_cap *nfit_struct = (fwts_acpi_table_nfit_platform_cap *) entry;
>  			uint32_t reserved1;
>  
> +			bool ret = check_length(fw, entry->length,
> +					FWTS_ACPI_NFIT_MINLEN_PLATFORM_CAPABILITY,
> +					FWTS_ACPI_NFIT_NAME_PLATFORM_CAPABILITY);
> +			if (!ret) {
> +				passed = false;
> +				break;
> +			}
> +
>  			reserved1 = (uint32_t) nfit_struct->reserved1[0] + ((uint32_t) nfit_struct->reserved1[1] << 8) +
>  				   ((uint32_t) nfit_struct->reserved1[2] << 16);
>  
> diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h
> index e550cd76..10dedb6a 100644
> --- a/src/lib/include/fwts_acpi.h
> +++ b/src/lib/include/fwts_acpi.h
> @@ -1181,6 +1181,24 @@ typedef enum {
>  	FWTS_ACPI_NFIT_TYPE_RESERVED             = 8     /* >= 8 are reserved */
>  } fwts_acpi_nfit_type;
>  
> +#define FWTS_ACPI_NFIT_NAME_SYSTEM_ADDRESS      "SPA Range structure"
> +#define FWTS_ACPI_NFIT_NAME_MEMORY_MAP          "NVDIMM Region Mapping structure"
> +#define FWTS_ACPI_NFIT_NAME_INTERLEAVE          "Interleave structure"
> +#define FWTS_ACPI_NFIT_NAME_SMBIOS              "SMBIOS Management Information structure"
> +#define FWTS_ACPI_NFIT_NAME_CONTROL_REGION      "NVDIMM Control Region structure"
> +#define FWTS_ACPI_NFIT_NAME_DATA_REGION         "NVDIMM Block Data Window Region structure"
> +#define FWTS_ACPI_NFIT_NAME_FLUSH_ADDRESS       "Flush Hint Address structure"
> +#define FWTS_ACPI_NFIT_NAME_PLATFORM_CAPABILITY "Platform Capabilities structure"
> +
> +#define FWTS_ACPI_NFIT_MINLEN_SYSTEM_ADDRESS      56
> +#define FWTS_ACPI_NFIT_MINLEN_MEMORY_MAP          48
> +#define FWTS_ACPI_NFIT_MINLEN_INTERLEAVE          16
> +#define FWTS_ACPI_NFIT_MINLEN_SMBIOS              8
> +#define FWTS_ACPI_NFIT_MINLEN_CONTROL_REGION      32
> +#define FWTS_ACPI_NFIT_MINLEN_DATA_REGION         32
> +#define FWTS_ACPI_NFIT_MINLEN_FLUSH_ADDRESS       16
> +#define FWTS_ACPI_NFIT_MINLEN_PLATFORM_CAPABILITY 16
> +
>  typedef struct {
>  	fwts_acpi_table_nfit_struct_header	header;
>  	uint16_t	range_index;
Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff mbox series

Patch

diff --git a/src/acpi/nfit/nfit.c b/src/acpi/nfit/nfit.c
index 216593d3..99231501 100644
--- a/src/acpi/nfit/nfit.c
+++ b/src/acpi/nfit/nfit.c
@@ -40,6 +40,16 @@  static const uint8_t guid_virtual_device[4][16] = {
 
 static fwts_acpi_table_info *table;
 
+static bool check_length(fwts_framework *fw, int actual, int min, const char *name) {
+	if (actual < min) {
+		fwts_failed(fw, LOG_LEVEL_HIGH, "NFITSubtableLength",
+			    "NFIT Subtable %s length %d bytes is too short, expected >= %d bytes",
+			    name, actual, min);
+		return false;
+	}
+	return true;
+}
+
 static int nfit_init(fwts_framework *fw)
 {
 	if (fwts_acpi_find_table(fw, "NFIT", 0, &table) != FWTS_OK) {
@@ -93,6 +103,14 @@  static int nfit_test1(fwts_framework *fw)
 			bool guid_skip = false;
 			size_t i;
 
+			bool ret = check_length(fw, entry->length,
+					FWTS_ACPI_NFIT_MINLEN_SYSTEM_ADDRESS,
+					FWTS_ACPI_NFIT_NAME_SYSTEM_ADDRESS);
+			if (!ret) {
+				passed = false;
+				break;
+			}
+
 			fwts_guid_buf_to_str(nfit_struct->range_guid, guid_str, sizeof(guid_str));
 
 			fwts_log_info_verbatim(fw, "    SPA Range Structure Index:              0x%4.4" PRIx16, nfit_struct->range_index);
@@ -180,6 +198,14 @@  static int nfit_test1(fwts_framework *fw)
 		} else if (entry->type == FWTS_ACPI_NFIT_TYPE_MEMORY_MAP) {
 			fwts_acpi_table_nfit_memory_map *nfit_struct = (fwts_acpi_table_nfit_memory_map *) entry;
 
+			bool ret = check_length(fw, entry->length,
+					FWTS_ACPI_NFIT_MINLEN_MEMORY_MAP,
+					FWTS_ACPI_NFIT_NAME_MEMORY_MAP);
+			if (!ret) {
+				passed = false;
+				break;
+			}
+
 			fwts_log_info_verbatim(fw, "    NFIT Device Handle:                     0x%8.8" PRIx32, nfit_struct->device_handle);
 			fwts_log_info_verbatim(fw, "    NVDIMM Physical ID:                     0x%4.4" PRIx16, nfit_struct->physical_id);
 			fwts_log_info_verbatim(fw, "    NVDIMM Region ID:                       0x%4.4" PRIx16, nfit_struct->region_id);
@@ -202,11 +228,28 @@  static int nfit_test1(fwts_framework *fw)
 			fwts_acpi_table_nfit_interleave *nfit_struct = (fwts_acpi_table_nfit_interleave *) entry;
 			uint32_t i;
 
+			bool ret = check_length(fw, entry->length,
+					FWTS_ACPI_NFIT_MINLEN_INTERLEAVE,
+					FWTS_ACPI_NFIT_NAME_INTERLEAVE);
+			if (!ret) {
+				passed = false;
+				break;
+			}
+
 			fwts_log_info_verbatim(fw, "    Interleave Structure Index:             0x%4.4" PRIx16, nfit_struct->interleave_index);
 			fwts_log_info_verbatim(fw, "    Reserved:                               0x%4.4" PRIx16, nfit_struct->reserved);
 			fwts_log_info_verbatim(fw, "    Number of Lines Described:              0x%8.8" PRIx32, nfit_struct->line_count);
 			fwts_log_info_verbatim(fw, "    Line Size:                              0x%8.8" PRIx32, nfit_struct->line_size);
 
+			ret = check_length(fw, entry->length,
+					FWTS_ACPI_NFIT_MINLEN_INTERLEAVE +
+						nfit_struct->line_count * sizeof nfit_struct->line_offset[0],
+					FWTS_ACPI_NFIT_NAME_INTERLEAVE);
+			if (!ret) {
+				passed = false;
+				break;
+			}
+
 			for (i = 0; i < nfit_struct->line_count; i++) {
 				fwts_log_info_verbatim(fw, "    Line Offset:                            0x%8.8"  PRIx32, nfit_struct->line_offset[i]);
 
@@ -230,20 +273,28 @@  static int nfit_test1(fwts_framework *fw)
 		} else if (entry->type == FWTS_ACPI_NFIT_TYPE_SMBIOS) {
 			fwts_acpi_table_nfit_smbios *nfit_struct = (fwts_acpi_table_nfit_smbios *) entry;
 
+			bool ret = check_length(fw, entry->length,
+					FWTS_ACPI_NFIT_MINLEN_SMBIOS,
+					FWTS_ACPI_NFIT_NAME_SMBIOS);
+			if (!ret) {
+				passed = false;
+				break;
+			}
+
 			fwts_log_info_verbatim(fw, "    Reserved:                               0x%8.8" PRIx32, nfit_struct->reserved);
 
 		} else if (entry->type == FWTS_ACPI_NFIT_TYPE_CONTROL_REGION) {
 			fwts_acpi_table_nfit_control_range *nfit_struct = (fwts_acpi_table_nfit_control_range *) entry;
 			uint64_t reserved1;
 
-			reserved1 = (uint64_t) nfit_struct->reserved1[0] + ((uint64_t) nfit_struct->reserved1[1] << 8) +
-				   ((uint64_t) nfit_struct->reserved1[2] << 16) + ((uint64_t) nfit_struct->reserved1[3] << 24) +
-				   ((uint64_t) nfit_struct->reserved1[4] << 32) + ((uint64_t) nfit_struct->reserved1[5] << 40);
-
-			if (reserved1 != 0)
-				reserved_passed = reserved1;
+			bool ret = check_length(fw, entry->length,
+					FWTS_ACPI_NFIT_MINLEN_CONTROL_REGION,
+					FWTS_ACPI_NFIT_NAME_CONTROL_REGION);
+			if (!ret) {
+				passed = false;
+				break;
+			}
 
-			fwts_log_info_verbatim(fw, "    NVDIMM Control Region Structure Index:  0x%4.4" PRIx16, nfit_struct->region_index);
 			fwts_log_info_verbatim(fw, "    Vendor ID:                              0x%4.4" PRIx16, nfit_struct->vendor_id);
 			fwts_log_info_verbatim(fw, "    Device ID:                              0x%4.4" PRIx16, nfit_struct->device_id);
 			fwts_log_info_verbatim(fw, "    Revision ID:                            0x%4.4" PRIx16, nfit_struct->revision_id);
@@ -257,13 +308,6 @@  static int nfit_test1(fwts_framework *fw)
 			fwts_log_info_verbatim(fw, "    Serial Number:                          0x%8.8" PRIx32, nfit_struct->serial_number);
 			fwts_log_info_verbatim(fw, "    Region Format Interface Code:           0x%4.4" PRIx16, nfit_struct->interface_code);
 			fwts_log_info_verbatim(fw, "    Number of Block Control Windows:        0x%4.4" PRIx16, nfit_struct->windows_num);
-			fwts_log_info_verbatim(fw, "    Size of Block Control Window:           0x%16.16" PRIx64, nfit_struct->window_size);
-			fwts_log_info_verbatim(fw, "    Command Register Offset:                0x%16.16" PRIx64, nfit_struct->command_offset);
-			fwts_log_info_verbatim(fw, "    Size of Command Register:               0x%16.16" PRIx64, nfit_struct->command_size);
-			fwts_log_info_verbatim(fw, "    Status RegisterOffset:                  0x%16.16" PRIx64, nfit_struct->status_offset);
-			fwts_log_info_verbatim(fw, "    Size of Status Register:                0x%16.16" PRIx64, nfit_struct->status_size);
-			fwts_log_info_verbatim(fw, "    NVDIMM Control Region Flag:             0x%4.4" PRIx16, nfit_struct->flags);
-			fwts_log_info_verbatim(fw, "    Reserved:                               0x%16.16" PRIx64, reserved1);
 
 			if (nfit_struct->revision_id & 0xFF00) {
 				passed = false;
@@ -285,11 +329,38 @@  static int nfit_test1(fwts_framework *fw)
 				reserved_passed = nfit_struct->reserved;
 
 			fwts_acpi_reserved_bits_check(fw, "NFIT", "Valid", nfit_struct->valid_fields, sizeof(nfit_struct->valid_fields), 1, 7, &passed);
-			fwts_acpi_reserved_bits_check(fw, "NFIT", "NVDIMM Control Region Flags", nfit_struct->flags, sizeof(nfit_struct->flags), 1, 15, &passed);
+
+			if (entry->length >= sizeof(*nfit_struct)) {
+				reserved1 = (uint64_t) nfit_struct->reserved1[0] + ((uint64_t) nfit_struct->reserved1[1] << 8) +
+					   ((uint64_t) nfit_struct->reserved1[2] << 16) + ((uint64_t) nfit_struct->reserved1[3] << 24) +
+					   ((uint64_t) nfit_struct->reserved1[4] << 32) + ((uint64_t) nfit_struct->reserved1[5] << 40);
+
+				if (reserved1 != 0)
+					reserved_passed = reserved1;
+
+				fwts_log_info_verbatim(fw, "    Size of Block Control Window:           0x%16.16" PRIx64, nfit_struct->window_size);
+				fwts_log_info_verbatim(fw, "    Command Register Offset:                0x%16.16" PRIx64, nfit_struct->command_offset);
+				fwts_log_info_verbatim(fw, "    Size of Command Register:               0x%16.16" PRIx64, nfit_struct->command_size);
+				fwts_log_info_verbatim(fw, "    Status RegisterOffset:                  0x%16.16" PRIx64, nfit_struct->status_offset);
+				fwts_log_info_verbatim(fw, "    Size of Status Register:                0x%16.16" PRIx64, nfit_struct->status_size);
+				fwts_log_info_verbatim(fw, "    NVDIMM Control Region Flag:             0x%4.4" PRIx16, nfit_struct->flags);
+				fwts_log_info_verbatim(fw, "    Reserved:                               0x%16.16" PRIx64, reserved1);
+
+				fwts_acpi_reserved_bits_check(fw, "NFIT", "NVDIMM Control Region Flags", nfit_struct->flags, sizeof(nfit_struct->flags), 1, 15, &passed);
+				fwts_log_info_verbatim(fw, "    NVDIMM Control Region Structure Index:  0x%4.4" PRIx16, nfit_struct->region_index);
+			}
 
 		} else if (entry->type == FWTS_ACPI_NFIT_TYPE_DATA_REGION) {
 			fwts_acpi_table_nfit_data_range *nfit_struct = (fwts_acpi_table_nfit_data_range *) entry;
 
+			bool ret = check_length(fw, entry->length,
+					FWTS_ACPI_NFIT_MINLEN_DATA_REGION,
+					FWTS_ACPI_NFIT_NAME_DATA_REGION);
+			if (!ret) {
+				passed = false;
+				break;
+			}
+
 			fwts_log_info_verbatim(fw, "    NVDIMM Control Region Structure Index:  0x%4.4" PRIx16, nfit_struct->region_index);
 			fwts_log_info_verbatim(fw, "    Number of Block Data Windows:           0x%4.4" PRIx16, nfit_struct->window_num);
 			fwts_log_info_verbatim(fw, "    Block Data Window Start Offset:         0x%16.16" PRIx64, nfit_struct->window_offset);
@@ -309,6 +380,14 @@  static int nfit_test1(fwts_framework *fw)
 			uint64_t reserved;
 			uint16_t i;
 
+			bool ret = check_length(fw, entry->length,
+					FWTS_ACPI_NFIT_MINLEN_FLUSH_ADDRESS,
+					FWTS_ACPI_NFIT_NAME_FLUSH_ADDRESS);
+			if (!ret) {
+				passed = false;
+				break;
+			}
+
 			reserved = (uint64_t) nfit_struct->reserved[0] + ((uint64_t) nfit_struct->reserved[1] << 8) +
 				   ((uint64_t) nfit_struct->reserved[2] << 16) + ((uint64_t) nfit_struct->reserved[3] << 24) +
 				   ((uint64_t) nfit_struct->reserved[4] << 32) + ((uint64_t) nfit_struct->reserved[5] << 40);
@@ -316,6 +395,16 @@  static int nfit_test1(fwts_framework *fw)
 			fwts_log_info_verbatim(fw, "    NFIT Device Handle:                     0x%8.8" PRIx32, nfit_struct->device_handle);
 			fwts_log_info_verbatim(fw, "    Number of Flush Hint Addresses:         0x%4.4" PRIx16, nfit_struct->hint_count);
 			fwts_log_info_verbatim(fw, "    Reserved:                               0x%16.16" PRIx64, reserved);
+
+			ret = check_length(fw, entry->length,
+					FWTS_ACPI_NFIT_MINLEN_FLUSH_ADDRESS +
+						nfit_struct->hint_count * sizeof nfit_struct->hint_address[0],
+					FWTS_ACPI_NFIT_NAME_FLUSH_ADDRESS);
+			if (!ret) {
+				passed = false;
+				break;
+			}
+
 			for (i = 0; i < nfit_struct->hint_count; i++)
 				fwts_log_info_verbatim(fw, "    Flush Hint Address:                     0x%16.16" PRIx64, nfit_struct->hint_address[i]);
 
@@ -326,6 +415,14 @@  static int nfit_test1(fwts_framework *fw)
 			fwts_acpi_table_nfit_platform_cap *nfit_struct = (fwts_acpi_table_nfit_platform_cap *) entry;
 			uint32_t reserved1;
 
+			bool ret = check_length(fw, entry->length,
+					FWTS_ACPI_NFIT_MINLEN_PLATFORM_CAPABILITY,
+					FWTS_ACPI_NFIT_NAME_PLATFORM_CAPABILITY);
+			if (!ret) {
+				passed = false;
+				break;
+			}
+
 			reserved1 = (uint32_t) nfit_struct->reserved1[0] + ((uint32_t) nfit_struct->reserved1[1] << 8) +
 				   ((uint32_t) nfit_struct->reserved1[2] << 16);
 
diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h
index e550cd76..10dedb6a 100644
--- a/src/lib/include/fwts_acpi.h
+++ b/src/lib/include/fwts_acpi.h
@@ -1181,6 +1181,24 @@  typedef enum {
 	FWTS_ACPI_NFIT_TYPE_RESERVED             = 8     /* >= 8 are reserved */
 } fwts_acpi_nfit_type;
 
+#define FWTS_ACPI_NFIT_NAME_SYSTEM_ADDRESS      "SPA Range structure"
+#define FWTS_ACPI_NFIT_NAME_MEMORY_MAP          "NVDIMM Region Mapping structure"
+#define FWTS_ACPI_NFIT_NAME_INTERLEAVE          "Interleave structure"
+#define FWTS_ACPI_NFIT_NAME_SMBIOS              "SMBIOS Management Information structure"
+#define FWTS_ACPI_NFIT_NAME_CONTROL_REGION      "NVDIMM Control Region structure"
+#define FWTS_ACPI_NFIT_NAME_DATA_REGION         "NVDIMM Block Data Window Region structure"
+#define FWTS_ACPI_NFIT_NAME_FLUSH_ADDRESS       "Flush Hint Address structure"
+#define FWTS_ACPI_NFIT_NAME_PLATFORM_CAPABILITY "Platform Capabilities structure"
+
+#define FWTS_ACPI_NFIT_MINLEN_SYSTEM_ADDRESS      56
+#define FWTS_ACPI_NFIT_MINLEN_MEMORY_MAP          48
+#define FWTS_ACPI_NFIT_MINLEN_INTERLEAVE          16
+#define FWTS_ACPI_NFIT_MINLEN_SMBIOS              8
+#define FWTS_ACPI_NFIT_MINLEN_CONTROL_REGION      32
+#define FWTS_ACPI_NFIT_MINLEN_DATA_REGION         32
+#define FWTS_ACPI_NFIT_MINLEN_FLUSH_ADDRESS       16
+#define FWTS_ACPI_NFIT_MINLEN_PLATFORM_CAPABILITY 16
+
 typedef struct {
 	fwts_acpi_table_nfit_struct_header	header;
 	uint16_t	range_index;