[v2] DMI: Fix the way to load SMBIOS 3.x tables via sysfs
diff mbox series

Message ID 20180823024131.5130-1-ge.song@hxt-semitech.com
State Accepted
Headers show
Series
  • [v2] DMI: Fix the way to load SMBIOS 3.x tables via sysfs
Related show

Commit Message

Ge Song Aug. 23, 2018, 2:41 a.m. UTC
From: Ge Song <ge.song@hxt-semitech.com>

In SMBIOS 3.x specs, the meaning of "Structure table maximum size"
field in SMBIOS Entry Point Structure is "Maximum size of SMBIOS
Structure Table, pointed to by the Structure Table Address, in bytes.
The actual size is guaranteed to be less or equal to the maximum size."

Thus we cannot assume the whole size of SMBIOS segment equals to the
value of entry->struct_table_length.

This fix the DMI test failure in some platforms when the real size of
SMBIOS segment are smaller than the value in "Structure table maximum
size" field

Signed-off-by: Ge Song <ge.song@hxt-semitech.com>
---
Change since V1:
  * Correct the increase of buffer pointer p in for loop.
  * Since the open() has no O_NONBLOCK flag, remove the unnecessary 
    judgement.
 src/dmi/dmicheck/dmicheck.c | 32 +++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

Comments

