nfit: scan SMBIOS Management Information structure

Message ID 20180706181354.57863-1-elliott@hpe.com
State Accepted
Headers show
Series
  • nfit: scan SMBIOS Management Information structure
Related show

Commit Message

Elliott, Robert (Persistent Memory) July 6, 2018, 6:13 p.m.
Perform a scan of the ACPI NFIT SMBIOS Management Information structure,
which contains extra SMBIOS structures that were not present at boot
and included in the real SMBIOS table (e.g., for hot-plugged Memory
Devices (type 17)).  Print the header information, including the
number of string bytes, and check that the lengths are plausible.

This could be enhanced to share functions in dmicheck.c like
dmi_scan_tables and dmicheck_entry to do more detailed checks.

Signed-off-by: Robert Elliott <elliott@hpe.com>
---
 src/acpi/nfit/nfit.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

Comments

Alex Hung July 9, 2018, 8:14 p.m. | #1
On 2018-07-06 11:13 AM, Robert Elliott wrote:
> Perform a scan of the ACPI NFIT SMBIOS Management Information structure,
> which contains extra SMBIOS structures that were not present at boot
> and included in the real SMBIOS table (e.g., for hot-plugged Memory
> Devices (type 17)).  Print the header information, including the
> number of string bytes, and check that the lengths are plausible.
> 
> This could be enhanced to share functions in dmicheck.c like
> dmi_scan_tables and dmicheck_entry to do more detailed checks.
> 
> Signed-off-by: Robert Elliott <elliott@hpe.com>
> ---
>   src/acpi/nfit/nfit.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 53 insertions(+)
> 
> diff --git a/src/acpi/nfit/nfit.c b/src/acpi/nfit/nfit.c
> index 961e5f28..53c86c24 100644
> --- a/src/acpi/nfit/nfit.c
> +++ b/src/acpi/nfit/nfit.c
> @@ -50,6 +50,50 @@ static bool check_length(fwts_framework *fw, int actual, int min, const char *na
>   	return true;
>   }
>   
> +static bool scan_nfit_smbios(fwts_framework *fw, int len, uint8_t *table) {
> +	fwts_dmi_header *hdr;
> +	int entry = 0;
> +
> +	while (len > 4) {
> +		int strbytes = 0;
> +
> +		hdr = (fwts_dmi_header *) table;
> +
> +		fwts_log_info_verbatim(fw, "  NFIT SMBIOS Entry %d:", entry++);
> +		fwts_log_info_verbatim(fw, "    Type:                                   0x%2.2" PRIx8, hdr->type);
> +		fwts_log_info_verbatim(fw, "    Length:                                 0x%2.2" PRIx8, hdr->length);
> +		fwts_log_info_verbatim(fw, "    Handle:                                 0x%4.4" PRIx16, hdr->handle);
> +
> +		if (hdr->length < 4) {
> +			fwts_failed(fw, LOG_LEVEL_HIGH, "NFIT_SMBIOS_EntryLength",
> +				    "NFIT SMBIOS Entry length %d bytes is too short (less than 4 bytes)",
> +				    hdr->length);
> +			return false;
> +		}
> +		len -= hdr->length;
> +		table += hdr->length;
> +
> +		/* Look for structure terminator, ends in two zero bytes */
> +		while (len > 2 && (table[0] != 0 || table[1] != 0)) {
> +			strbytes++;
> +		        table++;
> +			len--;
> +		}
> +		fwts_log_info_verbatim(fw, "    Strings:                                %d bytes", strbytes);
> +
> +		/* Skip over terminating two zero bytes, see section 6.1 of spec */
> +		table += 2;
> +		len -= 2;
> +
> +	}
> +	if (len) {
> +		fwts_failed(fw, LOG_LEVEL_HIGH, "NFIT_SMBIOS_ListLength",
> +			    "NFIT SMBIOS structure does not end with a complete entry");
> +		return false;
> +	}
> +	return true;
> +}
> +
>   static int nfit_init(fwts_framework *fw)
>   {
>   	if (fwts_acpi_find_table(fw, "NFIT", 0, &table) != FWTS_OK) {
> @@ -278,6 +322,15 @@ static int nfit_test1(fwts_framework *fw)
>   			}
>   
>   			fwts_log_info_verbatim(fw, "    Reserved:                               0x%8.8" PRIx32, nfit_struct->reserved);
> +			if (nfit_struct->reserved != 0)
> +				reserved_passed = nfit_struct->reserved;
> +
> +			if (entry->length < 8)
> +				break;
> +
> +			ret = scan_nfit_smbios(fw, entry->length - 8, nfit_struct->smbios);
> +			if (!ret)
> +				passed = false;
>   
>   		} 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;
> 


