Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asan fix #1799

Merged
merged 4 commits into from
Sep 14, 2017
Merged

Asan fix #1799

merged 4 commits into from
Sep 14, 2017

Conversation

jenswi-linaro
Copy link
Contributor

No description provided.

@jforissier
Copy link
Contributor

For "core: asan: tag access for .ARM.extab and .ARM.exidx":
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

@etienne-lms
Copy link
Contributor

for "core: asan: fix check_access() ":
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>

@jenswi-linaro
Copy link
Contributor Author

Tags applied

@prime-zeng
Copy link
Contributor

prime-zeng commented Sep 9, 2017

I have test this patch with xtest, and it resolves the problem i mentioned in #1790 , but i got another problem, this is my error message:

* gp_8060 a7-ef-8a
ERROR:   [0x0] TEE-CORE: Panic at core/kernel/asan.c:176 <check_access>
ERROR:   [0x0] TEE-CORE: Call stack:
ERROR:   [0x0] TEE-CORE:  0x7e0081e9x
ERROR:   [0x0] TEE-CORE:  0x7e010133x
ERROR:   [0x0] TEE-CORE:  0x7e010433x
ERROR:   [0x0] TEE-CORE:  0x7e13622dx
ERROR:   [0x0] TEE-CORE:  0x7e11a85fx
ERROR:   [0x0] TEE-CORE:  0x7e13611bx
ERROR:   [0x0] TEE-CORE:  0x7e00486dx
ERROR:   [0x0] TEE-CORE:  0x7e00344fx
ERROR:   [0x0] TEE-CORE:  0x7e00fc03x
ERROR:   [0x0] TEE-CORE:  0x7e00d47dx
ERROR:   [0x0] TEE-CORE:  0x7e00694bx
ERROR:   [0x0] TEE-CORE:  0x7e0011acx

It have applied the latest scripts/symbolize.py patch @jforissier, seems the stack have got a redundent 'x'.
The following is the stack translated by scripts/symbolize.py :

ERROR:   [0x0] TEE-CORE: Call stack:
ERROR:   [0x0] TEE-CORE:  0x7e0081e9 print_kernel_stack at /home/zengtao/Sources/work/SPC050_OPTEE/Code/source/tee/core/optee/optee_os/core/arch/arm/kernel/unwind_arm32.c:377x
ERROR:   [0x0] TEE-CORE:  0x7e010133 __do_panic at /home/zengtao/Sources/work/SPC050_OPTEE/Code/source/tee/core/optee/optee_os/core/kernel/panic.c:52x
ERROR:   [0x0] TEE-CORE:  0x7e010433 va_to_shadow at /home/zengtao/Sources/work/SPC050_OPTEE/Code/source/tee/core/optee/optee_os/core/kernel/asan.c:59x
ERROR:   [0x0] TEE-CORE:  0x7e13622d memset at /home/zengtao/Sources/work/SPC050_OPTEE/Code/source/tee/core/optee/optee_os/lib/libutils/isoc/newlib/memset.c:125x
ERROR:   [0x0] TEE-CORE:  0x7e11a85f bgetz at /home/zengtao/Sources/work/SPC050_OPTEE/Code/source/tee/core/optee/optee_os/lib/libutils/isoc/bget.c:785x
ERROR:   [0x0] TEE-CORE:  0x7e13611b raw_calloc at /home/zengtao/Sources/work/SPC050_OPTEE/Code/source/tee/core/optee/optee_os/lib/libutils/isoc/bget_malloc.c:345x
ERROR:   [0x0] TEE-CORE:  0x7e00486d elf_load_init at /home/zengtao/Sources/work/SPC050_OPTEE/Code/source/tee/core/optee/optee_os/core/arch/arm/kernel/elf_load.c:203x
ERROR:   [0x0] TEE-CORE:  0x7e00344f load_elf at /home/zengtao/Sources/work/SPC050_OPTEE/Code/source/tee/core/optee/optee_os/core/arch/arm/kernel/user_ta.c:201x
ERROR:   [0x0] TEE-CORE:  0x7e00fc03 tee_ta_init_session at /home/zengtao/Sources/work/SPC050_OPTEE/Code/source/tee/core/optee/optee_os/core/kernel/tee_ta_manager.c:513x
ERROR:   [0x0] TEE-CORE:  0x7e00d47d entry_open_session at /home/zengtao/Sources/work/SPC050_OPTEE/Code/source/tee/core/optee/optee_os/core/arch/arm/tee/entry_std.c:242x
ERROR:   [0x0] TEE-CORE:  0x7e00694b __thread_std_smc_entry at /home/zengtao/Sources/work/SPC050_OPTEE/Code/source/tee/core/optee/optee_os/core/arch/arm/kernel/thread.c:596x
ERROR:   [0x0] TEE-CORE:  0x7e0011ac thread_std_smc_entry at /home/zengtao/Sources/work/SPC050_OPTEE/Code/source/tee/core/optee/optee_os/core/arch/arm/kernel/thread_a32.S:355x

