io: always pass O_NONBLOCK to open()

Opening a FIFO may block indefinitely (until a writer connects). This is dangerous because it may be a DoS vulnerability in many programs that do not expect open() to block. This obsoletes the method FileDescriptor::OpenNonBlocking() which wasn't used anyway.
CM4all · Nov 19, 2024 · 63c9f01 · 63c9f01
1 parent b3c90ae
commit 63c9f01
Show file tree

Hide file tree

Showing 7 changed files with 18 additions and 32 deletions.
diff --git a/src/io/Beneath.cxx b/src/io/Beneath.cxx
@@ -12,7 +12,7 @@
 #include <fcntl.h>
 
 static constexpr struct open_how ro_beneath{
-	.flags = O_RDONLY|O_NOCTTY|O_CLOEXEC,
+	.flags = O_RDONLY|O_NOCTTY|O_CLOEXEC|O_NONBLOCK,
 	.resolve = RESOLVE_BENEATH|RESOLVE_NO_MAGICLINKS,
 };
 
@@ -37,7 +37,7 @@ OpenReadOnlyBeneath(FileAt file)
 }
 
 static constexpr struct open_how directory_beneath{
-	.flags = O_DIRECTORY|O_RDONLY|O_NOCTTY|O_CLOEXEC,
+	.flags = O_DIRECTORY|O_RDONLY|O_NOCTTY|O_CLOEXEC|O_NONBLOCK,
 	.resolve = RESOLVE_BENEATH|RESOLVE_NO_MAGICLINKS,
 };
 

diff --git a/src/io/FileDescriptor.cxx b/src/io/FileDescriptor.cxx
@@ -24,6 +24,13 @@
 #define O_CLOEXEC 0
 #endif
 
+/* this library implies the O_NONBLOCK in all open() calls to avoid
+   blocking the caller when a FIFO is opened; this may not only affect
+   the open() call but also other operations like mandatory locking */
+#ifndef O_NONBLOCK
+#define O_NONBLOCK 0
+#endif
+
 #ifndef _WIN32
 
 bool
@@ -61,7 +68,7 @@ bool
 FileDescriptor::Open(FileDescriptor dir, const char *pathname,
 		     int flags, mode_t mode) noexcept
 {
-	fd = ::openat(dir.Get(), pathname, flags | O_NOCTTY | O_CLOEXEC, mode);
+	fd = ::openat(dir.Get(), pathname, flags | O_NOCTTY | O_CLOEXEC | O_NONBLOCK, mode);
 	return IsDefined();
 }
 
@@ -70,7 +77,7 @@ FileDescriptor::Open(FileDescriptor dir, const char *pathname,
 bool
 FileDescriptor::Open(const char *pathname, int flags, mode_t mode) noexcept
 {
-	fd = ::open(pathname, flags | O_NOCTTY | O_CLOEXEC, mode);
+	fd = ::open(pathname, flags | O_NOCTTY | O_CLOEXEC | O_NONBLOCK, mode);
 	return IsDefined();
 }
 
@@ -79,7 +86,7 @@ FileDescriptor::Open(const char *pathname, int flags, mode_t mode) noexcept
 bool
 FileDescriptor::Open(const wchar_t *pathname, int flags, mode_t mode) noexcept
 {
-	fd = ::_wopen(pathname, flags | O_NOCTTY | O_CLOEXEC, mode);
+	fd = ::_wopen(pathname, flags | O_NOCTTY | O_CLOEXEC | O_NONBLOCK, mode);
 	return IsDefined();
 }
 
@@ -99,20 +106,6 @@ FileDescriptor::OpenReadOnly(FileDescriptor dir, const char *pathname) noexcept
 	return Open(dir, pathname, O_RDONLY);
 }
 
-#endif // __linux__
-
-#ifndef _WIN32
-
-bool
-FileDescriptor::OpenNonBlocking(const char *pathname) noexcept
-{
-	return Open(pathname, O_RDWR | O_NONBLOCK);
-}
-
-#endif
-
-#ifdef __linux__
-
 bool
 FileDescriptor::CreatePipe(FileDescriptor &r, FileDescriptor &w,
 			   int flags) noexcept

