GNOME Bugzilla – Bug 723774
Prevent use of a GSource after it’s been destroyed in the gnutls code
Last modified: 2014-02-18 15:07:12 UTC
It’s possible for a GSource to be modified (have child sources added or removed) after being destroyed in an earlier user-provided callback function. See the patch descriptions for full details.
Created attachment 268302 [details] [review] gnutls: Tidy up reference handling for a child GSource g_source_add_child_source() adds a reference to the child, so it only complicates matters to add one ourselves, especially when the child source should be finalised at the same time as the parent.
Created attachment 268303 [details] [review] gnutls: Prevent use of a destroyed GSource in a callback It’s possible for the GTlsConnectionGnutlsSource to be destroyed in its user-provided callback function. If gnutls_source_sync() is called after this, it ends up attempting to modify the GSource after it’s been destroyed, which is disallowed. Bail out of gnutls_source_sync() early if the source has already been destroyed.
Comment on attachment 268302 [details] [review] gnutls: Tidy up reference handling for a child GSource >g_source_add_child_source() adds a reference to the child, so it only >complicates matters to add one ourselves, especially when the child >source should be finalised at the same time as the parent. g_source_add_child_source() adds a ref for its own purposes, but if we are going to keep a pointer to it, then we ought to keep our own ref as well, for safety. Is the current code causing some problem, or did you just find it inaesthetic?
Comment on attachment 268303 [details] [review] gnutls: Prevent use of a destroyed GSource in a callback this one is fine, though I don't think the comment is necessary; the code itself is self-documenting
Comment on attachment 268303 [details] [review] gnutls: Prevent use of a destroyed GSource in a callback commit 2be6abe961294bfe893ea47f2b41ebb2894ca8a2 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Thu Feb 6 15:32:51 2014 +0000 gnutls: Prevent use of a destroyed GSource in a callback It’s possible for the GTlsConnectionGnutlsSource to be destroyed in its user-provided callback function. If gnutls_source_sync() is called after this, it ends up attempting to modify the GSource after it’s been destroyed, which is disallowed. Bail out of gnutls_source_sync() early if the source has already been destroyed. https://bugzilla.gnome.org/show_bug.cgi?id=723774 tls/gnutls/gtlsconnection-gnutls.c | 4 ++++ 1 file changed, 4 insertions(+)
(In reply to comment #3) > (From update of attachment 268302 [details] [review]) > >g_source_add_child_source() adds a reference to the child, so it only > >complicates matters to add one ourselves, especially when the child > >source should be finalised at the same time as the parent. > > g_source_add_child_source() adds a ref for its own purposes, but if we are > going to keep a pointer to it, then we ought to keep our own ref as well, for > safety. Is the current code causing some problem, or did you just find it > inaesthetic? The current code isn’t causing a problem, but was a little harder to follow when I was debugging this (I initially thought it was a ref counting issue). Personally I think it’s clearer if the reference is held by g_source_add_child_source(), since then it’s guaranteed not to be leaked when the child is removed, but that is just aesthetics with a touch of maintainability.
ok, i prefer it the way it is now, so I'm going to leave it as is. Thanks for the other patch (which made it into 2.39.90)