From 6726fc1bf38096ae44966a2580cb77fef505deb5 Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Mon, 2 Dec 2024 11:45:55 -0500 Subject: [PATCH] aio: fix data race in aio This particular problem reported by the sanitizer was unlikely to have any real impact, but using this boolean and avoiding the clearing of the expire_q avoids a possible NULL pointer dereference. --- src/core/aio.c | 24 ++++++++++++++---------- src/core/aio.h | 14 +++++++++----- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/core/aio.c b/src/core/aio.c index 27926e651..d3f3e3205 100644 --- a/src/core/aio.c +++ b/src/core/aio.c @@ -9,6 +9,7 @@ // #include "core/nng_impl.h" +#include "core/platform.h" #include "nng/nng.h" #include @@ -91,6 +92,12 @@ nni_aio_init(nni_aio *aio, nni_cb cb, void *arg) aio->a_init = true; } +static bool +aio_stopped(nni_aio *aio) +{ + return (nni_atomic_get_bool(&aio->a_stopped)); +} + void nni_aio_fini(nni_aio *aio) { @@ -125,7 +132,7 @@ nni_aio_free(nni_aio *aio) void nni_aio_reap(nni_aio *aio) { - if (aio != NULL && aio->a_expire_q != NULL) { + if (aio != NULL && aio->a_init) { nni_reap(&aio_reap_list, aio); } } @@ -158,7 +165,7 @@ nni_aio_set_iov(nni_aio *aio, unsigned nio, const nni_iov *iov) void nni_aio_stop(nni_aio *aio) { - if (aio != NULL && aio->a_expire_q != NULL) { + if (aio != NULL && aio->a_init && !aio_stopped(aio)) { nni_aio_cancel_fn fn; void *arg; nni_aio_expire_q *eq = aio->a_expire_q; @@ -180,17 +187,14 @@ nni_aio_stop(nni_aio *aio) nni_aio_wait(aio); - // Disconnect us from the expire queue. - // We cannot be used any more. This reduces - // lock contention during teardown. - aio->a_expire_q = NULL; + nni_atomic_set_bool(&aio->a_stopped, true); } } void nni_aio_close(nni_aio *aio) { - if (aio != NULL && aio->a_expire_q != NULL) { + if (aio != NULL && aio->a_init && !aio_stopped(aio)) { nni_aio_cancel_fn fn; void *arg; nni_aio_expire_q *eq = aio->a_expire_q; @@ -293,7 +297,7 @@ nni_aio_count(nni_aio *aio) void nni_aio_wait(nni_aio *aio) { - if (aio != NULL && aio->a_expire_q != NULL) { + if (aio != NULL && aio->a_init && !aio_stopped(aio)) { nni_task_wait(&aio->a_task); } } @@ -314,7 +318,7 @@ nni_aio_begin(nni_aio *aio) // checks may wish ignore or suppress these checks. nni_aio_expire_q *eq = aio->a_expire_q; - if (eq == NULL) { + if (aio_stopped(aio)) { NNI_ASSERT(!nni_list_node_active(&aio->a_expire_node)); aio->a_result = NNG_ECANCELED; aio->a_cancel_fn = NULL; @@ -341,7 +345,7 @@ nni_aio_begin(nni_aio *aio) aio->a_expire = NNI_TIME_NEVER; aio->a_sleep = false; aio->a_expire_ok = false; - aio->a_expire_q = NULL; + nni_atomic_set_bool(&aio->a_stopped, true); nni_mtx_unlock(&eq->eq_mtx); return (NNG_ECANCELED); diff --git a/src/core/aio.h b/src/core/aio.h index 7ec3c8001..80e50d936 100644 --- a/src/core/aio.h +++ b/src/core/aio.h @@ -1,5 +1,5 @@ // -// Copyright 2023 Staysail Systems, Inc. +// Copyright 2024 Staysail Systems, Inc. // Copyright 2018 Capitar IT Group BV // // This software is supplied under the terms of the MIT License, a @@ -13,6 +13,7 @@ #include "core/defs.h" #include "core/list.h" +#include "core/platform.h" #include "core/reap.h" #include "core/taskq.h" #include "core/thread.h" @@ -219,7 +220,8 @@ struct nng_aio { bool a_expiring; // Expiration in progress bool a_use_expire; // Use expire instead of timeout bool a_init; // This is initialized - nni_task a_task; + + nni_task a_task; // Read/write operations. nni_iov a_iov[8]; @@ -234,14 +236,16 @@ struct nng_aio { void *a_inputs[4]; void *a_outputs[4]; + nni_atomic_bool a_stopped; + nni_aio_expire_q *a_expire_q; + nni_list_node a_expire_node; // Expiration node + nni_reap_node a_reap_node; + // Provider-use fields. nni_aio_cancel_fn a_cancel_fn; void *a_cancel_arg; void *a_prov_data; nni_list_node a_prov_node; // Linkage on provider list. - nni_aio_expire_q *a_expire_q; - nni_list_node a_expire_node; // Expiration node - nni_reap_node a_reap_node; }; #endif // CORE_AIO_H