Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20221109104613.48062-1-izbyshev@ispras.ru>
Date: Wed,  9 Nov 2022 13:46:13 +0300
From: Alexey Izbyshev <izbyshev@...ras.ru>
To: musl@...ts.openwall.com
Subject: [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

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.