diff mbox series

[v2,1/1] lib: avoid bus error in fwts_safe_memcpy

Message ID 20201015222100.5880-1-xypron.glpk@gmx.de
State New
Headers show
Series [v2,1/1] lib: avoid bus error in fwts_safe_memcpy | expand

Commit Message

Heinrich Schuchardt Oct. 15, 2020, 10:21 p.m. UTC
The glibc 2.31 implementation of memcpy() may read 8 bytes at once, even if
this exceeds the source buffer. This can lead to a bus error.

Without this patch 'sudo fwts dmicheck -' results in an error when reading
the SMBIOS table on my system:

Test 1 of 4: Find and test SMBIOS Table Entry Points.
This test tries to find and sanity check the SMBIOS data structures.
Cannot read mmap'd SMBIOS entry at 0x0x7aef3000
SMBIOS entry loaded from /sys/firmware/dmi/tables/smbios_entry_point
FAILED [HIGH] SMBIOSNoEntryPoint: Test 1, Could not find any SMBIOS Table Entry Points.

Test 2 of 4: Test DMI/SMBIOS tables for errors.
SKIPPED: Test 2, Cannot find SMBIOS or DMI table entry, skip the test.

The reason is a bus error caught inside fwts_safe_memcpy():

Caught SIGNAL 7 (Bus error), aborting.
Backtrace:
0x0000ffff8e77d848 fwts/src/lib/src/.libs/libfwts.so.1(+0x16848)
0x0000ffff8e8fc7bc linux-vdso.so.1(__kernel_rt_sigreturn+0)
0x0000ffff8e6617ac /lib/aarch64-linux-gnu/libc.so.6(+0x887ac)
0x0000ffff8e7922a4 fwts/src/lib/src/.libs/libfwts.so.1(fwts_safe_memcpy+0x44)
0x0000ffff8e78f84c fwts/src/lib/src/.libs/libfwts.so.1(fwts_smbios_find_entry+0x6c)

The bus error is avoided by replacing memcpy() with a copy loop that
does not read outside the copied range:

PASSED: Test 1, SMBIOS Table Entry Point Checksum is valid.
PASSED: Test 1, SMBIOS Table Entry Point Length is valid.
PASSED: Test 1, SMBIOS Table Entry Intermediate Anchor String _DMI_ is valid.
PASSED: Test 1, SMBIOS Table Entry Point Intermediate Checksum is valid.
SMBIOS table loaded from /sys/firmware/dmi/tables/DMI
PASSED: Test 1, SMBIOS Table Entry Structure Table Address and Length looks valid.

Test 2 of 4: Test DMI/SMBIOS tables for errors.
SMBIOS table loaded from /sys/firmware/dmi/tables/DMI
PASSED: Test 2, Entry @ 0x7aef3020 'BIOS Information (Type 0)'
PASSED: Test 2, Entry @ 0x7aef3065 'System Information (Type 1)'
PASSED: Test 2, Entry @ 0x7aef30b2 'Base Board Information (Type 2)'
PASSED: Test 2, Entry @ 0x7aef30e0 'Chassis Information (Type 3)'

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2:
	move variable declaration to start of function
---
 src/lib/src/fwts_safe_mem.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

--
2.28.0

Comments

