| Message ID | 20201015182527.3448-1-xypron.glpk@gmx.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] lib: avoid bus error in fwts_safe_memcpy | expand |
On 15/10/2020 19:25, 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> > --- > src/lib/src/fwts_safe_mem.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/lib/src/fwts_safe_mem.c b/src/lib/src/fwts_safe_mem.c > index 2121d73c..abe6500c 100644 > --- a/src/lib/src/fwts_safe_mem.c > +++ b/src/lib/src/fwts_safe_mem.c > @@ -53,7 +53,8 @@ int OPTIMIZE0 fwts_safe_memcpy(void *dst, const void *src, const size_t n) > > 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 (size_t i = n; i; --i) > + *(char *)dst++ = *(char *)src++; Yep, good idea. Can you put the size_t i; declaration to be placed at the start of the function just to match the fwts coding style. Thanks. > fwts_sig_handler_restore(SIGSEGV, &old_segv_action); > fwts_sig_handler_restore(SIGBUS, &old_bus_action); > > -- > 2.28.0 >
diff --git a/src/lib/src/fwts_safe_mem.c b/src/lib/src/fwts_safe_mem.c index 2121d73c..abe6500c 100644 --- a/src/lib/src/fwts_safe_mem.c +++ b/src/lib/src/fwts_safe_mem.c @@ -53,7 +53,8 @@ int OPTIMIZE0 fwts_safe_memcpy(void *dst, const void *src, const size_t n) 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 (size_t i = n; i; --i) + *(char *)dst++ = *(char *)src++; fwts_sig_handler_restore(SIGSEGV, &old_segv_action); fwts_sig_handler_restore(SIGBUS, &old_bus_action);
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> --- src/lib/src/fwts_safe_mem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.28.0