From ffd87f771857c9470bea639903f6c937c401c6b1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 8 Dec 2020 19:37:49 +0100 Subject: shared: fix race in nm_ref_string_unref() We cannot drop the reference count to zero while having no lock. Otherwise, another thread might race doing s = nm_ref_string_new("..."); nm_ref_string_unref(s); and already successfully delete the instance. Hitting this race should be rather difficult, especially because we tend to use NMRefString only from one thread. But still, access to global variables must be race free. Fixes: 908fadec964e ('shared: add NMRefString') (cherry picked from commit 3490a09a7d966d11b971d08a2bb802f76f6d5e58) (cherry picked from commit 1a68a54471d304b150a9e9ace43c2fe3f939f139) (cherry picked from commit 71f56a5d76352f673166c3451984f265b4de1431) --- shared/nm-glib-aux/nm-ref-string.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/shared/nm-glib-aux/nm-ref-string.c b/shared/nm-glib-aux/nm-ref-string.c index 0a0b0d3a41..ece69b7473 100644 --- a/shared/nm-glib-aux/nm-ref-string.c +++ b/shared/nm-glib-aux/nm-ref-string.c @@ -169,24 +169,25 @@ void _nm_ref_string_unref_non_null (NMRefString *rstr) { RefString *const rstr0 = (RefString *) rstr; + int r; _ASSERT (rstr0); - if (G_LIKELY (!g_atomic_int_dec_and_test (&rstr0->ref_count))) + /* fast-path: first try to decrement the ref-count without bringing it + * to zero. */ + r = rstr0->ref_count; + if (G_LIKELY (r > 1 && g_atomic_int_compare_and_exchange (&rstr0->ref_count, r, r - 1))) return; + /* We apparently are about to return the last reference. Take a lock. */ + G_LOCK (gl_lock); - /* in the fast-path above, we already decremented the ref-count to zero. - * We need recheck that the ref-count is still zero. */ + nm_assert (g_hash_table_lookup (gl_hash, rstr0) == rstr0); - if (g_atomic_int_get (&rstr0->ref_count) == 0) { + if (G_LIKELY(g_atomic_int_dec_and_test (&rstr0->ref_count))) { if (!g_hash_table_remove (gl_hash, rstr0)) nm_assert_not_reached (); - } else { -#if NM_MORE_ASSERTS > 5 - nm_assert (g_hash_table_lookup (gl_hash, rstr0) == rstr0); -#endif } G_UNLOCK (gl_lock); -- cgit v1.2.1