Alex Hung Oct. 16, 2020, 8:19 p.m. UTC | #1
On 2020-10-15 4:21 p.m., Heinrich Schuchardt wrote:
> The glibc 2.31 implementation of memcpy() may read 8 bytes at once, even if
> this exceeds the source buffer. This can lead to a bus error.
> 
> Without this patch 'sudo fwts dmicheck -' results in an error when reading
> the SMBIOS table on my system:
> 
> Test 1 of 4: Find and test SMBIOS Table Entry Points.
> This test tries to find and sanity check the SMBIOS data structures.
> Cannot read mmap'd SMBIOS entry at 0x0x7aef3000
> SMBIOS entry loaded from /sys/firmware/dmi/tables/smbios_entry_point
> FAILED [HIGH] SMBIOSNoEntryPoint: Test 1, Could not find any SMBIOS Table Entry Points.
> 
> Test 2 of 4: Test DMI/SMBIOS tables for errors.
> SKIPPED: Test 2, Cannot find SMBIOS or DMI table entry, skip the test.
> 
> The reason is a bus error caught inside fwts_safe_memcpy():
> 
> Caught SIGNAL 7 (Bus error), aborting.
> Backtrace:
> 0x0000ffff8e77d848 fwts/src/lib/src/.libs/libfwts.so.1(+0x16848)
> 0x0000ffff8e8fc7bc linux-vdso.so.1(__kernel_rt_sigreturn+0)
> 0x0000ffff8e6617ac /lib/aarch64-linux-gnu/libc.so.6(+0x887ac)
> 0x0000ffff8e7922a4 fwts/src/lib/src/.libs/libfwts.so.1(fwts_safe_memcpy+0x44)
> 0x0000ffff8e78f84c fwts/src/lib/src/.libs/libfwts.so.1(fwts_smbios_find_entry+0x6c)
> 
> The bus error is avoided by replacing memcpy() with a copy loop that
> does not read outside the copied range:
> 
> PASSED: Test 1, SMBIOS Table Entry Point Checksum is valid.
> PASSED: Test 1, SMBIOS Table Entry Point Length is valid.
> PASSED: Test 1, SMBIOS Table Entry Intermediate Anchor String _DMI_ is valid.
> PASSED: Test 1, SMBIOS Table Entry Point Intermediate Checksum is valid.
> SMBIOS table loaded from /sys/firmware/dmi/tables/DMI
> PASSED: Test 1, SMBIOS Table Entry Structure Table Address and Length looks valid.
> 
> Test 2 of 4: Test DMI/SMBIOS tables for errors.
> SMBIOS table loaded from /sys/firmware/dmi/tables/DMI
> PASSED: Test 2, Entry @ 0x7aef3020 'BIOS Information (Type 0)'
> PASSED: Test 2, Entry @ 0x7aef3065 'System Information (Type 1)'
> PASSED: Test 2, Entry @ 0x7aef30b2 'Base Board Information (Type 2)'
> PASSED: Test 2, Entry @ 0x7aef30e0 'Chassis Information (Type 3)'
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2:
> 	move variable declaration to start of function
> ---
>  src/lib/src/fwts_safe_mem.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 飛機上有蛇
> diff --git a/src/lib/src/fwts_safe_mem.c b/src/lib/src/fwts_safe_mem.c
> index 2121d73c..15522b64 100644
> --- a/src/lib/src/fwts_safe_mem.c
> +++ b/src/lib/src/fwts_safe_mem.c
> @@ -48,12 +48,15 @@ static void sig_handler(int dummy)
>   */
>  int OPTIMIZE0 fwts_safe_memcpy(void *dst, const void *src, const size_t n)
>  {
> +	size_t i;
> +
>  	if (sigsetjmp(jmpbuf, 1) != 0)
>  		return FWTS_ERROR;
> 
>  	fwts_sig_handler_set(SIGSEGV, sig_handler, &old_segv_action);
>  	fwts_sig_handler_set(SIGBUS, sig_handler, &old_bus_action);
> -	memcpy(dst, src, n);
> +	for (i = n; i; --i)
> +		*(char *)dst++ = *(char *)src++;
>  	fwts_sig_handler_restore(SIGSEGV, &old_segv_action);
>  	fwts_sig_handler_restore(SIGBUS, &old_bus_action);
> 
> --
> 2.28.0
>
Alex Hung Oct. 16, 2020, 8:40 p.m. UTC | #2
On 2020-10-15 4:21 p.m., Heinrich Schuchardt wrote:
> The glibc 2.31 implementation of memcpy() may read 8 bytes at once, even if
> this exceeds the source buffer. This can lead to a bus error.
> 
> Without this patch 'sudo fwts dmicheck -' results in an error when reading
> the SMBIOS table on my system:
> 
> Test 1 of 4: Find and test SMBIOS Table Entry Points.
> This test tries to find and sanity check the SMBIOS data structures.
> Cannot read mmap'd SMBIOS entry at 0x0x7aef3000
> SMBIOS entry loaded from /sys/firmware/dmi/tables/smbios_entry_point
> FAILED [HIGH] SMBIOSNoEntryPoint: Test 1, Could not find any SMBIOS Table Entry Points.
> 
> Test 2 of 4: Test DMI/SMBIOS tables for errors.
> SKIPPED: Test 2, Cannot find SMBIOS or DMI table entry, skip the test.
> 
> The reason is a bus error caught inside fwts_safe_memcpy():
> 
> Caught SIGNAL 7 (Bus error), aborting.
> Backtrace:
> 0x0000ffff8e77d848 fwts/src/lib/src/.libs/libfwts.so.1(+0x16848)
> 0x0000ffff8e8fc7bc linux-vdso.so.1(__kernel_rt_sigreturn+0)
> 0x0000ffff8e6617ac /lib/aarch64-linux-gnu/libc.so.6(+0x887ac)
> 0x0000ffff8e7922a4 fwts/src/lib/src/.libs/libfwts.so.1(fwts_safe_memcpy+0x44)
> 0x0000ffff8e78f84c fwts/src/lib/src/.libs/libfwts.so.1(fwts_smbios_find_entry+0x6c)
> 
> The bus error is avoided by replacing memcpy() with a copy loop that
> does not read outside the copied range:
> 
> PASSED: Test 1, SMBIOS Table Entry Point Checksum is valid.
> PASSED: Test 1, SMBIOS Table Entry Point Length is valid.
> PASSED: Test 1, SMBIOS Table Entry Intermediate Anchor String _DMI_ is valid.
> PASSED: Test 1, SMBIOS Table Entry Point Intermediate Checksum is valid.
> SMBIOS table loaded from /sys/firmware/dmi/tables/DMI
> PASSED: Test 1, SMBIOS Table Entry Structure Table Address and Length looks valid.
> 
> Test 2 of 4: Test DMI/SMBIOS tables for errors.
> SMBIOS table loaded from /sys/firmware/dmi/tables/DMI
> PASSED: Test 2, Entry @ 0x7aef3020 'BIOS Information (Type 0)'
> PASSED: Test 2, Entry @ 0x7aef3065 'System Information (Type 1)'
> PASSED: Test 2, Entry @ 0x7aef30b2 'Base Board Information (Type 2)'
> PASSED: Test 2, Entry @ 0x7aef30e0 'Chassis Information (Type 3)'
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2:
> 	move variable declaration to start of function
> ---
>  src/lib/src/fwts_safe_mem.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/lib/src/fwts_safe_mem.c b/src/lib/src/fwts_safe_mem.c
> index 2121d73c..15522b64 100644
> --- a/src/lib/src/fwts_safe_mem.c
> +++ b/src/lib/src/fwts_safe_mem.c
> @@ -48,12 +48,15 @@ static void sig_handler(int dummy)
>   */
>  int OPTIMIZE0 fwts_safe_memcpy(void *dst, const void *src, const size_t n)
>  {
> +	size_t i;
> +
>  	if (sigsetjmp(jmpbuf, 1) != 0)
>  		return FWTS_ERROR;
> 
>  	fwts_sig_handler_set(SIGSEGV, sig_handler, &old_segv_action);
>  	fwts_sig_handler_set(SIGBUS, sig_handler, &old_bus_action);
> -	memcpy(dst, src, n);
> +	for (i = n; i; --i)
> +		*(char *)dst++ = *(char *)src++;
>  	fwts_sig_handler_restore(SIGSEGV, &old_segv_action);
>  	fwts_sig_handler_restore(SIGBUS, &old_bus_action);
> 
> --
> 2.28.0
> 


Acked-by: Alex Hung <alex.hung@canonical.com>
diff mbox series

Patch

diff --git a/src/lib/src/fwts_safe_mem.c b/src/lib/src/fwts_safe_mem.c
index 2121d73c..15522b64 100644
--- a/src/lib/src/fwts_safe_mem.c
+++ b/src/lib/src/fwts_safe_mem.c
@@ -48,12 +48,15 @@  static void sig_handler(int dummy)
  */
 int OPTIMIZE0 fwts_safe_memcpy(void *dst, const void *src, const size_t n)
 {
+	size_t i;
+
 	if (sigsetjmp(jmpbuf, 1) != 0)
 		return FWTS_ERROR;

 	fwts_sig_handler_set(SIGSEGV, sig_handler, &old_segv_action);
 	fwts_sig_handler_set(SIGBUS, sig_handler, &old_bus_action);
-	memcpy(dst, src, n);
+	for (i = n; i; --i)
+		*(char *)dst++ = *(char *)src++;
 	fwts_sig_handler_restore(SIGSEGV, &old_segv_action);
 	fwts_sig_handler_restore(SIGBUS, &old_bus_action);