Alex Hung Sept. 3, 2018, 5:22 a.m. UTC | #1
On 2018-08-22 07:41 PM, songgebird@gmail.com wrote:
> From: Ge Song <ge.song@hxt-semitech.com>
> 
> In SMBIOS 3.x specs, the meaning of "Structure table maximum size"
> field in SMBIOS Entry Point Structure is "Maximum size of SMBIOS
> Structure Table, pointed to by the Structure Table Address, in bytes.
> The actual size is guaranteed to be less or equal to the maximum size."
> 
> Thus we cannot assume the whole size of SMBIOS segment equals to the
> value of entry->struct_table_length.
> 
> This fix the DMI test failure in some platforms when the real size of
> SMBIOS segment are smaller than the value in "Structure table maximum
> size" field
> 
> Signed-off-by: Ge Song <ge.song@hxt-semitech.com>
> ---
> Change since V1:
>    * Correct the increase of buffer pointer p in for loop.
>    * Since the open() has no O_NONBLOCK flag, remove the unnecessary
>      judgement.
>   src/dmi/dmicheck/dmicheck.c | 32 +++++++++++++++++++-
>   1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/src/dmi/dmicheck/dmicheck.c b/src/dmi/dmicheck/dmicheck.c
> index 8497c2ab..a95b47c0 100644
> --- a/src/dmi/dmicheck/dmicheck.c
> +++ b/src/dmi/dmicheck/dmicheck.c
> @@ -276,6 +276,36 @@ static int dmi_load_file(const char* filename, void *buf, size_t size)
>   	return FWTS_OK;
>   }
>   
> +static int dmi_load_file_variable_size(const char* filename, void *buf, size_t *size)
> +{
> +	int fd;
> +	char *p;
> +	ssize_t count;
> +	size_t sz, total;
> +
> +	sz = *size;
> +	(void)memset(buf, 0, sz);
> +
> +	if ((fd = open(filename, O_RDONLY)) < 0)
> +		return FWTS_ERROR;
> +
> +	for (p = buf, total = count = 0; ; p += count) {
> +		count = read(fd, p, sz - total);
> +		if (count == -1) {
> +			close(fd);
> +			return FWTS_ERROR;
> +		}
> +		if (count == 0)
> +			break;
> +
> +		total += (size_t)count;
> +	}
> +
> +	(void)close(fd);
> +	*size = total;
> +	return FWTS_OK;
> +}
> +
>   static void* dmi_table_smbios(fwts_framework *fw, fwts_smbios_entry *entry)
>   {
>   	off_t addr = (off_t)entry->struct_table_address;
> @@ -358,7 +388,7 @@ static void* dmi_table_smbios30(fwts_framework *fw, fwts_smbios30_entry *entry)
>   		table = malloc(length);
>   		if (!table)
>   			return NULL;
> -		if (dmi_load_file("/sys/firmware/dmi/tables/DMI", table, length) == FWTS_OK) {
> +		if (dmi_load_file_variable_size("/sys/firmware/dmi/tables/DMI", table, &length) == FWTS_OK) {
>   			fwts_log_info(fw, "SMBIOS30 table loaded from /sys/firmware/dmi/tables/DMI\n");
>   			return table;
>   		}
> 


Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu Sept. 10, 2018, 3:33 a.m. UTC | #2
On 08/23/2018 10:41 AM, songgebird@gmail.com wrote:
> From: Ge Song <ge.song@hxt-semitech.com>
>
> In SMBIOS 3.x specs, the meaning of "Structure table maximum size"
> field in SMBIOS Entry Point Structure is "Maximum size of SMBIOS
> Structure Table, pointed to by the Structure Table Address, in bytes.
> The actual size is guaranteed to be less or equal to the maximum size."
>
> Thus we cannot assume the whole size of SMBIOS segment equals to the
> value of entry->struct_table_length.
>
> This fix the DMI test failure in some platforms when the real size of
> SMBIOS segment are smaller than the value in "Structure table maximum
> size" field
>
> Signed-off-by: Ge Song <ge.song@hxt-semitech.com>
> ---
> Change since V1:
>   * Correct the increase of buffer pointer p in for loop.
>   * Since the open() has no O_NONBLOCK flag, remove the unnecessary 
>     judgement.
>  src/dmi/dmicheck/dmicheck.c | 32 +++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/src/dmi/dmicheck/dmicheck.c b/src/dmi/dmicheck/dmicheck.c
> index 8497c2ab..a95b47c0 100644
> --- a/src/dmi/dmicheck/dmicheck.c
> +++ b/src/dmi/dmicheck/dmicheck.c
> @@ -276,6 +276,36 @@ static int dmi_load_file(const char* filename, void *buf, size_t size)
>  	return FWTS_OK;
>  }
>  
> +static int dmi_load_file_variable_size(const char* filename, void *buf, size_t *size)
> +{
> +	int fd;
> +	char *p;
> +	ssize_t count;
> +	size_t sz, total;
> +
> +	sz = *size;
> +	(void)memset(buf, 0, sz);
> +
> +	if ((fd = open(filename, O_RDONLY)) < 0)
> +		return FWTS_ERROR;
> +
> +	for (p = buf, total = count = 0; ; p += count) {
> +		count = read(fd, p, sz - total);
> +		if (count == -1) {
> +			close(fd);
> +			return FWTS_ERROR;
> +		}
> +		if (count == 0)
> +			break;
> +
> +		total += (size_t)count;
> +	}
> +
> +	(void)close(fd);
> +	*size = total;
> +	return FWTS_OK;
> +}
> +
>  static void* dmi_table_smbios(fwts_framework *fw, fwts_smbios_entry *entry)
>  {
>  	off_t addr = (off_t)entry->struct_table_address;
> @@ -358,7 +388,7 @@ static void* dmi_table_smbios30(fwts_framework *fw, fwts_smbios30_entry *entry)
>  		table = malloc(length);
>  		if (!table)
>  			return NULL;
> -		if (dmi_load_file("/sys/firmware/dmi/tables/DMI", table, length) == FWTS_OK) {
> +		if (dmi_load_file_variable_size("/sys/firmware/dmi/tables/DMI", table, &length) == FWTS_OK) {
>  			fwts_log_info(fw, "SMBIOS30 table loaded from /sys/firmware/dmi/tables/DMI\n");
>  			return table;
>  		}
Acked-by: Ivan Hu <ivan.hu@canonical.com>

Patch
diff mbox series

diff --git a/src/dmi/dmicheck/dmicheck.c b/src/dmi/dmicheck/dmicheck.c
index 8497c2ab..a95b47c0 100644
--- a/src/dmi/dmicheck/dmicheck.c
+++ b/src/dmi/dmicheck/dmicheck.c
@@ -276,6 +276,36 @@  static int dmi_load_file(const char* filename, void *buf, size_t size)
 	return FWTS_OK;
 }
 
+static int dmi_load_file_variable_size(const char* filename, void *buf, size_t *size)
+{
+	int fd;
+	char *p;
+	ssize_t count;
+	size_t sz, total;
+
+	sz = *size;
+	(void)memset(buf, 0, sz);
+
+	if ((fd = open(filename, O_RDONLY)) < 0)
+		return FWTS_ERROR;
+
+	for (p = buf, total = count = 0; ; p += count) {
+		count = read(fd, p, sz - total);
+		if (count == -1) {
+			close(fd);
+			return FWTS_ERROR;
+		}
+		if (count == 0)
+			break;
+
+		total += (size_t)count;
+	}
+
+	(void)close(fd);
+	*size = total;
+	return FWTS_OK;
+}
+
 static void* dmi_table_smbios(fwts_framework *fw, fwts_smbios_entry *entry)
 {
 	off_t addr = (off_t)entry->struct_table_address;
@@ -358,7 +388,7 @@  static void* dmi_table_smbios30(fwts_framework *fw, fwts_smbios30_entry *entry)
 		table = malloc(length);
 		if (!table)
 			return NULL;
-		if (dmi_load_file("/sys/firmware/dmi/tables/DMI", table, length) == FWTS_OK) {
+		if (dmi_load_file_variable_size("/sys/firmware/dmi/tables/DMI", table, &length) == FWTS_OK) {
 			fwts_log_info(fw, "SMBIOS30 table loaded from /sys/firmware/dmi/tables/DMI\n");
 			return table;
 		}