On deficient platforms where we must check for directories
ourselves when opening files, check before opening as well as after.
This prevents a hang when trying to open a special file like a
fifo in a context where a directory is required. Although there
is still a race so we could still hang in a perverse situation,
it’s the best we can do and it is better than hanging in the
more-common case.
* lib/open.c (lstatif): New static function.
(open) [REPLACE_FCHDIR]: Also inspect O_CREAT.
* lib/open.c (open), lib/openat.c (rpl_openat):
When checking for directories, also do this before opening.
Also, respect O_NOFOLLOW when checking for directories.
* lib/openat.c: Remove a few more unnecessary differences from open.c.
* modules/open (Depends-on): Depend on lstat.
* modules/openat (Depends-on): Add fstatat.
* modules/open-tests, modules/openat-tests:
(configure.ac) Check for alarm decl.
* tests/test-open.c, tests/test-openat.c:
Include sys/stat.h, for mkfifo.
[HAVE_DECL_ALARM]: Include signal.h, for alarm.
* tests/test-open.h (test_open): Fail if test takes too long
because we tried to open a fifo. Test opening /dev/null,
/dev/tty and a fifo, with a trailing "/" and with O_DIRECTORY.
---
ChangeLog | 27 +++++++++++++++
lib/open.c | 81 +++++++++++++++++++++++++++++---------------
lib/openat.c | 65 +++++++++++++++++++++++------------
modules/open | 1 +
modules/open-tests | 2 ++
modules/openat | 1 +
modules/openat-tests | 2 ++
tests/test-open.c | 5 +++
tests/test-open.h | 39 ++++++++++++++++++++-
tests/test-openat.c | 5 +++
10 files changed, 178 insertions(+), 50 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index cb84a5856e..4e26800c33 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,30 @@
+2025-05-28 Paul Eggert <egg...@cs.ucla.edu>
+
+ open, openat: handle O_DIRECTORY on special files
+ On deficient platforms where we must check for directories
+ ourselves when opening files, check before opening as well as after.
+ This prevents a hang when trying to open a special file like a
+ fifo in a context where a directory is required. Although there
+ is still a race so we could still hang in a perverse situation,
+ it’s the best we can do and it is better than hanging in the
+ more-common case.
+ * lib/open.c (lstatif): New static function.
+ (open) [REPLACE_FCHDIR]: Also inspect O_CREAT.
+ * lib/open.c (open), lib/openat.c (rpl_openat):
+ When checking for directories, also do this before opening.
+ Also, respect O_NOFOLLOW when checking for directories.
+ * lib/openat.c: Remove a few more unnecessary differences from open.c.
+ * modules/open (Depends-on): Depend on lstat.
+ * modules/openat (Depends-on): Add fstatat.
+ * modules/open-tests, modules/openat-tests:
+ (configure.ac) Check for alarm decl.
+ * tests/test-open.c, tests/test-openat.c:
+ Include sys/stat.h, for mkfifo.
+ [HAVE_DECL_ALARM]: Include signal.h, for alarm.
+ * tests/test-open.h (test_open): Fail if test takes too long
+ because we tried to open a fifo. Test opening /dev/null,
+ /dev/tty and a fifo, with a trailing "/" and with O_DIRECTORY.
+
2025-05-28 Collin Funk <collin.fu...@gmail.com>
vasnprintf: Fix uninitialized values.
diff --git a/lib/open.c b/lib/open.c
index 0d30df961a..624572c8dd 100644
--- a/lib/open.c
+++ b/lib/open.c
@@ -63,6 +63,12 @@ orig_open (const char *filename, int flags, mode_t mode)
# define REPLACE_OPEN_DIRECTORY false
#endif
+static int
+lstatif (char const *filename, struct stat *st, int flags)
+{
+ return flags & O_NOFOLLOW ? lstat (filename, st) : stat (filename, st);
+}
+
int
open (const char *filename, int flags, ...)
{
@@ -112,15 +118,42 @@ open (const char *filename, int flags, ...)
directories,
- if O_WRONLY or O_RDWR is specified, open() must fail because the
file does not contain a '.' directory. */
- if (OPEN_TRAILING_SLASH_BUG
+ bool check_for_slash_bug;
+ if (OPEN_TRAILING_SLASH_BUG)
+ {
+ size_t len = strlen (filename);
+ check_for_slash_bug = len && filename[len - 1] == '/';
+ }
+ else
+ check_for_slash_bug = false;
+
+ if (check_for_slash_bug
&& (flags & O_CREAT
|| (flags & O_ACCMODE) == O_RDWR
|| (flags & O_ACCMODE) == O_WRONLY))
{
- size_t len = strlen (filename);
- if (len > 0 && filename[len - 1] == '/')
+ errno = EISDIR;
+ return -1;
+ }
+
+ /* With the trailing slash bug or without working O_DIRECTORY, check with
+ stat first lest we hang trying to open a fifo. Although there is
+ a race between this and opening the file, we can do no better.
+ After opening the file we will check again with fstat. */
+ bool check_directory =
+ (check_for_slash_bug
+ || (!HAVE_WORKING_O_DIRECTORY && flags & O_DIRECTORY));
+ if (check_directory)
+ {
+ struct stat statbuf;
+ if (lstatif (filename, &statbuf, flags) < 0)
+ {
+ if (! (flags & O_CREAT && errno == ENOENT))
+ return -1;
+ }
+ else if (!S_ISDIR (statbuf.st_mode))
{
- errno = EISDIR;
+ errno = ENOTDIR;
return -1;
}
}
@@ -155,16 +188,18 @@ open (const char *filename, int flags, ...)
#if REPLACE_FCHDIR
/* Implementing fchdir and fdopendir requires the ability to open a
directory file descriptor. If open doesn't support that (as on
- mingw), we use a dummy file that behaves the same as directories
+ mingw), use a dummy file that behaves the same as directories
on Linux (ie. always reports EOF on attempts to read()), and
- override fstat() in fchdir.c to hide the fact that we have a
- dummy. */
+ override fstat in fchdir.c to hide the dummy. */
if (REPLACE_OPEN_DIRECTORY && fd < 0 && errno == EACCES
- && ((flags & O_ACCMODE) == O_RDONLY
- || (O_SEARCH != O_RDONLY && (flags & O_ACCMODE) == O_SEARCH)))
+ && ((flags & (O_ACCMODE | O_CREAT)) == O_RDONLY
+ || (O_SEARCH != O_RDONLY
+ && (flags & (O_ACCMODE | O_CREAT)) == O_SEARCH))
{
struct stat statbuf;
- if (stat (filename, &statbuf) == 0 && S_ISDIR (statbuf.st_mode))
+ if (check_directory
+ || (lstatif (filename, &statbuf, flags) == 0
+ && S_ISDIR (statbuf.st_mode)))
{
/* Maximum recursion depth of 1. */
fd = open ("/dev/null", flags, mode);
@@ -176,8 +211,7 @@ open (const char *filename, int flags, ...)
}
#endif
- /* If the filename ends in a slash or O_DIRECTORY is given,
- then fail if fd does not refer to a directory.
+ /* If checking for directories, fail if fd does not refer to a directory.
Rationale: A filename ending in slash cannot name a non-directory
<https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13>:
"A pathname that contains at least one non-<slash> character and that
@@ -186,23 +220,16 @@ open (const char *filename, int flags, ...)
<slash> characters names an existing directory"
If the named file without the slash is not a directory, open() must fail
with ENOTDIR. */
- if (((!HAVE_WORKING_O_DIRECTORY && flags & O_DIRECTORY)
- || OPEN_TRAILING_SLASH_BUG)
- && 0 <= fd)
+ if (check_directory && 0 <= fd)
{
- /* FILENAME must be nonempty, as open did not fail with ENOENT. */
- if ((!HAVE_WORKING_O_DIRECTORY && flags & O_DIRECTORY)
- || filename[strlen (filename) - 1] == '/')
+ struct stat statbuf;
+ int r = fstat (fd, &statbuf);
+ if (r < 0 || !S_ISDIR (statbuf.st_mode))
{
- struct stat statbuf;
- int r = fstat (fd, &statbuf);
- if (r < 0 || !S_ISDIR (statbuf.st_mode))
- {
- int err = r < 0 ? errno : ENOTDIR;
- close (fd);
- errno = err;
- return -1;
- }
+ int err = r < 0 ? errno : ENOTDIR;
+ close (fd);
+ errno = err;
+ return -1;
}
}
diff --git a/lib/openat.c b/lib/openat.c
index 66b53264a2..e7560259d7 100644
--- a/lib/openat.c
+++ b/lib/openat.c
@@ -35,6 +35,7 @@ orig_openat (int fd, char const *filename, int flags, mode_t
mode)
}
#endif
+/* Specification. */
#ifdef __osf__
/* Write "fcntl.h" here, not <fcntl.h>, otherwise OSF/1 5.1 DTK cc eliminates
this include because of the preliminary #include <fcntl.h> above. */
@@ -47,12 +48,12 @@ orig_openat (int fd, char const *filename, int flags,
mode_t mode)
#include "cloexec.h"
+#include <errno.h>
#include <stdarg.h>
#include <stddef.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
-#include <errno.h>
#ifndef OPEN_TRAILING_SLASH_BUG
# define OPEN_TRAILING_SLASH_BUG false
@@ -97,15 +98,43 @@ rpl_openat (int dfd, char const *filename, int flags, ...)
directories,
- if O_WRONLY or O_RDWR is specified, open() must fail because the
file does not contain a '.' directory. */
- if (OPEN_TRAILING_SLASH_BUG
+ bool check_for_slash_bug;
+ if (OPEN_TRAILING_SLASH_BUG)
+ {
+ size_t len = strlen (filename);
+ check_for_slash_bug = len && filename[len - 1] == '/';
+ }
+ else
+ check_for_slash_bug = false;
+
+ if (check_for_slash_bug
&& (flags & O_CREAT
|| (flags & O_ACCMODE) == O_RDWR
|| (flags & O_ACCMODE) == O_WRONLY))
{
- size_t len = strlen (filename);
- if (len > 0 && filename[len - 1] == '/')
+ errno = EISDIR;
+ return -1;
+ }
+
+ /* With the trailing slash bug or without working O_DIRECTORY, check with
+ stat first lest we hang trying to open a fifo. Although there is
+ a race between this and opening the file, we can do no better.
+ After opening the file we will check again with fstat. */
+ bool check_directory =
+ (check_for_slash_bug
+ || (!HAVE_WORKING_O_DIRECTORY && flags & O_DIRECTORY));
+ if (check_directory)
+ {
+ struct stat statbuf;
+ int fstatat_flags = flags & O_NOFOLLOW ? AT_SYMLINK_NOFOLLOW : 0;
+ if (fstatat (dfd, filename, &statbuf, fstatat_flags) < 0)
+ {
+ if (! (flags & O_CREAT && errno == ENOENT))
+ return -1;
+ }
+ else if (!S_ISDIR (statbuf.st_mode))
{
- errno = EISDIR;
+ errno = ENOTDIR;
return -1;
}
}
@@ -137,8 +166,7 @@ rpl_openat (int dfd, char const *filename, int flags, ...)
}
- /* If the filename ends in a slash or O_DIRECTORY is given,
- then fail if fd does not refer to a directory.
+ /* If checking for directories, fail if fd does not refer to a directory.
Rationale: A filename ending in slash cannot name a non-directory
<https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13>:
"A pathname that contains at least one non-<slash> character and that
@@ -147,23 +175,16 @@ rpl_openat (int dfd, char const *filename, int flags, ...)
<slash> characters names an existing directory"
If the named file without the slash is not a directory, open() must fail
with ENOTDIR. */
- if (((!HAVE_WORKING_O_DIRECTORY && flags & O_DIRECTORY)
- || OPEN_TRAILING_SLASH_BUG)
- && 0 <= fd)
+ if (check_directory && 0 <= fd)
{
- /* FILENAME must be nonempty, as open did not fail with ENOENT. */
- if ((!HAVE_WORKING_O_DIRECTORY && flags & O_DIRECTORY)
- || filename[strlen (filename) - 1] == '/')
+ struct stat statbuf;
+ int r = fstat (fd, &statbuf);
+ if (r < 0 || !S_ISDIR (statbuf.st_mode))
{
- struct stat statbuf;
- int r = fstat (fd, &statbuf);
- if (r < 0 || !S_ISDIR (statbuf.st_mode))
- {
- int err = r < 0 ? errno : ENOTDIR;
- close (fd);
- errno = err;
- return -1;
- }
+ int err = r < 0 ? errno : ENOTDIR;
+ close (fd);
+ errno = err;
+ return -1;
}
}
diff --git a/modules/open b/modules/open
index f322b74794..62b711f681 100644
--- a/modules/open
+++ b/modules/open
@@ -13,6 +13,7 @@ fcntl-h
largefile
cloexec [test $REPLACE_OPEN = 1]
fstat [test $REPLACE_OPEN = 1]
+lstat [test $REPLACE_OPEN = 1]
stat [test $REPLACE_OPEN = 1]
configure.ac:
diff --git a/modules/open-tests b/modules/open-tests
index 06420236c1..0d1be8bb28 100644
--- a/modules/open-tests
+++ b/modules/open-tests
@@ -7,9 +7,11 @@ tests/macros.h
Depends-on:
bool
fcntl
+mkfifo
symlink
configure.ac:
+AC_CHECK_DECLS_ONCE([alarm])
Makefile.am:
TESTS += test-open
diff --git a/modules/openat b/modules/openat
index 16d91a13a4..72ff45a7aa 100644
--- a/modules/openat
+++ b/modules/openat
@@ -17,6 +17,7 @@ bool [test $HAVE_OPENAT = 0 || test
$REPLACE_OPENAT = 1]
sys_stat-h [test $HAVE_OPENAT = 0 || test $REPLACE_OPENAT = 1]
cloexec [test $REPLACE_OPENAT = 1]
fstat [test $REPLACE_OPENAT = 1]
+fstatat [test $REPLACE_OPENAT = 1]
at-internal [test $HAVE_OPENAT = 0]
errno-h [test $HAVE_OPENAT = 0]
fchdir [test $HAVE_OPENAT = 0]
diff --git a/modules/openat-tests b/modules/openat-tests
index c4a72f5fb8..1863ecfade 100644
--- a/modules/openat-tests
+++ b/modules/openat-tests
@@ -6,9 +6,11 @@ tests/macros.h
Depends-on:
fcntl
+mkfifo
symlink
configure.ac:
+AC_CHECK_DECLS_ONCE([alarm])
Makefile.am:
TESTS += test-openat
diff --git a/tests/test-open.c b/tests/test-open.c
index f47b4609e8..84df160f2c 100644
--- a/tests/test-open.c
+++ b/tests/test-open.c
@@ -25,8 +25,13 @@ SIGNATURE_CHECK (open, int, (char const *, int, ...));
#include <errno.h>
#include <stdio.h>
+#include <sys/stat.h>
#include <unistd.h>
+#if HAVE_DECL_ALARM
+# include <signal.h>
+#endif
+
#include "macros.h"
#define BASE "test-open.t"
diff --git a/tests/test-open.h b/tests/test-open.h
index 36823a4cbe..b745a2b08d 100644
--- a/tests/test-open.h
+++ b/tests/test-open.h
@@ -41,9 +41,18 @@
static ALWAYS_INLINE int
test_open (int (*func) (char const *, int, ...), bool print)
{
+#if HAVE_DECL_ALARM
+ /* Declare failure if test takes too long, by using default abort
+ caused by SIGALRM. */
+ int alarm_value = 5;
+ signal (SIGALRM, SIG_DFL);
+ alarm (alarm_value);
+#endif
+
int fd;
/* Remove anything from prior partial run. */
+ unlink (BASE "fifo");
unlink (BASE "file");
unlink (BASE "e.exe");
unlink (BASE "link");
@@ -69,11 +78,39 @@ test_open (int (*func) (char const *, int, ...), bool print)
ASSERT (func (BASE "file/", O_RDONLY) == -1);
ASSERT (errno == ENOTDIR || errno == EISDIR || errno == EINVAL);
- /* Cannot open non-directory with O_DIRECTORY. */
+ /* Cannot open regular file with O_DIRECTORY. */
errno = 0;
ASSERT (func (BASE "file", O_RDONLY | O_DIRECTORY) == -1);
ASSERT (errno == ENOTDIR);
+ /* Cannot open /dev/null with trailing slash or O_DIRECTORY. */
+ errno = 0;
+ ASSERT (func ("/dev/null/", O_RDONLY) == -1);
+ ASSERT (errno == ENOTDIR || errno == EISDIR || errno == EINVAL);
+
+ errno = 0;
+ ASSERT (func ("/dev/null", O_RDONLY | O_DIRECTORY) == -1);
+ ASSERT (errno == ENOTDIR);
+
+ /* Cannot open /dev/tty with trailing slash or O_DIRECTORY,
+ though errno may differ as there may not be a controlling tty. */
+ ASSERT (func ("/dev/tty/", O_RDONLY) == -1);
+ ASSERT (func ("/dev/tty", O_RDONLY | O_DIRECTORY) == -1);
+
+ /* Cannot open fifo with trailing slash or O_DIRECTORY. */
+ if (mkfifo (BASE "fifo", 0666) == 0)
+ {
+ errno = 0;
+ ASSERT (func (BASE "fifo/", O_RDONLY) == -1);
+ ASSERT (errno == ENOTDIR || errno == EISDIR || errno == EINVAL);
+
+ errno = 0;
+ ASSERT (func (BASE "fifo", O_RDONLY | O_DIRECTORY) == -1);
+ ASSERT (errno == ENOTDIR);
+
+ ASSERT (unlink (BASE "fifo") == 0);
+ }
+
/* Directories cannot be opened for writing. */
errno = 0;
ASSERT (func (".", O_WRONLY) == -1);
diff --git a/tests/test-openat.c b/tests/test-openat.c
index 8a83913a42..1405f4f7a5 100644
--- a/tests/test-openat.c
+++ b/tests/test-openat.c
@@ -26,8 +26,13 @@ SIGNATURE_CHECK (openat, int, (int, char const *, int, ...));
#include <errno.h>
#include <stdarg.h>
#include <stdio.h>
+#include <sys/stat.h>
#include <unistd.h>
+#if HAVE_DECL_ALARM
+# include <signal.h>
+#endif
+
#include "macros.h"
#define BASE "test-openat.t"
--
2.48.1
