From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_MSPIKE_H2 autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 23682 invoked from network); 9 Nov 2022 10:46:48 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 9 Nov 2022 10:46:48 -0000 Received: (qmail 7882 invoked by uid 550); 9 Nov 2022 10:46:44 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 7847 invoked from network); 9 Nov 2022 10:46:43 -0000 DKIM-Filter: OpenDKIM Filter v2.11.0 mail.ispras.ru 24A4A419E9ED From: Alexey Izbyshev To: musl@lists.openwall.com Date: Wed, 9 Nov 2022 13:46:13 +0300 Message-Id: <20221109104613.48062-1-izbyshev@ispras.ru> X-Mailer: git-send-email 2.37.2 MIME-Version: 1.0 Mail-Followup-To: musl@lists.openwall.com Content-Transfer-Encoding: 8bit Subject: [musl] [PATCH] mq_notify: fix close/recv race on failure path In case of failure mq_notify closes the socket immediately after sending a cancellation request to the worker thread that is going to call or have already called recv on that socket. Even if we don't consider the kernel behavior when the only descriptor to an object that is being used in a system call is closed, if the socket descriptor is closed before the kernel looks at it, another thread could open a descriptor with the same value in the meantime, resulting in recv acting on a wrong object. Fix the race by moving pthread_cancel call before the barrier wait to guarantee that the cancellation flag is set before the worker thread enters recv. --- Other ways to fix this: * Remove the racing close call from mq_notify and surround recv with pthread_cleanup_push/pop. * Make the worker thread joinable initially, join it before closing the socket on the failure path, and detach it on the happy path. This would also require disabling cancellation around join/detach to ensure that mq_notify itself is not cancelled in an inappropriate state. Alexey --- src/mq/mq_notify.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/mq/mq_notify.c b/src/mq/mq_notify.c index 221591c7..177caccd 100644 --- a/src/mq/mq_notify.c +++ b/src/mq/mq_notify.c @@ -34,7 +34,7 @@ int mq_notify(mqd_t mqd, const struct sigevent *sev) struct args args = { .sev = sev }; pthread_attr_t attr; pthread_t td; - int s; + int s, r; struct sigevent sev2; static const char zeros[32]; @@ -56,18 +56,19 @@ int mq_notify(mqd_t mqd, const struct sigevent *sev) return -1; } - pthread_barrier_wait(&args.barrier); - pthread_barrier_destroy(&args.barrier); - sev2.sigev_notify = SIGEV_THREAD; sev2.sigev_signo = s; sev2.sigev_value.sival_ptr = (void *)&zeros; - if (syscall(SYS_mq_notify, mqd, &sev2) < 0) { + r = syscall(SYS_mq_notify, mqd, &sev2); + if (r < 0) pthread_cancel(td); + + pthread_barrier_wait(&args.barrier); + pthread_barrier_destroy(&args.barrier); + + if (r < 0) __syscall(SYS_close, s); - return -1; - } - return 0; + return r; } -- 2.37.2