fuzz: handle advanced signatures as well #64

mrc0mmand · 2022-05-01T20:34:47Z

very much WIP

mrc0mmand · 2022-05-01T21:16:18Z

Oh well, this sort of works, but it's wrapped in another tuple, which makes it a bit useless. After reading through the glib sources I got another idea, but that needs to wait until tomorrow.

mrc0mmand · 2022-05-02T08:59:55Z

Ah, nevermind, it works as it supposed to, but currently dfuzzer disassembles the signature to arguments, which the new function doesn't like. Let me fix that.

mrc0mmand · 2022-05-02T13:49:40Z

@evverx so, this should now, hopefully, do what it's supposed to. As this ended up touching a lot of more code than I originally anticipated (along with dropping the libffi dependency completely and a lot of other code) it would be stellar if you could unleash some of your shenanigans on this, since you have the gift for finding interesting issues ;-)

There's still some work to do, but the core stuff should work™.

evverx · 2022-05-02T15:50:13Z

@mrc0mmand I'll take a look tomorrow I think.

it would be stellar if you could unleash some of your shenanigans on this, since you have the gift for finding interesting issues

Do you mean issues like

==1==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 5521 byte(s) in 10 object(s) allocated from:
    #0 0x7faf155638f7 in strdup (/lib64/libasan.so.6+0x598f7)
    #1 0x7faf1313d06f in strv_extend ../src/basic/strv.c:538
    #2 0x7faf12e4947b in normalize_linked_files ../src/shared/install.c:2839
    #3 0x7faf12e49bc5 in unit_file_reenable ../src/shared/install.c:2872
    #4 0x7faf148e7292 in method_enable_unit_files_generic ../src/core/dbus-manager.c:2332
    #5 0x7faf148e74e6 in method_reenable_unit_files ../src/core/dbus-manager.c:2348
    #6 0x7faf1323c235 in method_callbacks_run ../src/libsystemd/sd-bus/bus-objects.c:406
    #7 0x7faf13245a38 in object_find_and_run ../src/libsystemd/sd-bus/bus-objects.c:1310
    #8 0x7faf132470fa in bus_process_object ../src/libsystemd/sd-bus/bus-objects.c:1430
    #9 0x7faf132a6585 in process_message ../src/libsystemd/sd-bus/sd-bus.c:2962
    #10 0x7faf132a6a9f in process_running ../src/libsystemd/sd-bus/sd-bus.c:3004
    #11 0x7faf132a9900 in bus_process_internal ../src/libsystemd/sd-bus/sd-bus.c:3224
    #12 0x7faf132a9a9a in sd_bus_process ../src/libsystemd/sd-bus/sd-bus.c:3251
    #13 0x7faf132ad19b in io_callback ../src/libsystemd/sd-bus/sd-bus.c:3602
    #14 0x7faf13465b8c in source_dispatch ../src/libsystemd/sd-event/sd-event.c:3591
    #15 0x7faf1346e3a6 in sd_event_dispatch ../src/libsystemd/sd-event/sd-event.c:4175
    #16 0x7faf1346f50a in sd_event_run ../src/libsystemd/sd-event/sd-event.c:4236
    #17 0x7faf14a85dcd in manager_loop ../src/core/manager.c:3068
    #18 0x41c1e3 in invoke_main_loop ../src/core/main.c:1894
    #19 0x42484a in main ../src/core/main.c:2957
    #20 0x7faf1175555f in __libc_start_call_main (/lib64/libc.so.6+0x2d55f)

