Bug 41126 - dbus_g_proxy_new_for_peer, on a bus, causes crashes
Summary: dbus_g_proxy_new_for_peer, on a bus, causes crashes
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: GLib (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: John (J5) Palmieri
URL: http://cgit.freedesktop.org/~smcv/dbu...
Whiteboard: review+
Keywords: patch
Depends on: 40151
Blocks: 41129
  Show dependency treegraph
 
Reported: 2011-09-22 11:15 UTC by Simon McVittie
Modified: 2012-06-25 09:20 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Fix two crashes when dbus_g_proxy_new_for_peer is used on a bus (8.25 KB, patch)
2011-09-22 11:16 UTC, Simon McVittie
Details | Splinter Review
Add a utility function to tear down a private connection in tests (6.40 KB, patch)
2011-09-22 11:18 UTC, Simon McVittie
Details | Splinter Review
[1/2] Add a utility function to tear down a private connection in tests (6.42 KB, patch)
2012-04-16 04:13 UTC, Simon McVittie
Details | Splinter Review
[2/2] Fix two crashes when dbus_g_proxy_new_for_peer is used on a bus (8.30 KB, patch)
2012-04-16 04:13 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2011-09-22 11:15:15 UTC
dbus_g_proxy_new_for_peer, on a bus (dbus-daemon), ought to talk to the dbus-daemon itself (admittedly it's not amazingly useful to do so, since most of the bus daemon's methods require the destination "org.freedesktop.DBus" to be specified). It certainly shouldn't crash dbus-glib, which I discovered it did when I tried using it in an unrelated regression test. Patch to follow.
Comment 1 Simon McVittie 2011-09-22 11:16:26 UTC
Created attachment 51526 [details] [review]
Fix two crashes when dbus_g_proxy_new_for_peer is used on a bus

The first part of the bug is that when NameOwnerChanged is received with
a dbus_g_proxy_new_for_peer (which has no name) alive, checking
whether it was affected by the NameOwnerChanged caused a NULL
dereference and segfault.

The second part of the bug is that if the last proxy in existence is
for a peer, when it was unregistered there would be no owner_match_rules,
causing a crash.

Both are exercised in the new test added here.
Comment 2 Simon McVittie 2011-09-22 11:18:33 UTC
Created attachment 51527 [details] [review]
Add a utility function to tear down a private connection in tests

The test added in Attachment #51526 [details] needs this, too.
Comment 3 Simon McVittie 2012-04-16 04:13:09 UTC
Created attachment 60057 [details] [review]
[1/2] Add a utility function to tear down a private connection  in tests

---

Rebased onto the branch from Bug #40151, since that branch is required in order to make tests pass again.
Comment 4 Simon McVittie 2012-04-16 04:13:39 UTC
Created attachment 60058 [details] [review]
[2/2] Fix two crashes when dbus_g_proxy_new_for_peer is used    on a bus

The first part of the bug is that when NameOwnerChanged is received with
a dbus_g_proxy_new_for_peer (which has no name) alive, checking
whether it was affected by the NameOwnerChanged caused a NULL
dereference and segfault.              
                                       
The second part of the bug is that if the last proxy in existence is
for a peer, when it was unregistered there would be no owner_match_rules,      
causing a crash.                                        
                                                   
Both are exercised in the new test added here.

---

Also rebased onto Bug #40151.
Comment 5 Will Thompson 2012-04-20 07:14:33 UTC
Comment on attachment 60057 [details] [review]
[1/2] Add a utility function to tear down a private connection  in tests

Review of attachment 60057 [details] [review]:
-----------------------------------------------------------------

Looks good.
Comment 6 Will Thompson 2012-04-20 07:14:45 UTC
Comment on attachment 60058 [details] [review]
[2/2] Fix two crashes when dbus_g_proxy_new_for_peer is used    on a bus

Review of attachment 60058 [details] [review]:
-----------------------------------------------------------------

Great, ship it.
Comment 7 Simon McVittie 2012-06-25 09:20:53 UTC
Thanks, fixed in git for 0.100


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.