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

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

mrc0mmand
Copy link
Member

WIP

@mrc0mmand mrc0mmand mentioned this pull request May 6, 2022
Merged
@mrc0mmand
Copy link
Member Author

@evverx I wonder, do you have the exact exception, which prevented you from running Thaw 1000 times? Because right now I can crash systemd even without this patch (and finally on the latest systemd main):

# dfuzzer -s -d --log-dir logs -n org.freedesktop.systemd1 -o /org/freedesktop/systemd1/unit/system_2dgetty_2eslice -i org.freedesktop.systemd1.Unit -t Thaw
[SESSION BUS]
Bus not found.
Error in g_bus_get_sync(): Cannot autolaunch D-Bus without X11 $DISPLAY
[SYSTEM BUS]
[PROCESS: /usr/lib/systemd/systemd]
[CONNECTED TO PID: 1]
Object: /org/freedesktop/systemd1/unit/system_2dgetty_2eslice
 Interface: org.freedesktop.systemd1.Unit
  Method: Thaw () => 10 iterations
  Thaw...
  SKIP Thaw - timeout reached
Exit status: 0

# dfuzzer -s -d --log-dir logs -n org.freedesktop.systemd1 -o /org/freedesktop/systemd1/unit/system_2dgetty_2eslice -i org.freedesktop.systemd1.Unit -t Thaw
[SESSION BUS]
Bus not found.
Error in g_bus_get_sync(): Cannot autolaunch D-Bus without X11 $DISPLAY
[SYSTEM BUS]
[PROCESS: /usr/lib/systemd/systemd]
[CONNECTED TO PID: 1]
Object: /org/freedesktop/systemd1/unit/system_2dgetty_2eslice
 Interface: org.freedesktop.systemd1.Unit
  Method: Thaw () => 10 iterations
  Thaw...
Broadcast message from systemd-journald@fedora (Fri 2022-05-06 14:58:39 UTC):

systemd[1]: Caught <ABRT> from our own process.


Broadcast message from systemd-journald@fedora (Fri 2022-05-06 14:58:40 UTC):

systemd[1]: Caught <ABRT>, dumped core as pid 817.

Exception: org.freedesktop.DBus.Error.NoReply
   -- Signature: ()
   -- Value: ()
Exception: org.freedesktop.DBus.Error.ServiceUnknown
  EXCE Thaw - D-Bus exception thrown: The name is not activatable
   -- Signature: ()
   -- Value: ()
Exception: org.freedesktop.DBus.Error.ServiceUnknown
  EXCE Thaw - D-Bus exception thrown: The name is not activatable
   -- Signature: ()
   -- Value: ()
Exception: org.freedesktop.DBus.Error.ServiceUnknown
  EXCE Thaw - D-Bus exception thrown: The name is not activatable
   -- Signature: ()
   -- Value: ()
Exception: org.freedesktop.DBus.Error.ServiceUnknown
  EXCE Thaw - D-Bus exception thrown: The name is not activatable
   -- Signature: ()
   -- Value: ()
Exception: org.freedesktop.DBus.Error.ServiceUnknown
  EXCE Thaw - D-Bus exception thrown: The name is not activatable
   -- Signature: ()
   -- Value: ()
Exception: org.freedesktop.DBus.Error.ServiceUnknown
  EXCE Thaw - D-Bus exception thrown: The name is not activatable
   -- Signature: ()
   -- Value: ()
Exception: org.freedesktop.DBus.Error.ServiceUnknown
  EXCE Thaw - D-Bus exception thrown: The name is not activatable
   -- Signature: ()
   -- Value: ()
Exception: org.freedesktop.DBus.Error.ServiceUnknown
  EXCE Thaw - D-Bus exception thrown: The name is not activatable
   -- Signature: ()
   -- Value: ()
Exception: org.freedesktop.DBus.Error.ServiceUnknown
  EXCE Thaw - D-Bus exception thrown: The name is not activatable

Broadcast message from systemd-journald@fedora (Fri 2022-05-06 14:58:40 UTC):

systemd[1]: Freezing execution.

   -- Signature: ()
   -- Value: ()
  PASS Thaw
Exit status: 0

@evverx
Copy link
Member

evverx commented May 6, 2022

I can crash systemd more or less reliably too. It's just that if I pass -I 1000 I'd expect it to call methods exactly 1000 times regardless of whether there are exceptions or not. In this particular case the exception was

  EXCE Thaw - D-Bus exception thrown: Unit has a pending job.

I'm not sure it should be removed though.

@mrc0mmand
Copy link
Member Author

I see, thanks, I haven't seen that exception before.

Anyway, I think we should continue even if we receive an "unhandled"[0] exception n-times, since that might be useful anyway, e.g.:

Object: /org/freedesktop/systemd1/unit/boot_2emount
 Interface: org.freedesktop.DBus.Peer
  PASS Ping
  PASS GetMachineId
 Interface: org.freedesktop.DBus.Introspectable
  PASS Introspect
 Interface: org.freedesktop.DBus.Properties
...
  Clean...
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.systemd1.NothingToClean
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.systemd1.NothingToClean
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.systemd1.NothingToClean
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.systemd1.NothingToClean
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.DBus.Error.InvalidArgs
Exception: org.freedesktop.systemd1.NothingToClean
...

since sometimes we might hit different paths based on the passed value. Basically what I mentioned in https://github.com/matusmarhefka/dfuzzer/pull/64#issuecomment-1117395111.

[0] Exception not handled by
https://github.com/matusmarhefka/dfuzzer/blob/03bfcae6a7619c859d3dbc3c3c1bf2281fb04a15/src/fuzz.c#L582-L597

@evverx
Copy link
Member

evverx commented May 6, 2022

I think the idea was to somewhat try to figure out whether dfuzzer gets any closer to triggering new code paths and it was probably based on the assumption that it doesn't make sense to keep wasting time triggering the same exceptions over and over again. For that to really work I think dfuzzer should actually look at exceptions it triggers instead of just counting them.

@evverx
Copy link
Member

evverx commented May 6, 2022

What I'm trying to say is that before removing it it would be great to figure out why it was introduced in the first place. My guess would be that it was used as a signal allowing dfuzzer to decide whether whatever it does makes sense but I'm just speculating

@mrc0mmand
Copy link
Member Author

I see. I guess modifying the counter to keep counting only if the current invocation caused the same exception as the one before would make sense (and shouldn't be too complicated to implement).

@mrc0mmand
Copy link
Member Author

What I'm trying to say is that before removing it it would be great to figure out why it was introduced in the first place. My guess would be that it was used as a signal allowing dfuzzer to decide whether whatever it does makes sense but I'm just speculating

I guess that would make sense, given the original commit 7b821a8 but yeah, it just a speculation.

@evverx
Copy link
Member

evverx commented May 9, 2022

@mrc0mmand I think this PR can be closed. I think it's safe to say that it's one of those heuristics guiding dfuzzer that can be discussed in https://github.com/matusmarhefka/dfuzzer/issues/81. WDYT?

@mrc0mmand
Copy link
Member Author

@mrc0mmand I think this PR can be closed. I think it's safe to say that it's one of those heuristics guiding dfuzzer that can be discussed in #81. WDYT?

That definitely sounds like a plan :-)

@mrc0mmand mrc0mmand closed this May 9, 2022
@mrc0mmand mrc0mmand deleted the exceptions-tweak branch May 9, 2022 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mrc0mmand @evverx