Acked-by: Alex Hung <alex.hung@canonical.com>
ivanhu July 16, 2018, 10 a.m. | #2
On 07/07/2018 02:13 AM, Robert Elliott wrote:
> Perform a scan of the ACPI NFIT SMBIOS Management Information structure,
> which contains extra SMBIOS structures that were not present at boot
> and included in the real SMBIOS table (e.g., for hot-plugged Memory
> Devices (type 17)).  Print the header information, including the
> number of string bytes, and check that the lengths are plausible.
>
> This could be enhanced to share functions in dmicheck.c like
> dmi_scan_tables and dmicheck_entry to do more detailed checks.
>
> Signed-off-by: Robert Elliott <elliott@hpe.com>
> ---
>  src/acpi/nfit/nfit.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>
> diff --git a/src/acpi/nfit/nfit.c b/src/acpi/nfit/nfit.c
> index 961e5f28..53c86c24 100644
> --- a/src/acpi/nfit/nfit.c
> +++ b/src/acpi/nfit/nfit.c
> @@ -50,6 +50,50 @@ static bool check_length(fwts_framework *fw, int actual, int min, const char *na
>  	return true;
>  }
>  
> +static bool scan_nfit_smbios(fwts_framework *fw, int len, uint8_t *table) {
> +	fwts_dmi_header *hdr;
> +	int entry = 0;
> +
> +	while (len > 4) {
> +		int strbytes = 0;
> +
> +		hdr = (fwts_dmi_header *) table;
> +
> +		fwts_log_info_verbatim(fw, "  NFIT SMBIOS Entry %d:", entry++);
> +		fwts_log_info_verbatim(fw, "    Type:                                   0x%2.2" PRIx8, hdr->type);
> +		fwts_log_info_verbatim(fw, "    Length:                                 0x%2.2" PRIx8, hdr->length);
> +		fwts_log_info_verbatim(fw, "    Handle:                                 0x%4.4" PRIx16, hdr->handle);
> +
> +		if (hdr->length < 4) {
> +			fwts_failed(fw, LOG_LEVEL_HIGH, "NFIT_SMBIOS_EntryLength",
> +				    "NFIT SMBIOS Entry length %d bytes is too short (less than 4 bytes)",
> +				    hdr->length);
> +			return false;
> +		}
> +		len -= hdr->length;
> +		table += hdr->length;
> +
> +		/* Look for structure terminator, ends in two zero bytes */
> +		while (len > 2 && (table[0] != 0 || table[1] != 0)) {
> +			strbytes++;
> +		        table++;
> +			len--;
> +		}
> +		fwts_log_info_verbatim(fw, "    Strings:                                %d bytes", strbytes);
> +
> +		/* Skip over terminating two zero bytes, see section 6.1 of spec */
> +		table += 2;
> +		len -= 2;
> +
> +	}
> +	if (len) {
> +		fwts_failed(fw, LOG_LEVEL_HIGH, "NFIT_SMBIOS_ListLength",
> +			    "NFIT SMBIOS structure does not end with a complete entry");
> +		return false;
> +	}
> +	return true;
> +}
> +
>  static int nfit_init(fwts_framework *fw)
>  {
>  	if (fwts_acpi_find_table(fw, "NFIT", 0, &table) != FWTS_OK) {
> @@ -278,6 +322,15 @@ static int nfit_test1(fwts_framework *fw)
>  			}
>  
>  			fwts_log_info_verbatim(fw, "    Reserved:                               0x%8.8" PRIx32, nfit_struct->reserved);
> +			if (nfit_struct->reserved != 0)
> +				reserved_passed = nfit_struct->reserved;
> +
> +			if (entry->length < 8)
> +				break;
> +
> +			ret = scan_nfit_smbios(fw, entry->length - 8, nfit_struct->smbios);
> +			if (!ret)
> +				passed = false;
>  
>  		} 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;
Acked-by: Ivan Hu <ivan.hu@canonical.com>

Patch

diff --git a/src/acpi/nfit/nfit.c b/src/acpi/nfit/nfit.c
index 961e5f28..53c86c24 100644
--- a/src/acpi/nfit/nfit.c
+++ b/src/acpi/nfit/nfit.c
@@ -50,6 +50,50 @@  static bool check_length(fwts_framework *fw, int actual, int min, const char *na
 	return true;
 }
 
+static bool scan_nfit_smbios(fwts_framework *fw, int len, uint8_t *table) {
+	fwts_dmi_header *hdr;
+	int entry = 0;
+
+	while (len > 4) {
+		int strbytes = 0;
+
+		hdr = (fwts_dmi_header *) table;
+
+		fwts_log_info_verbatim(fw, "  NFIT SMBIOS Entry %d:", entry++);
+		fwts_log_info_verbatim(fw, "    Type:                                   0x%2.2" PRIx8, hdr->type);
+		fwts_log_info_verbatim(fw, "    Length:                                 0x%2.2" PRIx8, hdr->length);
+		fwts_log_info_verbatim(fw, "    Handle:                                 0x%4.4" PRIx16, hdr->handle);
+
+		if (hdr->length < 4) {
+			fwts_failed(fw, LOG_LEVEL_HIGH, "NFIT_SMBIOS_EntryLength",
+				    "NFIT SMBIOS Entry length %d bytes is too short (less than 4 bytes)",
+				    hdr->length);
+			return false;
+		}
+		len -= hdr->length;
+		table += hdr->length;
+
+		/* Look for structure terminator, ends in two zero bytes */
+		while (len > 2 && (table[0] != 0 || table[1] != 0)) {
+			strbytes++;
+		        table++;
+			len--;
+		}
+		fwts_log_info_verbatim(fw, "    Strings:                                %d bytes", strbytes);
+
+		/* Skip over terminating two zero bytes, see section 6.1 of spec */
+		table += 2;
+		len -= 2;
+
+	}
+	if (len) {
+		fwts_failed(fw, LOG_LEVEL_HIGH, "NFIT_SMBIOS_ListLength",
+			    "NFIT SMBIOS structure does not end with a complete entry");
+		return false;
+	}
+	return true;
+}
+
 static int nfit_init(fwts_framework *fw)
 {
 	if (fwts_acpi_find_table(fw, "NFIT", 0, &table) != FWTS_OK) {
@@ -278,6 +322,15 @@  static int nfit_test1(fwts_framework *fw)
 			}
 
 			fwts_log_info_verbatim(fw, "    Reserved:                               0x%8.8" PRIx32, nfit_struct->reserved);
+			if (nfit_struct->reserved != 0)
+				reserved_passed = nfit_struct->reserved;
+
+			if (entry->length < 8)
+				break;
+
+			ret = scan_nfit_smbios(fw, entry->length - 8, nfit_struct->smbios);
+			if (!ret)
+				passed = false;
 
 		} 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;