[PATCH] fwts:dmicheck: replace memcpy with fwts_memcpy_unaligned for SMBIOS 3.0 table
wufan
wufan at codeaurora.org
Tue Nov 29 20:37:36 UTC 2016
>From f507dd17853f7e17cecfa2d32d28448913c60873 Mon Sep 17 00:00:00 2001
From: Fan Wu <wufan at codeaurora.org>
Date: Tue, 29 Nov 2016 20:28:13 +0000
Subject: [PATCH] fwts:dmicheck: replace memcpy with fwts_memcpy_unaligned
for
SMBIOS 3.0 table
Mapping of SMBIOS 3.0 table in AARCH64 platforms through /dev/mem
has the memory tagged as device memory, and memcpy could trigger
alignment fault if the SMBIOS table is of odd size. The fix is to
replace memcpy with fwts_memcpy_unaligned which does byte copy.
---
src/dmi/dmicheck/dmicheck.c | 4 ++--
src/lib/include/fwts_stringextras.h | 1 +
src/lib/src/fwts_stringextras.c | 16 ++++++++++++++++
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/dmi/dmicheck/dmicheck.c b/src/dmi/dmicheck/dmicheck.c
index 1934ebe..62eef9c 100644
--- a/src/dmi/dmicheck/dmicheck.c
+++ b/src/dmi/dmicheck/dmicheck.c
@@ -370,8 +370,8 @@ static void* dmi_table_smbios30(fwts_framework *fw,
fwts_smbios30_entry *entry)
mem = fwts_mmap(addr, length);
if (mem != FWTS_MAP_FAILED) {
table = malloc(length);
- if (table)
- memcpy(table, mem, length);
+ if (table)
+ fwts_memcpy_unaligned(table, mem, length);
(void)fwts_munmap(mem, length);
return table;
}
diff --git a/src/lib/include/fwts_stringextras.h
b/src/lib/include/fwts_stringextras.h
index 6256254..b364127 100644
--- a/src/lib/include/fwts_stringextras.h
+++ b/src/lib/include/fwts_stringextras.h
@@ -25,5 +25,6 @@
void fwts_chop_newline(char *str);
char *fwts_realloc_strcat(char *orig, const char *newstr);
char *fwts_string_endswith(const char *str, const char *postfix);
+void fwts_memcpy_unaligned(void *dst, const void *src, size_t n);
#endif
diff --git a/src/lib/src/fwts_stringextras.c
b/src/lib/src/fwts_stringextras.c
index 17d833e..25465a8 100644
--- a/src/lib/src/fwts_stringextras.c
+++ b/src/lib/src/fwts_stringextras.c
@@ -92,3 +92,19 @@ char* fwts_string_endswith(const char *str, const char
*postfix)
return (char*) str + sl - pl;
}
+
+
+/*
+ * fwts_memcpy_unaligned()
+ * perform byte copy of n bytes from src to dst.
+ */
+void fwts_memcpy_unaligned(void *dst, const void *src, size_t n)
+{
+ size_t i = n;
+
+ if (dst == NULL || src == NULL || n == 0 )
+ return;
+
+ while (i--)
+ ((uint8_t*)dst)[i] = ((uint8_t*)src)[i];
+}
--
2.7.4
-----Original Message-----
From: Colin Ian King [mailto:colin.king at canonical.com]
Sent: Monday, November 28, 2016 1:50 PM
To: wufan <wufan at codeaurora.org>; fwts-devel at lists.ubuntu.com
Subject: Re: [PATCH] fwts:dmicheck: replace memcpy with byte copy for SMBIOS
3.0 table
On 28/11/16 20:12, wufan wrote:
> This is a real issue. Without the patch the dmicheck test will fail
> with
> segfault:
>
> ubuntu at ubuntu:~/fwts/src$ sudo ./fwts dmicheck - Results generated by
> fwts: Version V16.11.00 (2016-11-10 08:39:49).
>
> Some of this work - Copyright (c) 1999 - 2016, Intel Corp. All rights
> reserved.
> Some of this work - Copyright (c) 2010 - 2016, Canonical.
> Some of this work - Copyright (c) 2016 IBM.
>
> This test run on 28/11/16 at 18:56:40 on host Linux ubuntu 4.7.0 #2
> SMP Sun Oct 16 14:57:06 MDT 2016 aarch64.
>
> Command: "fwts dmicheck -".
> Running tests: dmicheck.
>
> dmicheck: DMI/SMBIOS table tests.
> ----------------------------------------------------------------------
> ------
> ----------------------------------------------------
> Test 1 of 3: Find and test SMBIOS Table Entry Points.
> This test tries to find and sanity check the SMBIOS data structures.
> PASSED: Test 1, Found SMBIOS30 Table Entry Point at 0x43fffb0000
> SMBIOS30 Entry Point Structure:
> Anchor String : _SM3_
> Checksum : 0x9b
> Entry Point Length : 0x18
> Major Version : 0x03
> Minor Version : 0x00
> Docrev : 0x00
> Entry Point Revision : 0x01
> Reserved : 0x00
> Table maximum size : 0x0000263d
> Table address : 0x00000043f9190000
>
> PASSED: Test 1, SMBIOS30 Table Entry Point Checksum is valid.
> PASSED: Test 1, SMBIOS30 Table Entry Point Length is valid.
>
> Caught SIGNAL 11 (Segmentation fault), aborting.
> Backtrace:
> 0x0000ffffac10ff84
> /home/ubuntu/fwts/src/lib/src/.libs/libfwts.so.1(+0xff84)
>
>
> Fan
>
>
> -----Original Message-----
> From: Colin Ian King [mailto:colin.king at canonical.com]
> Sent: Monday, November 28, 2016 1:08 PM
> To: wufan <wufan at codeaurora.org>; fwts-devel at lists.ubuntu.com
> Subject: Re: [PATCH] fwts:dmicheck: replace memcpy with byte copy for
> SMBIOS
> 3.0 table
>
> On 28/11/16 18:26, wufan wrote:
>> From 00cae446405d4d856fbd56222f835c207d18d8e8 Mon Sep 17 00:00:00
>> 2001
>>
>> From: Fan Wu <wufan at codeaurora.org>
>>
>> Date: Thu, 3 Nov 2016 14:03:40 -0600
>>
>> Subject: [PATCH] fwts:dmicheck: replace memcpy with byte copy for
>> SMBIOS 3.0
>>
>> table
>>
>>
>>
>> Mapping of SMBIOS 3.0 table in AARCH64 platforms through /dev/mem
>>
>> has the memory tagged as device memory, and memcpy could trigger
>>
>> alignment fault if the SMBIOS table is of odd size. The fix is to
>>
>> replace memcpy with byte copy.
>>
>>
>>
>> Change-Id: I52a5be2fedcd057fdd5e510ff090e4129f128221
>>
>> ---
>>
>> src/dmi/dmicheck/dmicheck.c | 7 +++++--
>>
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>>
>>
>> diff --git a/src/dmi/dmicheck/dmicheck.c
>> b/src/dmi/dmicheck/dmicheck.c
>>
>> index 1934ebe..98db76b 100644
>>
>> --- a/src/dmi/dmicheck/dmicheck.c
>>
>> +++ b/src/dmi/dmicheck/dmicheck.c
>>
>> @@ -370,8 +370,11 @@ static void* dmi_table_smbios30(fwts_framework
>> *fw, fwts_smbios30_entry *entry)
>>
>> mem = fwts_mmap(addr, length);
>>
>> if (mem != FWTS_MAP_FAILED) {
>>
>> table = malloc(length);
>>
>> - if (table)
>>
>> - memcpy(table, mem, length);
>>
>> + if (table) {
>>
>> + size_t i = length;
>>
>> + while (i--)
>>
>> + ((uint8_t*)table)[i] =
>> + ((uint8_t*)mem)[i];
>>
>> + }
>>
>> (void)fwts_munmap(mem, length);
>>
>> return table;
>>
>> }
>>
>> --
>>
>> 1.8.2.1
>>
> Is this a hypothetical issue or does it fix an observed issue with
> non-aligned SMBIOS tables?
>
> Colin
>
>
OK. I'm surprised that memcpy is tripping this, I thought it did unaligned
copying correctly.
I'm inclined to suggest adding a fwts_memcpy_unaligned() to
src/lib/src/fwts_stringextras.c and the prototype in
src/lib/include/fwts_stringextras.h and using this. I suspect we may need
this kind of unaligned memcpy again in the future.
Colin
More information about the fwts-devel
mailing list