Direct leak of 963 byte(s) in 10 object(s) allocated from:
    #0 0x7faf155b891f in __interceptor_malloc (/lib64/libasan.so.6+0xae91f)
    #1 0x7faf1555ee04 in __interceptor_strndup.part.0 (/lib64/libasan.so.6+0x54e04)
    #2 0x7faf130e7119 in path_extract_filename ../src/basic/path-util.c:1053
    #3 0x7faf12e48d16 in normalize_linked_files ../src/shared/install.c:2810
    #4 0x7faf12e49bc5 in unit_file_reenable ../src/shared/install.c:2872
    #5 0x7faf148e7292 in method_enable_unit_files_generic ../src/core/dbus-manager.c:2332
    #6 0x7faf148e74e6 in method_reenable_unit_files ../src/core/dbus-manager.c:2348
    #7 0x7faf1323c235 in method_callbacks_run ../src/libsystemd/sd-bus/bus-objects.c:406
    #8 0x7faf13245a38 in object_find_and_run ../src/libsystemd/sd-bus/bus-objects.c:1310
    #9 0x7faf132470fa in bus_process_object ../src/libsystemd/sd-bus/bus-objects.c:1430
    #10 0x7faf132a6585 in process_message ../src/libsystemd/sd-bus/sd-bus.c:2962
    #11 0x7faf132a6a9f in process_running ../src/libsystemd/sd-bus/sd-bus.c:3004
    #12 0x7faf132a9900 in bus_process_internal ../src/libsystemd/sd-bus/sd-bus.c:3224
    #13 0x7faf132a9a9a in sd_bus_process ../src/libsystemd/sd-bus/sd-bus.c:3251
    #14 0x7faf132ad19b in io_callback ../src/libsystemd/sd-bus/sd-bus.c:3602
    #15 0x7faf13465b8c in source_dispatch ../src/libsystemd/sd-event/sd-event.c:3591
    #16 0x7faf1346e3a6 in sd_event_dispatch ../src/libsystemd/sd-event/sd-event.c:4175
    #17 0x7faf1346f50a in sd_event_run ../src/libsystemd/sd-event/sd-event.c:4236
    #18 0x7faf14a85dcd in manager_loop ../src/core/manager.c:3068
    #19 0x41c1e3 in invoke_main_loop ../src/core/main.c:1894
    #20 0x42484a in main ../src/core/main.c:2957
    #21 0x7faf1175555f in __libc_start_call_main (/lib64/libc.so.6+0x2d55f)

?

mrc0mmand · 2022-05-02T15:51:06Z

@mrc0mmand I'll take a look tomorrow I think.

it would be stellar if you could unleash some of your shenanigans on this, since you have the gift for finding interesting issues

Do you mean issues like

==1==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 5521 byte(s) in 10 object(s) allocated from:
    #0 0x7faf155638f7 in strdup (/lib64/libasan.so.6+0x598f7)
    #1 0x7faf1313d06f in strv_extend ../src/basic/strv.c:538
    #2 0x7faf12e4947b in normalize_linked_files ../src/shared/install.c:2839
    #3 0x7faf12e49bc5 in unit_file_reenable ../src/shared/install.c:2872
    #4 0x7faf148e7292 in method_enable_unit_files_generic ../src/core/dbus-manager.c:2332
    #5 0x7faf148e74e6 in method_reenable_unit_files ../src/core/dbus-manager.c:2348
    #6 0x7faf1323c235 in method_callbacks_run ../src/libsystemd/sd-bus/bus-objects.c:406
    #7 0x7faf13245a38 in object_find_and_run ../src/libsystemd/sd-bus/bus-objects.c:1310
    #8 0x7faf132470fa in bus_process_object ../src/libsystemd/sd-bus/bus-objects.c:1430
    #9 0x7faf132a6585 in process_message ../src/libsystemd/sd-bus/sd-bus.c:2962
    #10 0x7faf132a6a9f in process_running ../src/libsystemd/sd-bus/sd-bus.c:3004
    #11 0x7faf132a9900 in bus_process_internal ../src/libsystemd/sd-bus/sd-bus.c:3224
    #12 0x7faf132a9a9a in sd_bus_process ../src/libsystemd/sd-bus/sd-bus.c:3251
    #13 0x7faf132ad19b in io_callback ../src/libsystemd/sd-bus/sd-bus.c:3602
    #14 0x7faf13465b8c in source_dispatch ../src/libsystemd/sd-event/sd-event.c:3591
    #15 0x7faf1346e3a6 in sd_event_dispatch ../src/libsystemd/sd-event/sd-event.c:4175
    #16 0x7faf1346f50a in sd_event_run ../src/libsystemd/sd-event/sd-event.c:4236
    #17 0x7faf14a85dcd in manager_loop ../src/core/manager.c:3068
    #18 0x41c1e3 in invoke_main_loop ../src/core/main.c:1894
    #19 0x42484a in main ../src/core/main.c:2957
    #20 0x7faf1175555f in __libc_start_call_main (/lib64/libc.so.6+0x2d55f)

