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.

Sign up for GitHub

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

Jump to bottom

Fix memory leaks #52

Merged
merged 2 commits into from
Nov 22, 2022
Merged

Fix memory leaks #52

merged 2 commits into from
Nov 22, 2022

Conversation

t-8ch
Copy link
Contributor

@t-8ch t-8ch commented Jul 2, 2022

Fixes #51

t-8ch added 2 commits July 2, 2022 23:47
@t-8ch
fix: prevent memory leak in object_add_field()
b1c045b 
On errors the Field has to be freed with field_freep to also free the
allocated name member.

Fixes varlink#51
@t-8ch
tests: prevent memory leak in test-json
d0833e6
@haraldh haraldh merged commit 1ac0dab into varlink:master Nov 22, 2022
@evverx
Copy link
Contributor

evverx commented Nov 28, 2022 •
edited
Loading

@t-8ch thanks! I totally forgot about that memory leak. I also found two more issues at the time but decided not to report them because I wasn't sure whether the project was alive. Looks like it is so I'll leave them here just in case:

heap-use-after-free in test-server-client 
CC=clang meson -Db_sanitize=address -Db_lundef=false build
meson test -C ./build/ --print-errorlog test-server-client
ninja: Entering directory `/libvarlink/build'
ninja: no work to do.
1/1 test-server-client        FAIL            0.30s   exit status 1
>>> MALLOC_PERTURB_=82 /libvarlink/build/lib/test-server-client
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― ✀  ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
stderr:
=================================================================
==2638==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000000b10 at pc 0x00000052a1ed bp 0x7ffff15b0230 sp 0x7ffff15b0228
READ of size 8 at 0x606000000b10 thread T0
    #0 0x52a1ec in varlink_call_reply /libvarlink/build/../lib/service.c:660:23
    #1 0x518172 in org_varlink_example_Echo /libvarlink/build/../lib/test-server-client.c:33:9
    #2 0x526bb2 in varlink_service_method_callback /libvarlink/build/../lib/service.c:282:16
    #3 0x5298f2 in varlink_service_dispatch_connection /libvarlink/build/../lib/service.c:555:29
    #4 0x528bee in varlink_service_process_events /libvarlink/build/../lib/service.c:604:29
    #5 0x518a8e in test_process_events /libvarlink/build/../lib/test-server-client.c:66:25
    #6 0x5173e5 in main /libvarlink/build/../lib/test-server-client.c:155:25
    #7 0x7fb5e6f4750f in __libc_start_call_main (/lib64/libc.so.6+0x2750f) (BuildId: 765237b0355c030ff41d969eedcb87bfccb43595)
    #8 0x7fb5e6f475c8 in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x275c8) (BuildId: 765237b0355c030ff41d969eedcb87bfccb43595)
    #9 0x41e414 in _start (/libvarlink/build/lib/test-server-client+0x41e414) (BuildId: 895f2f71b3295e0d2de926f12b3f985f4b8f704d)

0x606000000b10 is located 16 bytes inside of 64-byte region [0x606000000b00,0x606000000b40)
freed by thread T0 here:
    #0 0x4d2138 in __interceptor_free.part.0 asan_malloc_linux.cpp.o
    #1 0x52523c in varlink_call_unref /libvarlink/build/../lib/service.c:101:17
    #2 0x52a1bf in varlink_call_reply /libvarlink/build/../lib/service.c:660:42
    #3 0x518172 in org_varlink_example_Echo /libvarlink/build/../lib/test-server-client.c:33:9
    #4 0x526bb2 in varlink_service_method_callback /libvarlink/build/../lib/service.c:282:16
    #5 0x5298f2 in varlink_service_dispatch_connection /libvarlink/build/../lib/service.c:555:29
    #6 0x528bee in varlink_service_process_events /libvarlink/build/../lib/service.c:604:29
    #7 0x518a8e in test_process_events /libvarlink/build/../lib/test-server-client.c:66:25
    #8 0x5173e5 in main /libvarlink/build/../lib/test-server-client.c:155:25
    #9 0x7fb5e6f4750f in __libc_start_call_main (/lib64/libc.so.6+0x2750f) (BuildId: 765237b0355c030ff41d969eedcb87bfccb43595)

previously allocated by thread T0 here:
    #0 0x4d3457 in __interceptor_calloc (/libvarlink/build/lib/test-server-client+0x4d3457) (BuildId: 895f2f71b3295e0d2de926f12b3f985f4b8f704d)
    #1 0x52b8df in varlink_call_new /libvarlink/build/../lib/service.c:69:16
    #2 0x52970e in varlink_service_dispatch_connection /libvarlink/build/../lib/service.c:551:29
    #3 0x528bee in varlink_service_process_events /libvarlink/build/../lib/service.c:604:29
    #4 0x518a8e in test_process_events /libvarlink/build/../lib/test-server-client.c:66:25
    #5 0x5173e5 in main /libvarlink/build/../lib/test-server-client.c:155:25
    #6 0x7fb5e6f4750f in __libc_start_call_main (/lib64/libc.so.6+0x2750f) (BuildId: 765237b0355c030ff41d969eedcb87bfccb43595)

SUMMARY: AddressSanitizer: heap-use-after-free /libvarlink/build/../lib/service.c:660:23 in varlink_call_reply
Shadow bytes around the buggy address:
  0x0c0c7fff8110: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c0c7fff8120: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
  0x0c0c7fff8130: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
  0x0c0c7fff8140: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c0c7fff8150: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
=>0x0c0c7fff8160: fd fd[fd]fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x0c0c7fff8170: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff8180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff8190: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff81a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff81b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
==2638==ABORTING
Conditional jump or move depends on uninitialised value(s) in varlink_array_unrefp 
meson build
ninja -C ./build
printf '{"":[\0' | valgrind --track-origins=yes ./build/tool/varlink bridge
==2931== Memcheck, a memory error detector
==2931== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==2931== Using Valgrind-3.20.0 and LibVEX; rerun with -h for copyright info
==2931== Command: ./build/tool/varlink bridge
==2931==
==2931== Conditional jump or move depends on uninitialised value(s)
==2931==    at 0x410C7A: varlink_value_clear (value.c:13)
==2931==    by 0x4073CA: varlink_array_unref (array.c:110)
==2931==    by 0x407427: varlink_array_unrefp (array.c:121)
==2931==    by 0x407352: varlink_array_new_from_scanner (array.c:54)
==2931==    by 0x410DAE: varlink_value_read_from_scanner (value.c:56)
==2931==    by 0x40B0FD: varlink_object_new_from_scanner (object.c:122)
==2931==    by 0x40B247: varlink_object_new_from_json (object.c:156)
==2931==    by 0x40DDB9: varlink_stream_read (stream.c:229)
==2931==    by 0x40448C: handleBridge (command-bridge.c:147)
==2931==    by 0x404C63: bridge_run (command-bridge.c:339)
==2931==    by 0x40353B: cli_run (cli.c:496)
==2931==    by 0x40700D: main (main.c:14)
==2931==  Uninitialised value was created by a heap allocation
==2931==    at 0x484378A: malloc (vg_replace_malloc.c:392)
==2931==    by 0x484870B: realloc (vg_replace_malloc.c:1451)
==2931==    by 0x4070DF: array_append (array.c:22)
==2931==    by 0x407276: varlink_array_new_from_scanner (array.c:73)
==2931==    by 0x410DAE: varlink_value_read_from_scanner (value.c:56)
==2931==    by 0x40B0FD: varlink_object_new_from_scanner (object.c:122)
==2931==    by 0x40B247: varlink_object_new_from_json (object.c:156)
==2931==    by 0x40DDB9: varlink_stream_read (stream.c:229)
==2931==    by 0x40448C: handleBridge (command-bridge.c:147)
==2931==    by 0x404C63: bridge_run (command-bridge.c:339)
==2931==    by 0x40353B: cli_run (cli.c:496)
==2931==    by 0x40700D: main (main.c:14)
==2931==
==2931== Conditional jump or move depends on uninitialised value(s)
==2931==    at 0x410C7F: varlink_value_clear (value.c:13)
==2931==    by 0x4073CA: varlink_array_unref (array.c:110)
==2931==    by 0x407427: varlink_array_unrefp (array.c:121)
==2931==    by 0x407352: varlink_array_new_from_scanner (array.c:54)
==2931==    by 0x410DAE: varlink_value_read_from_scanner (value.c:56)
==2931==    by 0x40B0FD: varlink_object_new_from_scanner (object.c:122)
==2931==    by 0x40B247: varlink_object_new_from_json (object.c:156)
==2931==    by 0x40DDB9: varlink_stream_read (stream.c:229)
==2931==    by 0x40448C: handleBridge (command-bridge.c:147)
==2931==    by 0x404C63: bridge_run (command-bridge.c:339)
==2931==    by 0x40353B: cli_run (cli.c:496)
==2931==    by 0x40700D: main (main.c:14)
==2931==  Uninitialised value was created by a heap allocation
==2931==    at 0x484378A: malloc (vg_replace_malloc.c:392)
==2931==    by 0x484870B: realloc (vg_replace_malloc.c:1451)
==2931==    by 0x4070DF: array_append (array.c:22)
==2931==    by 0x407276: varlink_array_new_from_scanner (array.c:73)
==2931==    by 0x410DAE: varlink_value_read_from_scanner (value.c:56)
==2931==    by 0x40B0FD: varlink_object_new_from_scanner (object.c:122)
==2931==    by 0x40B247: varlink_object_new_from_json (object.c:156)
==2931==    by 0x40DDB9: varlink_stream_read (stream.c:229)
==2931==    by 0x40448C: handleBridge (command-bridge.c:147)
==2931==    by 0x404C63: bridge_run (command-bridge.c:339)
==2931==    by 0x40353B: cli_run (cli.c:496)
==2931==    by 0x40700D: main (main.c:14)
==2931==
==2931== Conditional jump or move depends on uninitialised value(s)
==2931==    at 0x410C84: varlink_value_clear (value.c:13)
==2931==    by 0x4073CA: varlink_array_unref (array.c:110)
==2931==    by 0x407427: varlink_array_unrefp (array.c:121)
==2931==    by 0x407352: varlink_array_new_from_scanner (array.c:54)
==2931==    by 0x410DAE: varlink_value_read_from_scanner (value.c:56)
==2931==    by 0x40B0FD: varlink_object_new_from_scanner (object.c:122)
==2931==    by 0x40B247: varlink_object_new_from_json (object.c:156)
==2931==    by 0x40DDB9: varlink_stream_read (stream.c:229)
==2931==    by 0x40448C: handleBridge (command-bridge.c:147)
==2931==    by 0x404C63: bridge_run (command-bridge.c:339)
==2931==    by 0x40353B: cli_run (cli.c:496)
==2931==    by 0x40700D: main (main.c:14)
==2931==  Uninitialised value was created by a heap allocation
==2931==    at 0x484378A: malloc (vg_replace_malloc.c:392)
==2931==    by 0x484870B: realloc (vg_replace_malloc.c:1451)
==2931==    by 0x4070DF: array_append (array.c:22)
==2931==    by 0x407276: varlink_array_new_from_scanner (array.c:73)
==2931==    by 0x410DAE: varlink_value_read_from_scanner (value.c:56)
==2931==    by 0x40B0FD: varlink_object_new_from_scanner (object.c:122)
==2931==    by 0x40B247: varlink_object_new_from_json (object.c:156)
==2931==    by 0x40DDB9: varlink_stream_read (stream.c:229)
==2931==    by 0x40448C: handleBridge (command-bridge.c:147)
==2931==    by 0x404C63: bridge_run (command-bridge.c:339)
==2931==    by 0x40353B: cli_run (cli.c:496)
==2931==    by 0x40700D: main (main.c:14)
==2931==
==2931== Conditional jump or move depends on uninitialised value(s)
==2931==    at 0x410C89: varlink_value_clear (value.c:13)
==2931==    by 0x4073CA: varlink_array_unref (array.c:110)
==2931==    by 0x407427: varlink_array_unrefp (array.c:121)
==2931==    by 0x407352: varlink_array_new_from_scanner (array.c:54)
==2931==    by 0x410DAE: varlink_value_read_from_scanner (value.c:56)
==2931==    by 0x40B0FD: varlink_object_new_from_scanner (object.c:122)
==2931==    by 0x40B247: varlink_object_new_from_json (object.c:156)
==2931==    by 0x40DDB9: varlink_stream_read (stream.c:229)
==2931==    by 0x40448C: handleBridge (command-bridge.c:147)
==2931==    by 0x404C63: bridge_run (command-bridge.c:339)
==2931==    by 0x40353B: cli_run (cli.c:496)
==2931==    by 0x40700D: main (main.c:14)
==2931==  Uninitialised value was created by a heap allocation
==2931==    at 0x484378A: malloc (vg_replace_malloc.c:392)
==2931==    by 0x484870B: realloc (vg_replace_malloc.c:1451)
==2931==    by 0x4070DF: array_append (array.c:22)
==2931==    by 0x407276: varlink_array_new_from_scanner (array.c:73)
==2931==    by 0x410DAE: varlink_value_read_from_scanner (value.c:56)
==2931==    by 0x40B0FD: varlink_object_new_from_scanner (object.c:122)
==2931==    by 0x40B247: varlink_object_new_from_json (object.c:156)
==2931==    by 0x40DDB9: varlink_stream_read (stream.c:229)
==2931==    by 0x40448C: handleBridge (command-bridge.c:147)
==2931==    by 0x404C63: bridge_run (command-bridge.c:339)
==2931==    by 0x40353B: cli_run (cli.c:496)
==2931==    by 0x40700D: main (main.c:14)
==2931==
==2931== Conditional jump or move depends on uninitialised value(s)
==2931==    at 0x410C8E: varlink_value_clear (value.c:13)
==2931==    by 0x4073CA: varlink_array_unref (array.c:110)
==2931==    by 0x407427: varlink_array_unrefp (array.c:121)
==2931==    by 0x407352: varlink_array_new_from_scanner (array.c:54)
==2931==    by 0x410DAE: varlink_value_read_from_scanner (value.c:56)
==2931==    by 0x40B0FD: varlink_object_new_from_scanner (object.c:122)
==2931==    by 0x40B247: varlink_object_new_from_json (object.c:156)
==2931==    by 0x40DDB9: varlink_stream_read (stream.c:229)
==2931==    by 0x40448C: handleBridge (command-bridge.c:147)
==2931==    by 0x404C63: bridge_run (command-bridge.c:339)
==2931==    by 0x40353B: cli_run (cli.c:496)
==2931==    by 0x40700D: main (main.c:14)
==2931==  Uninitialised value was created by a heap allocation
==2931==    at 0x484378A: malloc (vg_replace_malloc.c:392)
==2931==    by 0x484870B: realloc (vg_replace_malloc.c:1451)
==2931==    by 0x4070DF: array_append (array.c:22)
==2931==    by 0x407276: varlink_array_new_from_scanner (array.c:73)
==2931==    by 0x410DAE: varlink_value_read_from_scanner (value.c:56)
==2931==    by 0x40B0FD: varlink_object_new_from_scanner (object.c:122)
==2931==    by 0x40B247: varlink_object_new_from_json (object.c:156)
==2931==    by 0x40DDB9: varlink_stream_read (stream.c:229)
==2931==    by 0x40448C: handleBridge (command-bridge.c:147)
==2931==    by 0x404C63: bridge_run (command-bridge.c:339)
==2931==    by 0x40353B: cli_run (cli.c:496)
==2931==    by 0x40700D: main (main.c:14)
==2931==
==2931==

I moved those bugs to #55 and #54

@t-8ch t-8ch deleted the fix-memleaks branch November 28, 2022 03:29
@t-8ch t-8ch mentioned this pull request Nov 28, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haraldh haraldh haraldh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

There seems to be a memory leak
3 participants
@t-8ch @evverx @haraldh