COVESA / dlt-daemon

Diagnostic Log and Trace.
https://covesa.github.io/dlt-daemon/
Mozilla Public License 2.0
381 stars 295 forks source link

dlt-user: fix memory issues #667

Closed alexmohr closed 3 weeks ago

alexmohr commented 4 months ago

This fixed #522 on my machines but your that's not the main purpose of this PR. Therefore before resolving #522 more testing is required.

  1. Fixes the following address sanitizer issue

    
    exiting
    #0 0x7ffff7625fc6 in __interceptor_sigaltstack ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:9986
    #1 0x7ffff768c493 in __asan::PlatformUnpoisonStacks() ../../../../src/libsanitizer/asan/asan_posix.cpp:44
    #2 0x7ffff769202c in __asan_handle_no_return ../../../../src/libsanitizer/asan/asan_rtl.cpp:612
    #4 0x7ffff7073ac2 in start_thread nptl/pthread_create.c:442
    #5 0x7ffff710584f  (/lib/x86_64-linux-gnu/libc.so.6+0x12684f)
    
    at offset 160 in frame
    
    This frame has 2 object(s):
    [32, 88) 'log_new' (line 5017)
    [128, 160) 'handle' (line 5016) <== Memory access at offset 160 overflows this variable
    HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
    Thread T37879 created by T0 here:
    #0 0x7ffff762a685 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216

SUMMARY: AddressSanitizer: stack-buffer-overflow ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:9986 in __interceptor_sigaltstack Shadow bytes around the buggy address: 0x10007e637b10: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 f8 f2 0x10007e637b20: f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 00 06 0x10007e637b30: f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 0x10007e637b40: 04 f3 f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 0x10007e637b50: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 =>0x10007e637b60: 00 00 00 f2 f2 f2 f2 f2 00 00 00 00[f3]f3 f3 f3 0x10007e637b70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007e637b80: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 0x10007e637b90: f1 f1 00 00 f2 f2 f8 f2 f2 f2 f8 f8 f2 f2 00 00 0x10007e637ba0: 00 00 00 00 00 00 00 00 00 00 00 f3 f3 f3 f3 f3 0x10007e637bb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb Shadow gap: cc ==3067906==ABORTING Process finished with exit code 1


2. Fixes some minor sanitzer issues in the unit tests due to the leaked objects. 
3. Fixes a crash in libdlt due to presumably double freeing

0 __pthread_kill_implementation (threadid=, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44

1 0x0000007cd8711fe4 in __pthread_kill_internal (signo=6, threadid=) at pthread_kill.c:78

2 0x0000007cd86cde20 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26

3 0x0000007cd86baf0c in __GI_abort () at abort.c:79

4 0x0000007cd8705ea8 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7cd87e4998 "%s\n") at ../sysdeps/posix/libc_fatal.c:155

5 0x0000007cd871c080 in malloc_printerr (str=str@entry=0x7cd87df978 "malloc_consolidate(): unaligned fastbin chunk detected") at malloc.c:5659

6 0x0000007cd871c9d8 in malloc_consolidate (av=av@entry=0x7cd882aa78 ) at malloc.c:4745

7 0x0000007cd871df64 in _int_free (av=0x7cd882aa78 , p=0x5aafe40790, have_lock=) at malloc.c:4669

8 0x0000007cd87208f8 in __GI___libc_free (mem=) at malloc.c:3386

9 0x0000007cd8bbb858 in dlt_free () at /src/lib/dlt_user.c:1190

10 0x0000007cd86d08fc in __cxa_finalize (d=0x7cd8bf0000) at cxa_finalize.c:83

11 0x0000007cd8bb9de0 in __do_global_dtors_aux () from /usr/lib/libdlt.so.2

12 0x0000007cdeafcee8 in _dl_call_fini (closure_map=closure_map@entry=0x7cdeacf530) at dl-call_fini.c:43

13 0x0000007cdeb002d0 in _dl_fini () at dl-fini.c:78

14 0x0000007cd86d0398 in run_exit_handlers (status=0, listp=0x7cd882a698 <exit_funcs>, run_list_atexit=run_list_atexit@entry=true, run_dtors=run_dtors@entry=true) at exit.c:116

15 0x0000007cd86d046c in __GI_exit (status=) at exit.c:146

16 0x0000007cd86bb1c4 in __libc_start_call_main (main=main@entry=0x5a80cc67ec <main(int, char const const)>, argc=argc@entry=67, argv=argv@entry=0x7febd193c8)

at ../sysdeps/nptl/libc_start_call_main.h:74

17 0x0000007cd86bb29c in __libc_start_main_impl (main=0x5a80cc67ec <main(int, char const const)>, argc=67, argv=0x7febd193c8, init=, fini=,

rtld_fini=<optimized out>, stack_end=<optimized out>) at ../csu/libc-start.c:389

18 0x0000005a80cf5530 in _start () at ../sysdeps/aarch64/start.S:81



This stacktrace above has been detected in an endurance test of dlt. After making the changes the issue did not occur during a 30 hour test again. 
Please note that our internal testing was not based on master but the release we're currently using on the actual systems and this patch has been ported from that release to master. 

The program was tested solely for our own use cases, which might differ from yours.
Licensed under Mozilla Public License Version 2.0

<sup>Alexander Mohr, <alexander.m.mohr@mercedes-benz.com>, Mercedes-Benz Tech Innovation GmbH, [imprint](https://github.com/mercedes-benz/foss/blob/master/PROVIDER_INFORMATION.md)</sup>
lti9hc commented 1 month ago

This PR has reviewed by Michael and Alexander Mohr also updated the latest change, so I am going to merge it. Thanks for your contribution

lti9hc commented 1 month ago

Hello @alexmohr, The conflict is happing on this branch, could you resolve it?

alexmohr commented 1 month ago

This PR has reviewed by Michael and Alexander Mohr also updated the latest change, so I am going to merge it. Thanks for your contribution

There are still some threads remaining where I had questions to @michael-methner . It would be great if we could answer these before merging it as it might require some changes.

alexmohr commented 1 month ago

Rebased and I think I'm done. Please give it another review.

michael-methner commented 1 month ago

Rebased and I think I'm done. Please give it another review.

Hello @alexmohr , i think I had some comments while you were rebasing. Can you shortly check the open discussions. I also tried to answer your questions. Sorry again for taking so much time.

michael-methner commented 3 weeks ago

Hello @alexmohr , thanks for the rework. I think this PR is ready to merge. @lti9hc , @minminlittleshrimp : Could some one of you merge please?

lti9hc commented 3 weeks ago

@alexmohr thanks for your contribution @michael-methner thanks for your review I am going to merge