@jenswi-linaro Only runs the gp_8060 can't reproduce the problem, i must run xtest all to produce this error, so i guess maybe some branch code of the calloc can't deal with asan very well.

@jforissier
Copy link
Contributor

@prime-zeng I have mistakenly inserted an extra 'x' in the stack dump. Fixed in #1802. Thanks for reporting!

@jforissier
Copy link
Contributor

I got a slightly different call stack when testing this PR on QEMU (make -j9 CFG_CORE_SANITIZE_KADDRESS=y run). It crashed in regression_6016, so before even running the GP tests. I suppose it could happen anywhere since it looks related to the allocation code. I'm running xtest again to see if I get a different stack trace. Anyway, here's what I got:

$ xtest
...
* regression_6016 Storage concurency
o regression_6016.1 Storage id: 00000001
    threads: 4, loops: 8

Hangs there, secure console shows a panic which decodes to:

$ ./optee_os/scripts/symbolize.py -d optee_os/out/arm/core -s `pwd`
[copy/paste crash dump]
ERROR:   [0x0] TEE-CORE: Panic at core/kernel/asan.c:176 <check_access>
ERROR:   [0x0] TEE-CORE: Call stack:
ERROR:   [0x0] TEE-CORE:  0x0e02d7b5 print_kernel_stack at optee_os/core/arch/arm/kernel/unwind_arm32.c:376x
ERROR:   [0x0] TEE-CORE:  0x0e042d79 __do_panic at optee_os/core/kernel/panic.c:54 (discriminator 1)x
ERROR:   [0x0] TEE-CORE:  0x0e043689 check_access at optee_os/core/kernel/asan.c:174 (discriminator 2)x
ERROR:   [0x0] TEE-CORE:  0x0e04371d check_store at optee_os/core/kernel/asan.c:191x
ERROR:   [0x0] TEE-CORE:  0x0e0437ef __asan_store4_noabort at optee_os/core/kernel/asan.c:223x
ERROR:   [0x0] TEE-CORE:  0x0e069137 memset at optee_os/lib/libutils/isoc/newlib/memset.c:116x
ERROR:   [0x0] TEE-CORE:  0x0e066f87 bgetz at optee_os/lib/libutils/isoc/bget.c:784x
ERROR:   [0x0] TEE-CORE:  0x0e067839 raw_calloc at optee_os/lib/libutils/isoc/bget_malloc.c:345x
ERROR:   [0x0] TEE-CORE:  0x0e067fa5 calloc at optee_os/lib/libutils/isoc/bget_malloc.c:832x
ERROR:   [0x0] TEE-CORE:  0x0e036adf mobj_mm_alloc at optee_os/core/arch/arm/mm/mobj.c:293x
ERROR:   [0x0] TEE-CORE:  0x0e0221e1 alloc_ta_mem at optee_os/core/arch/arm/kernel/user_ta.c:186x
ERROR:   [0x0] TEE-CORE:  0x0e02239f load_elf at optee_os/core/arch/arm/kernel/user_ta.c:225x
ERROR:   [0x0] TEE-CORE:  0x0e000cc9 ta_load at optee_os/core/arch/arm/kernel/user_ta.c:296x
ERROR:   [0x0] TEE-CORE:  0x0e022f35 tee_ta_init_user_ta_session at optee_os/core/arch/arm/kernel/user_ta.c:633x
ERROR:   [0x0] TEE-CORE:  0x0e041ed3 tee_ta_init_session at optee_os/core/kernel/tee_ta_manager.c:513x
ERROR:   [0x0] TEE-CORE:  0x0e04200b tee_ta_open_session at optee_os/core/kernel/tee_ta_manager.c:540x
ERROR:   [0x0] TEE-CORE:  0x0e038b47 entry_open_session at optee_os/core/arch/arm/tee/entry_std.c:239x
ERROR:   [0x0] TEE-CORE:  0x0e003ddf tee_entry_std at optee_os/core/arch/arm/tee/entry_std.c:396x
ERROR:   [0x0] TEE-CORE:  0x0e0270f7 __thread_std_smc_entry at optee_os/core/arch/arm/kernel/thread.c:596x
ERROR:   [0x0] TEE-CORE:  0x0e0004c0 thread_std_smc_entry at optee_os/core/arch/arm/kernel/thread_a32.S:369x