diff --git a/src/io/FileDescriptor.hxx b/src/io/FileDescriptor.hxx
@@ -115,14 +115,7 @@ public:
 	[[nodiscard]]
 	bool OpenReadOnly(FileDescriptor dir,
 			  const char *pathname) noexcept;
-#endif
-
-#ifndef _WIN32
-	[[nodiscard]]
-	bool OpenNonBlocking(const char *pathname) noexcept;
-#endif
 
-#ifdef __linux__
 	[[nodiscard]]
 	static bool CreatePipe(FileDescriptor &r, FileDescriptor &w,
 			       int flags) noexcept;

diff --git a/src/io/StateDirectories.cxx b/src/io/StateDirectories.cxx
@@ -45,7 +45,7 @@ static FileDescriptor
 OpenReadOnlyNoFollow(FileDescriptor directory, const char *path)
 {
 	static constexpr struct open_how how{
-		.flags = O_RDONLY|O_NOFOLLOW|O_NOCTTY|O_CLOEXEC,
+		.flags = O_RDONLY|O_NOFOLLOW|O_NOCTTY|O_CLOEXEC|O_NONBLOCK,
 		.resolve = RESOLVE_IN_ROOT|RESOLVE_NO_MAGICLINKS|RESOLVE_NO_SYMLINKS,
 	};
 

diff --git a/src/io/uring/CoOperation.cxx b/src/io/uring/CoOperation.cxx
@@ -79,7 +79,7 @@ CoOpenOperation::CoOpenOperation(struct io_uring_sqe &s,
 				 int flags, mode_t mode) noexcept
 {
 	io_uring_prep_openat(&s, directory_fd.Get(), path,
-			     flags|O_NOCTTY|O_CLOEXEC, mode);
+			     flags|O_NOCTTY|O_CLOEXEC|O_NONBLOCK, mode);
 }
 
 CoOperation<CoOpenOperation>

diff --git a/src/io/uring/Open.cxx b/src/io/uring/Open.cxx
@@ -15,7 +15,7 @@
 #include <linux/openat2.h> //  for RESOLVE_*
 
 static constexpr struct open_how ro_beneath{
-	.flags = O_RDONLY|O_NOCTTY|O_CLOEXEC,
+	.flags = O_RDONLY|O_NOCTTY|O_CLOEXEC|O_NONBLOCK,
 	.resolve = RESOLVE_BENEATH|RESOLVE_NO_MAGICLINKS,
 };
 
@@ -27,7 +27,7 @@ Open::StartOpen(FileAt file, int flags, mode_t mode) noexcept
 	auto &s = queue.RequireSubmitEntry();
 
 	io_uring_prep_openat(&s, file.directory.Get(), file.name,
-			     flags|O_NOCTTY|O_CLOEXEC, mode);
+			     flags|O_NOCTTY|O_CLOEXEC|O_NONBLOCK, mode);
 	queue.Push(s, *this);
 }
 

diff --git a/src/io/uring/OpenStat.cxx b/src/io/uring/OpenStat.cxx
@@ -16,7 +16,7 @@
 #include <linux/openat2.h> //  for RESOLVE_*
 
 static constexpr struct open_how ro_beneath{
-	.flags = O_RDONLY|O_NOCTTY|O_CLOEXEC,
+	.flags = O_RDONLY|O_NOCTTY|O_CLOEXEC|O_NONBLOCK,
 	.resolve = RESOLVE_BENEATH|RESOLVE_NO_MAGICLINKS,
 };
 
@@ -31,7 +31,7 @@ OpenStat::StartOpenStat(FileAt file,
 	auto &s = queue.RequireSubmitEntry();
 
 	io_uring_prep_openat(&s, file.directory.Get(), file.name,
-			     flags|O_NOCTTY|O_CLOEXEC, mode);
+			     flags|O_NOCTTY|O_CLOEXEC|O_NONBLOCK, mode);
 	queue.Push(s, *this);
 }