From 63c9f01d507e99b39bf663302de103e508fb3b78 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 19 Nov 2024 11:08:29 +0100 Subject: [PATCH] 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. --- src/io/Beneath.cxx | 4 ++-- src/io/FileDescriptor.cxx | 27 ++++++++++----------------- src/io/FileDescriptor.hxx | 7 ------- src/io/StateDirectories.cxx | 2 +- src/io/uring/CoOperation.cxx | 2 +- src/io/uring/Open.cxx | 4 ++-- src/io/uring/OpenStat.cxx | 4 ++-- 7 files changed, 18 insertions(+), 32 deletions(-) diff --git a/src/io/Beneath.cxx b/src/io/Beneath.cxx index 574856366..d8f1f4c01 100644 --- a/src/io/Beneath.cxx +++ b/src/io/Beneath.cxx @@ -12,7 +12,7 @@ #include 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 index 031579cc4..bd900d3f0 100644 --- 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 index 6bb9c45c3..48f5d6359 100644 --- 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 index 93f147f45..cfcdaee83 100644 --- 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 index e4d49ff98..1c50d88dd 100644 --- 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 diff --git a/src/io/uring/Open.cxx b/src/io/uring/Open.cxx index 75fd1e09e..8939e7512 100644 --- a/src/io/uring/Open.cxx +++ b/src/io/uring/Open.cxx @@ -15,7 +15,7 @@ #include // 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 index 673efc55c..9ae53e134 100644 --- a/src/io/uring/OpenStat.cxx +++ b/src/io/uring/OpenStat.cxx @@ -16,7 +16,7 @@ #include // 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); }