@jforissier
Copy link
Contributor

A slightly different stack still in xtest 6016:

ERROR:   [0x0] TEE-CORE: Panic at core/kernel/asan.c:176 <check_access>
ERROR:   [0x0] TEE-CORE: Call stack:
ERROR:   [0x0] TEE-CORE:  0x0e02d7b5 print_kernel_stack at optee_os/core/arch/arm/kernel/unwind_arm32.c:376x
ERROR:   [0x0] TEE-CORE:  0x0e042d79 __do_panic at optee_os/core/kernel/panic.c:54 (discriminator 1)x
ERROR:   [0x0] TEE-CORE:  0x0e043689 check_access at optee_os/core/kernel/asan.c:174 (discriminator 2)x
ERROR:   [0x0] TEE-CORE:  0x0e04371d check_store at optee_os/core/kernel/asan.c:191x
ERROR:   [0x0] TEE-CORE:  0x0e0437ef __asan_store4_noabort at optee_os/core/kernel/asan.c:223x
ERROR:   [0x0] TEE-CORE:  0x0e0690ff memset at optee_os/lib/libutils/isoc/newlib/memset.c:110x
ERROR:   [0x0] TEE-CORE:  0x0e066f87 bgetz at optee_os/lib/libutils/isoc/bget.c:784x
ERROR:   [0x0] TEE-CORE:  0x0e067839 raw_calloc at optee_os/lib/libutils/isoc/bget_malloc.c:345x
ERROR:   [0x0] TEE-CORE:  0x0e067fa5 calloc at optee_os/lib/libutils/isoc/bget_malloc.c:832x
ERROR:   [0x0] TEE-CORE:  0x0e0373b5 mobj_shm_alloc at optee_os/core/arch/arm/mm/mobj.c:688x
ERROR:   [0x0] TEE-CORE:  0x0e029131 thread_rpc_alloc at optee_os/core/arch/arm/kernel/thread.c:1409x
ERROR:   [0x0] TEE-CORE:  0x0e02925d thread_rpc_alloc_payload at optee_os/core/arch/arm/kernel/thread.c:1428x
ERROR:   [0x0] TEE-CORE:  0x0e053123 tee_fs_rpc_cache_alloc at optee_os/core/tee/tee_fs_rpc_cache.c:66x
ERROR:   [0x0] TEE-CORE:  0x0e052895 tee_fs_rpc_read_init at optee_os/core/tee/tee_fs_rpc.c:169x
ERROR:   [0x0] TEE-CORE:  0x0e04d93d ree_fs_rpc_read_init at optee_os/core/tee/tee_ree_fs.c:221x
ERROR:   [0x0] TEE-CORE:  0x0e050d5f tee_fs_htree_read_block at optee_os/core/tee/fs_htree.c:913x
ERROR:   [0x0] TEE-CORE:  0x0e04d621 out_of_place_write at optee_os/core/tee/tee_ree_fs.c:100x
ERROR:   [0x0] TEE-CORE:  0x0e04deeb ree_fs_write_primitive at optee_os/core/tee/tee_ree_fs.c:381x
ERROR:   [0x0] TEE-CORE:  0x0e04eaff ree_fs_write at optee_os/core/tee/tee_ree_fs.c:738x
ERROR:   [0x0] TEE-CORE:  0x0e04ced1 syscall_storage_obj_write at optee_os/core/tee/tee_svc_storage.c:906x
ERROR:   [0x0] TEE-CORE:  0x0e00350c tee_svc_do_call at optee_os/core/arch/arm/tee/arch_svc_a32.S:82x

