From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from hall.aurel32.net (hall.aurel32.net [IPv6:2001:bc8:30d7:100::1]) by sourceware.org (Postfix) with ESMTPS id E16F0384F985 for ; Sun, 3 Dec 2023 12:11:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E16F0384F985 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=aurel32.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=aurel32.net ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E16F0384F985 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:bc8:30d7:100::1 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701605509; cv=none; b=V7VhZMTUz7Qc9yFMBRqb4X6unW6BWJhX1Y+69CBC0SySwMzk1py3sHfmrcYcRQfcN/cyY+mj0VwQ/wXstyIbwFkrHPkrTWDeuOZabEbZgvwb0S2LDS+PX8GyQZhp4gr5KdCnNapFEYKA++VA6oGaRcmSts+lo7wxCn73qjSdqpY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701605509; c=relaxed/simple; bh=61IbPR08V6/OW0QCdvSVOa/LGlthWwQRifT0pO/ls+I=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=A0UV4OZmwhgcMFXYdB00Afa4MvgjoSkL+xlR+rvZM/sRh27xhyEZoO9ejAbgvAOBOfGLtob6Bo7//JYpBSVvWBNVucLOe6vq4KAPGC5hYsuhgzMyptpOIU3x451/L/nc51b70CPbhz0pX6HLedPT/GQ5kDCQTi+y73d6uSzl868= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=aurel32.net ; s=202004.hall; h=Content-Transfer-Encoding:MIME-Version:Message-ID:Date: Subject:Cc:To:From:Content-Type:From:Reply-To:Subject:Content-ID: Content-Description:In-Reply-To:References:X-Debbugs-Cc; bh=59+gA18pKOlGODbof7RInxLPtLFgGqkEzqWhfEtF3DE=; b=DOLuTGDlHk61/uy/l8YEy0rP9p F4SQMKuMiBECa96NpHdRag2NBctGCutVKz4m4mwA4xF1YjqxVLlI0F2oC9+610j4aIpoUaDCZl4hm aukFbCxlxkhs/q7nZ35Rp4VBFPVhJ1g7WgXIxyw7vUheh7wto1Z5pI4kPL1+KYtFiXdjKtpp+uUQT n5CCpD3uUHAoL9dPZQSFHgZegG9mnJWJ9mNSKQrHK1JXSH5NflkewlS+s5nK8h+RGcROCI5kt4+vE OiX9jold06JqGFcwffs9vO/btxuZ7nyROyyNVRBETNWlPScine90XCD0jl3OeDOo9wmULZCvLxq3F k1rJkaVg==; Received: from ohm.aurel32.net ([2001:bc8:30d7:111::2] helo=ohm.rr44.fr) by hall.aurel32.net with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1r9lK4-00BkoP-3d; Sun, 03 Dec 2023 13:11:44 +0100 Received: from aurel32 by ohm.rr44.fr with local (Exim 4.97) (envelope-from ) id 1r9lK3-0000000FdAH-2qCk; Sun, 03 Dec 2023 13:11:43 +0100 From: Aurelien Jarno To: libc-stable@sourceware.org Cc: Sergio Durigan Junior , Simon Chopin , Adhemerval Zanella Netto Subject: [COMMITTED 2.28] sysdeps: sem_open: Clear O_CREAT when semaphore file is expected to exist [BZ #30789] Date: Sun, 3 Dec 2023 12:50:07 +0100 Message-ID: <20231203121102.3708415-1-aurelien@aurel32.net> X-Mailer: git-send-email 2.42.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-13.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: From: Sergio Durigan Junior When invoking sem_open with O_CREAT as one of its flags, we'll end up in the second part of sem_open's "if ((oflag & O_CREAT) == 0 || (oflag & O_EXCL) == 0)", which means that we don't expect the semaphore file to exist. In that part, open_flags is initialized as "O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC" and there's an attempt to open(2) the file, which will likely fail because it won't exist. After that first (expected) failure, some cleanup is done and we go back to the label "try_again", which lives in the first part of the aforementioned "if". The problem is that, in that part of the code, we expect the semaphore file to exist, and as such O_CREAT (this time the flag we pass to open(2)) needs to be cleaned from open_flags, otherwise we'll see another failure (this time unexpected) when trying to open the file, which will lead the call to sem_open to fail as well. This can cause very strange bugs, especially with OpenMPI, which makes extensive use of semaphores. Fix the bug by simplifying the logic when choosing open(2) flags and making sure O_CREAT is not set when the semaphore file is expected to exist. A regression test for this issue would require a complex and cpu time consuming logic, since to trigger the wrong code path is not straightforward due the racy condition. There is a somewhat reliable reproducer in the bug, but it requires using OpenMPI. This resolves BZ #30789. See also: https://bugs.launchpad.net/ubuntu/+source/h5py/+bug/2031912 Signed-off-by: Sergio Durigan Junior Co-Authored-By: Simon Chopin Co-Authored-By: Adhemerval Zanella Netto Fixes: 533deafbdf189f5fbb280c28562dd43ace2f4b0f ("Use O_CLOEXEC in more places (BZ #15722)") (cherry picked from commit f957f47df75b9fab995754011491edebc6feb147) --- NEWS | 2 ++ sysdeps/pthread/sem_open.c | 10 ++++------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index f117874e34..5ac488bf9b 100644 --- a/NEWS +++ b/NEWS @@ -32,6 +32,8 @@ Security related changes: The following bugs are resolved with this release: [30723] posix_memalign repeatedly scans long bin lists + [30789] sem_open will fail on multithreaded scenarios when semaphore + file doesn't exist (O_CREAT) [30804] F_GETLK, F_SETLK, and F_SETLKW value change for powerpc64 with -D_FILE_OFFSET_BITS=64 [30842] Stack read overflow in getaddrinfo in no-aaaa mode (CVE-2023-4527) diff --git a/sysdeps/pthread/sem_open.c b/sysdeps/pthread/sem_open.c index e5db929d20..0e331a7445 100644 --- a/sysdeps/pthread/sem_open.c +++ b/sysdeps/pthread/sem_open.c @@ -32,11 +32,12 @@ # define __unlink unlink #endif +#define SEM_OPEN_FLAGS (O_RDWR | O_NOFOLLOW | O_CLOEXEC) + sem_t * __sem_open (const char *name, int oflag, ...) { int fd; - int open_flags; sem_t *result; /* Check that shared futexes are supported. */ @@ -65,10 +66,8 @@ __sem_open (const char *name, int oflag, ...) /* If the semaphore object has to exist simply open it. */ if ((oflag & O_CREAT) == 0 || (oflag & O_EXCL) == 0) { - open_flags = O_RDWR | O_NOFOLLOW | O_CLOEXEC; - open_flags |= (oflag & ~(O_CREAT|O_ACCMODE)); try_again: - fd = __open (dirname.name, open_flags); + fd = __open (dirname.name, (oflag & O_EXCL) | SEM_OPEN_FLAGS); if (fd == -1) { @@ -135,8 +134,7 @@ __sem_open (const char *name, int oflag, ...) } /* Open the file. Make sure we do not overwrite anything. */ - open_flags = O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC; - fd = __open (tmpfname, open_flags, mode); + fd = __open (tmpfname, O_CREAT | O_EXCL | SEM_OPEN_FLAGS, mode); if (fd == -1) { if (errno == EEXIST) -- 2.42.0