Direct leak of 963 byte(s) in 10 object(s) allocated from:
    #0 0x7faf155b891f in __interceptor_malloc (/lib64/libasan.so.6+0xae91f)
    #1 0x7faf1555ee04 in __interceptor_strndup.part.0 (/lib64/libasan.so.6+0x54e04)
    #2 0x7faf130e7119 in path_extract_filename ../src/basic/path-util.c:1053
    #3 0x7faf12e48d16 in normalize_linked_files ../src/shared/install.c:2810
    #4 0x7faf12e49bc5 in unit_file_reenable ../src/shared/install.c:2872
    #5 0x7faf148e7292 in method_enable_unit_files_generic ../src/core/dbus-manager.c:2332
    #6 0x7faf148e74e6 in method_reenable_unit_files ../src/core/dbus-manager.c:2348
    #7 0x7faf1323c235 in method_callbacks_run ../src/libsystemd/sd-bus/bus-objects.c:406
    #8 0x7faf13245a38 in object_find_and_run ../src/libsystemd/sd-bus/bus-objects.c:1310
    #9 0x7faf132470fa in bus_process_object ../src/libsystemd/sd-bus/bus-objects.c:1430
    #10 0x7faf132a6585 in process_message ../src/libsystemd/sd-bus/sd-bus.c:2962
    #11 0x7faf132a6a9f in process_running ../src/libsystemd/sd-bus/sd-bus.c:3004
    #12 0x7faf132a9900 in bus_process_internal ../src/libsystemd/sd-bus/sd-bus.c:3224
    #13 0x7faf132a9a9a in sd_bus_process ../src/libsystemd/sd-bus/sd-bus.c:3251
    #14 0x7faf132ad19b in io_callback ../src/libsystemd/sd-bus/sd-bus.c:3602
    #15 0x7faf13465b8c in source_dispatch ../src/libsystemd/sd-event/sd-event.c:3591
    #16 0x7faf1346e3a6 in sd_event_dispatch ../src/libsystemd/sd-event/sd-event.c:4175
    #17 0x7faf1346f50a in sd_event_run ../src/libsystemd/sd-event/sd-event.c:4236
    #18 0x7faf14a85dcd in manager_loop ../src/core/manager.c:3068
    #19 0x41c1e3 in invoke_main_loop ../src/core/main.c:1894
    #20 0x42484a in main ../src/core/main.c:2957
    #21 0x7faf1175555f in __libc_start_call_main (/lib64/libc.so.6+0x2d55f)

?

That's exactly what I'm talking about :-)

mrc0mmand · 2022-05-02T16:16:13Z

@mrc0mmand I'll take a look tomorrow I think.

Thanks! I'll extend the tests to cover the new stuff in the meanwhile, so no rush :-)

evverx · 2022-05-06T11:42:19Z

I think it would have helped to unleash dfuzzer on Thaw to trigger that crash more reliably. But I think what was looking for there was a way to call that function exactly 1000 times. With --min-iterations= and --max-iterations I would have to pass --min-iterations=1000 and --max-iterations=1000 instead of something like --runs 1000. Then again I'm not sure how often I'd need to call methods exactly 1000 times.

evverx · 2022-05-06T11:54:11Z

Also having played with -x a bit more it appears the timeout dfuzzer hits eventually when -x is large enough doesn't change regardless of how many times methods are supposed to be called. I think it should be either floating or maybe dfuzzer should just somehow report that the allotted time isn't enough to call methods with all combinations.

evverx · 2022-05-06T12:00:34Z

it appears the timeout dfuzzer hits eventually when -x is large enough

It could be I'm missing something here. I'll take a look at the logs to figure out whether dfuzzer timed out because the method timed out or whether there is a global dfuzzer timeout.

mrc0mmand · 2022-05-06T12:03:50Z

I think it would have helped to unleash dfuzzer on Thaw to trigger that crash more reliably. But I think what was looking for there was a way to call that function exactly 1000 times. With --min-iterations= and --max-iterations I would have to pass --min-iterations=1000 and --max-iterations=1000 instead of something like --runs 1000. Then again I'm not sure how often I'd need to call methods exactly 1000 times.

Maybe renaming the -x/--max-iterations= option to just -?/--iterations= (and changing the accompanying code) would make sense, since it would cover both your use case and the CI one.

mrc0mmand · 2022-05-06T12:05:00Z

it appears the timeout dfuzzer hits eventually when -x is large enough

It could be I'm missing something here. I'll take a look at the logs to figure out whether dfuzzer timed out because the method timed out or whether there is a global dfuzzer timeout.

Interesting, I'm not aware of any global dfuzzer timeout, but it's definitely something that should get figured out.

evverx · 2022-05-06T12:21:09Z

Maybe renaming the -x/--max-iterations= option to just -?/--iterations=

It would cover that particular use case but in general --max-iterations= is more useful I think. With --iterations=n every method would be called n times and that would mean that void methods would be tested as many times as methods with advanced signatures and that wouldn't be ideal.

mrc0mmand · 2022-05-06T12:41:01Z

Maybe renaming the -x/--max-iterations= option to just -?/--iterations=

It would cover that particular use case but in general --max-iterations= is more useful I think. With --iterations=n every method would be called n times and that would mean that void methods would be tested as many times as methods with advanced signatures and that wouldn't be ideal.

Gotcha. To make matters easier, I tweaked the arguments as follows:

-x/--max-iterations= sets the upper boundary
-y/--min-iterations= sets the lower boundary
-I/--iterations= sets both boundaries to the same value

That should, hopefully, make everybody happy :-)

evverx · 2022-05-06T13:23:07Z

meson.build


 subdir('src')

 executable(
        'dfuzzer',
        dfuzzer_sources,
-        dependencies : [libgio, libffi],
+        dependencies : [libgio],


Could you remove it from README, Makefile, .github/workflows/build.yml and .lgtm.yaml?

Speaking of Makefile, I got access to the Fedora dfuzzer repo, so once we're finished here, I can update the package and drop the Makefile altogether.

Good to know. Thanks!

That would break scripts like https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/c006d814b0776bea492c683d13fb466ffeea4296/tests/security/atsec/dbus_fuzzer.pm#L25-L29 but I guess it should be possible to notify them before dropping Makefile

evverx · 2022-05-06T13:29:26Z

src/dfuzzer.c

@@ -1041,6 +1068,13 @@ void df_print_help(const char *name)
         "                              Requires -o and -i to be set as well.\n"
         "  -b --buffer-limit=SIZE      Maximum buffer size for generated strings in bytes.\n"
         "                              Default: 50K, minimum: 256B.\n"
+         "  -x --max-iterations=ITER    Maximum number of iterations done for each method.\n"
+         "                              By default this value is dynamically calculated from each\n"


I think dfuzzer should default to something more or less meaningful here. Without it -x would always have to be passed and that's not the best user experience I think. It would also make scripts where dfuzzer -n ... is already used run for hours. Another option would be not to roll out advanced signatures by default

I mean, the current limits are somewhat reasonable (when not run under Valgrind), the first implementation used, for example, 1024 iterations for double, or 512 iterations for string (so one of our custom complex signatures yielded over 12k iterations, which ran almost 5 minutes under Valgrind), whereas now it's 32 and 64 respectively.

The "issue" in our CI is that we run dfuzzer on the whole systemd dbus service twice + a couple of less demanding cases, hence the need of -x. But I see your point, I'm not completely sure how to make it more "elegant" though.

(when not run under Valgrind)

I'm not sure what you're implying :-)