@jenswi-linaro
Copy link
Contributor Author

Added two commits that should fix the problem reported. These two commits should be reordered first before merging this PR.

@etienne-lms
Copy link
Contributor

For the 3 first Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>.

For the last: Ack-by: Etienne Carriere <etienne.carriere@linaro.org>.
I don't understand why the free wipe conflicts with asan.

@jenswi-linaro
Copy link
Contributor Author

@etienne-lms , bget() tags the requested amount of memory allocated, but eventual padding etc isn't tagged so writes there from instrumented functions (for instance the normal memset()) will be caught.

@etienne-lms
Copy link
Contributor

Crystal clear, thanks.
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>

@jenswi-linaro
Copy link
Contributor Author

Tags applied, commits reordered.

@etienne-lms
Copy link
Contributor

By the way, would you mind adding your explanation

bget() tags the requested amount of memory allocated, but eventual padding etc isn't tagged so writes there from instrumented functions, for instance the normal memset(), will be caught.

into the commit comment?

@jenswi-linaro
Copy link
Contributor Author

commit message amended.

@jforissier
Copy link
Contributor

It looks like asan.h lacks #include <string.h>.

Provides asan_memset_unchecked() which does a memset that isn't checked
against the tagging in the ASAN shadow area. If ASAN isn't enabled it's
replaced by a direct call to memset().

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
The malloc implementation uses the new asan_memset_unchecked() function
internally instead of memset() to avoid unexpected asserts when the
address sanitizer is enabled.

bget() tags the requested amount of memory allocated, but eventual
padding etc isn't tagged so writes there from instrumented functions,
for instance the normal memset(), will be caught.

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Prior to this patch the for loop in check_access() that checks the
access in the shadow area is skipping accesses smaller than a ASAN block
(8 bytes). This patch fixes that problem and checks also smaller
accesses.

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
The two sections .ARM.extab and .ARM.exidx are accessed when printing a
stack trace. Tag access for these two sections to avoid recursive panics
due to failing checks against shadow area.

Tested-by: Jens Wiklander <jens.wiklander@linaro.org> (QEMU)
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
@jenswi-linaro
Copy link
Contributor Author

#include <string.h> added
Patches rebased

@jforissier
Copy link
Contributor

The remaining build failures are caused by 6a815af ("core: introduce TEE_RAM_VA_START and TEE_TEXT_VA_START"). So I'm merging this PR now.

@jforissier jforissier merged commit 69b9f69 into OP-TEE:master Sep 14, 2017
@jenswi-linaro jenswi-linaro deleted the asan_fix branch September 14, 2017 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants