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

fwupd uses GFileMonitor, which has potential deadlock bug #2350

Closed
bleungatchromium opened this issue Sep 1, 2020 · 0 comments · Fixed by #2351
Closed

fwupd uses GFileMonitor, which has potential deadlock bug #2350

bleungatchromium opened this issue Sep 1, 2020 · 0 comments · Fixed by #2351
Labels

Comments

@bleungatchromium
Copy link
Collaborator

Describe the bug
This is a report from an issue we observed in Chrome OS's infrastructure.

Hi all, we recently uncovered a bug in the p2p package that came back to an issue in GLib's file monitor deadlocking:

https://crrev.com/c/2382130
https://bugs.chromium.org/p/chromium/issues/detail?id=1119393

(GLib bug here: https://gitlab.gnome.org/GNOME/glib/-/issues/1941)

I searched through the codebase for other uses and fwupd seems to be the only location, so I wanted to bring your attention to it. fwupd might need to be patched or updated.

The workaround seems to be to ensure g_file_monitor_cancel is called explicitly before disposing of the handle.

Steps to Reproduce
Currently, it is hypothetical in fwupd, but given that this is a known issue in GFileMonitor, it would be a good idea to defensively apply the workaround (g_file_monitor_cancel before disposal of the handle).

There's only a handful of uses of g_file_monitor in fwupd.
$grep -rni g_file_monitor * plugins/thunderbolt/fu-self-test.c:716: ok = g_file_monitor_cancel (monitor); plugins/thunderbolt/fu-self-test.c:771: monitor = g_file_monitor_file (f, G_FILE_MONITOR_NONE, NULL, &error); src/fu-remote-list.c:90: monitor = g_file_monitor (file, G_FILE_MONITOR_NONE, NULL, error); src/fu-config.c:206: self->monitor = g_file_monitor (file, G_FILE_MONITOR_NONE, NULL, error); src/fu-main.c:1665: priv->argv0_monitor = g_file_monitor_file (argv0_file, G_FILE_MONITOR_NONE,

Expected behavior
A clear and concise description of what you expected to happen.

fwupd version information
Please provide the version of the daemon and client.

$ fwupdmgr --version

Please note how you installed it (apt, dnf, pacman, source, etc):

fwupd device information
Please provide the output of the fwupd devices recognized in your system.

$ fwupdmgr get-devices --show-all-devices

Additional questions

  • Operating system and version:
  • Have you tried rebooting?
  • Is this a regression?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

1 participant