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,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 11569 invoked from network); 3 Sep 2022 00:19:59 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 3 Sep 2022 00:19:59 -0000 Received: (qmail 1860 invoked by uid 550); 3 Sep 2022 00:19:56 -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 1825 invoked from network); 3 Sep 2022 00:19:55 -0000 From: Alexey Izbyshev To: musl@lists.openwall.com Date: Sat, 3 Sep 2022 03:19:40 +0300 Message-Id: <20220903001940.185345-1-izbyshev@ispras.ru> X-Mailer: git-send-email 2.37.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [musl] [PATCH] fix potential ref count overflow in sem_open() sem_open() attempts to avoid overflow on the future ref count increment by computing the sum of all ref counts in semtab and checking that it's not INT_MAX when semtab is locked for slot reservation. This approach suffers from a TOCTTOU: by the time semtab is re-locked after opening a semaphore, the sum could have already been increased by a concurrent sem_open(), so it will overflow on the increment. An individual ref count can be overflowed as well if the call happened to open a duplicate of the only other semaphore. Moreover, since the overflow avoidance check admits a negative (i.e. overflowed) sum, after this state is reached, an individual ref count can be incremented until it reaches 1 again, and then sem_close() will incorrectly free the semaphore, promoting what could be just a signed-overflow UB to a use-after-free. Fix the overflow check by accounting for the maximum possible number of concurrent sem_open() calls that have already reserved a slot, but haven't incremented the ref count yet. --- Alternatively, we could use the number of currently reserved slots instead of SEM_NSEMS_MAX, preserving the ability of individual ref counts to reach INT_MAX in non-concurrent scenarios. I'm not sure whether it matters, so I'm sending a smaller of the two fixes. Alexey --- src/thread/sem_open.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/thread/sem_open.c b/src/thread/sem_open.c index 0ad29de9..611a3f64 100644 --- a/src/thread/sem_open.c +++ b/src/thread/sem_open.c @@ -61,7 +61,7 @@ sem_t *sem_open(const char *name, int flags, ...) if (!semtab[i].sem && slot < 0) slot = i; } /* Avoid possibility of overflow later */ - if (cnt == INT_MAX || slot < 0) { + if (cnt > INT_MAX - SEM_NSEMS_MAX || slot < 0) { errno = EMFILE; UNLOCK(lock); return SEM_FAILED; -- 2.37.2