I see your point, I'm not completely sure how to make it more "elegant" though.

Let's get it merged and either wait for bug reports or tune it in the process of integrating systemd/systemd#22547

(when not run under Valgrind)

I'm not sure what you're implying :-)

Nothing at all, Valgrind is invaluable when hunting down memleaks in the glib stuff, it just doesn't mix well (performance wise) with all the allocations glib does to generate the GVariant objects, but that's not Valgrind's fault :-)

evverx · 2022-05-06T13:53:24Z

I'll send it to Coverity a bit later

evverx · 2022-05-06T14:28:08Z

src/fuzz.h

 /** Maximum amount of unimportant exceptions for one method; if reached
  * testing continues with a next method */
-#define MAX_EXCEPTIONS 10
+#define MAX_EXCEPTIONS 50


This seems to prevent Thaw from really being run 1000 times. It should probably be configurable as well or maybe it should be dropped altogether. I'm not sure

I'm just looking into it, let me prep a PR which drops it altogether, so we can give it a try.

https://github.com/matusmarhefka/dfuzzer/pull/69

evverx · 2022-05-07T04:45:39Z

src/dfuzzer.c

+         "  -y --min-iterations=ITER    Minimum number of iterations done for each method.\n"
+         "                              Default: 10 iterations; minimum: 1 iteration.\n"
+         "  -I --iterations=ITER        Set both the minimum and maximum number of iterations to ITER\n"
+         "                              See --max-iterations= and --min-iterations= above\n"


The man page should have probably been updated as well. I completely forgot it's there

That's something I want to do as well, but completely forgot, mainly because writing something directly in troff is quite painful. But I'll update it as part of #71.

evverx · 2022-05-07T17:05:20Z

@mrc0mmand looks like method names are no longer included in log files:

org.freedesktop.dfuzzerInterface;/org/freedesktop/dfuzzerObject;df_crash;o;2f50712f4b2f63;Crash

turned into

org.freedesktop.dfuzzerInterface;/org/freedesktop/dfuzzerObject;(o);(objectpath '/',);Crash

It kind of broke reprogen.py: https://github.com/matusmarhefka/dfuzzer/issues/74. Though it isn't compatible with advanced signatures anyway

This got accidentally omitted during the latest rewrite. Addresses: https://github.com/matusmarhefka/dfuzzer/pull/64#issuecomment-1120242055

mrc0mmand force-pushed the complex-types branch from e139fef to 327685c Compare May 2, 2022 12:45

This comment was marked as outdated.

Sign in to view

mrc0mmand force-pushed the complex-types branch from 327685c to 3809f7a Compare May 2, 2022 13:13

This comment was marked as outdated.

Sign in to view

mrc0mmand force-pushed the complex-types branch from 3809f7a to 3d6dae5 Compare May 2, 2022 13:42

mrc0mmand force-pushed the complex-types branch from 3d6dae5 to 8641ef9 Compare May 2, 2022 14:38

mrc0mmand force-pushed the complex-types branch from 8641ef9 to fbbeed2 Compare May 2, 2022 16:13

mrc0mmand force-pushed the complex-types branch 8 times, most recently from 86e4e4a to 61fcce5 Compare May 3, 2022 11:17

mrc0mmand marked this pull request as ready for review May 3, 2022 11:34

evverx mentioned this pull request May 4, 2022

methods receiving booleans are called 10000 times #66

Closed

mrc0mmand force-pushed the complex-types branch 4 times, most recently from 08b1db3 to d953aa9 Compare May 4, 2022 14:05

This comment was marked as resolved.

Sign in to view

mrc0mmand force-pushed the complex-types branch 2 times, most recently from 3b95511 to c49b964 Compare May 4, 2022 14:28

mrc0mmand force-pushed the complex-types branch from 39805f9 to c3aaf5f Compare May 6, 2022 12:39

mrc0mmand force-pushed the complex-types branch 3 times, most recently from df3bfc0 to 921d4f8 Compare May 6, 2022 13:13

evverx reviewed May 6, 2022

View reviewed changes

mrc0mmand force-pushed the complex-types branch from 921d4f8 to 0a56a8c Compare May 6, 2022 13:24

mrc0mmand added 5 commits May 6, 2022 15:27

fuzz: handle advanced signatures as well

3094d2a

test-server: test complex signatures as well

c6fe26a

introspection: get rid of global variables

45471b8

rand: improve random data generation

de2cc04

test: cover a couple of error paths as well

ccfd446

mrc0mmand force-pushed the complex-types branch from 0a56a8c to ccfd446 Compare May 6, 2022 13:27

evverx reviewed May 6, 2022

View reviewed changes

evverx merged commit 03bfcae into dbus-fuzzer:master May 6, 2022

mrc0mmand deleted the complex-types branch May 6, 2022 13:53

evverx reviewed May 6, 2022

View reviewed changes

mrc0mmand mentioned this pull request May 6, 2022

fuzz: don't bail out early on exceptions #69

Closed

evverx reviewed May 7, 2022

View reviewed changes

evverx mentioned this pull request May 7, 2022

reprogen.py seems to be obsolete #74

Closed

mrc0mmand referenced this pull request in mrc0mmand/dfuzzer May 7, 2022

fuzz: log the method name as well

b93649c

This got accidentally omitted during the latest rewrite. Addresses: https://github.com/matusmarhefka/dfuzzer/pull/64#issuecomment-1120242055

mrc0mmand mentioned this pull request May 7, 2022

fuzz: log the method name as well #76

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fuzz: handle advanced signatures as well #64

fuzz: handle advanced signatures as well #64

mrc0mmand commented May 1, 2022

mrc0mmand commented May 1, 2022

mrc0mmand commented May 2, 2022

This comment was marked as outdated.

This comment was marked as outdated.

mrc0mmand commented May 2, 2022 •

edited

Loading

evverx commented May 2, 2022

mrc0mmand commented May 2, 2022

mrc0mmand commented May 2, 2022

This comment was marked as resolved.

evverx commented May 6, 2022

evverx commented May 6, 2022 •

edited

Loading

evverx commented May 6, 2022

mrc0mmand commented May 6, 2022

mrc0mmand commented May 6, 2022

evverx commented May 6, 2022

mrc0mmand commented May 6, 2022

evverx May 6, 2022

mrc0mmand May 6, 2022

mrc0mmand May 6, 2022

evverx May 6, 2022

evverx May 6, 2022

mrc0mmand May 6, 2022 •

edited

Loading

evverx May 6, 2022

mrc0mmand May 6, 2022

evverx commented May 6, 2022

evverx May 6, 2022

mrc0mmand May 6, 2022

mrc0mmand May 6, 2022

evverx May 7, 2022

mrc0mmand May 7, 2022

evverx commented May 7, 2022 •

edited

Loading

fuzz: handle advanced signatures as well #64

fuzz: handle advanced signatures as well #64

Conversation

mrc0mmand commented May 1, 2022

mrc0mmand commented May 1, 2022

mrc0mmand commented May 2, 2022

This comment was marked as outdated.

This comment was marked as outdated.

mrc0mmand commented May 2, 2022 • edited Loading

evverx commented May 2, 2022

mrc0mmand commented May 2, 2022

mrc0mmand commented May 2, 2022

This comment was marked as resolved.

evverx commented May 6, 2022

evverx commented May 6, 2022 • edited Loading

evverx commented May 6, 2022

mrc0mmand commented May 6, 2022

mrc0mmand commented May 6, 2022

evverx commented May 6, 2022

mrc0mmand commented May 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrc0mmand May 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evverx commented May 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evverx commented May 7, 2022 • edited Loading

mrc0mmand commented May 2, 2022 •

edited

Loading

evverx commented May 6, 2022 •

edited

Loading

mrc0mmand May 6, 2022 •

edited

Loading

evverx commented May 7, 2022 •

edited

Loading