fastpath: Only allow IO to approved file types and file systems

CHANGELOG.md

@@ -15,6 +15,7 @@
 * The AIS optimized IO path will automatically fallback to the POSIX IO path if a failure occurs and the compatibility mode has not been disabled.
 * Added check in the Fastpath/AIS backend to ensure the HIP Runtime is initialized. This avoids causing a segfault in the HIP Runtime.
 * The default CMake build type was changed from `Debug` to `RelWithDebInfo`
+* Added file type and file system validation in Fastpath. Fastpath will only accept IO targeting block devices or regular files backed by xfs or ext4 with ordered journaling mode. Other file systems can be explicitly allowed via the `HIPFILE_UNSUPPORTED_FILE_SYSTEMS` environment variable.
 
 ### Removed
 * The rocFile library has been completely removed and the code is now a part of hipFile.

src/amd_detail/backend/fallback.cpp

@@ -97,7 +97,7 @@ Fallback::_io_impl(IoType type, std::shared_ptr<IFile> file, std::shared_ptr<IBu
             switch (type) {
                 case IoType::Read:
                     io_bytes =
-                        Context<Sys>::get()->pread(file->getBufferedFd(), bounce_buffer.get(), count, offset);
+                        Context<Sys>::get()->pread(file->bufferedFd(), bounce_buffer.get(), count, offset);
                     if (io_bytes > 0) {
                         Context<Hip>::get()->hipMemcpy(device_buffer_position, bounce_buffer.get(),
                                                        static_cast<size_t>(io_bytes), hipMemcpyHostToDevice);
@@ -107,9 +107,9 @@ Fallback::_io_impl(IoType type, std::shared_ptr<IFile> file, std::shared_ptr<IBu
                     Context<Hip>::get()->hipMemcpy(bounce_buffer.get(), device_buffer_position, count,
                                                    hipMemcpyDeviceToHost);
                     Context<Hip>::get()->hipStreamSynchronize(nullptr);
-                    io_bytes = Context<Sys>::get()->pwrite(file->getBufferedFd(), bounce_buffer.get(), count,
-                                                           offset);
-                    Context<Sys>::get()->fdatasync(file->getBufferedFd());
+                    io_bytes =
+                        Context<Sys>::get()->pwrite(file->bufferedFd(), bounce_buffer.get(), count, offset);
+                    Context<Sys>::get()->fdatasync(file->bufferedFd());
                     break;
                 default:
                     throw std::runtime_error("Invalid IO type");
@@ -274,11 +274,11 @@ async_io_cpu_copy(void *userargs)
         try {
             switch (op->io_type) {
                 case IoType::Read:
-                    ret = Context<Sys>::get()->pread(op->file->getBufferedFd(), cur_buf_position,
+                    ret = Context<Sys>::get()->pread(op->file->bufferedFd(), cur_buf_position,
                                                      remaining_bytes, cur_file_offset);
                     break;
                 case IoType::Write:
-                    ret = Context<Sys>::get()->pwrite(op->file->getBufferedFd(), cur_buf_position,
+                    ret = Context<Sys>::get()->pwrite(op->file->bufferedFd(), cur_buf_position,
                                                       remaining_bytes, cur_file_offset);
                     break;
                 default:

src/amd_detail/backend/fastpath.cpp

@@ -136,28 +136,28 @@ Fastpath::score(shared_ptr<IFile> file, shared_ptr<IBuffer> buffer, size_t size,
 
     accept_io &= Context<Configuration>::get()->fastpath();
 
-    accept_io &= file->getUnbufferedFd().has_value();
+    accept_io &= file->unbufferedFd().has_value();
 
     accept_io &= buffer->getType() == hipMemoryTypeDevice;
 
     accept_io &= 0 <= file_offset;
     accept_io &= 0 <= buffer_offset;
 
-    uint64_t mem_align_mask{4096 - 1};
-    uint64_t offset_align_mask{4096 - 1};
+    bool is_regular_file{file->isRegularFile()};
+    bool is_block_device{file->isBlockDevice()};
+    bool on_ext4_ordered{file->onExt4Ordered()};
+    bool on_xfs{file->onXfs()};
+    bool unsupported_file_systems{Context<Configuration>::get()->unsupportedFileSystems()};
+    accept_io &=
+        is_block_device || (is_regular_file && (unsupported_file_systems || on_ext4_ordered || on_xfs));
 
-#if defined(STATX_DIOALIGN)
-    const struct statx &stx{file->getStatx()};
-    accept_io &= !!(stx.stx_mask & STATX_DIOALIGN);
-    accept_io &= stx.stx_dio_offset_align && stx.stx_dio_mem_align;
-    mem_align_mask    = stx.stx_dio_mem_align - 1;
-    offset_align_mask = stx.stx_dio_offset_align - 1;
-#endif
+    const uint32_t dio_offset_align{file->dioOffsetAlign()};
+    accept_io &= dio_offset_align && !(file_offset & (dio_offset_align - 1));
+    accept_io &= dio_offset_align && !(size & (dio_offset_align - 1));
 
-    accept_io &= !(size & offset_align_mask);
-    accept_io &= !(file_offset & static_cast<int64_t>(offset_align_mask));
-    auto buffer_address{reinterpret_cast<intptr_t>(buffer->getBuffer())};
-    accept_io &= !((buffer_address + buffer_offset) & static_cast<int64_t>(mem_align_mask));
+    const uint32_t dio_mem_align{file->dioMemAlign()};
+    const auto     mem_addr{reinterpret_cast<intptr_t>(buffer->getBuffer()) + buffer_offset};
+    accept_io &= dio_mem_align && !(mem_addr & (dio_mem_align - 1));
 
     return accept_io ? 100 : -1;
 }
@@ -174,7 +174,7 @@ Fastpath::_io_impl(IoType type, shared_ptr<IFile> file, shared_ptr<IBuffer> buff
     hipAmdFileHandle_t handle{};
     size_t             nbytes{};
 
-    handle.fd = file->getUnbufferedFd().value();
+    handle.fd = file->unbufferedFd().value();
 
     if (!paramsValid(buffer, size, file_offset, buffer_offset)) {
         throw std::invalid_argument("The selected file or buffer region is invalid");

src/amd_detail/batch/batch.cpp

@@ -56,7 +56,7 @@ BatchOperation::BatchOperation(std::unique_ptr<const hipFileIOParams_t> params,
     }
 
     // Check File parameters
-    if (io_params->fh != file->getHandle()) {
+    if (io_params->fh != file->handle()) {
         throw std::invalid_argument("File does not match handle specified in io_params.");
     }
     if (io_params->u.batch.file_offset < 0) {

src/amd_detail/configuration.cpp

@@ -46,3 +46,10 @@ Configuration::statsLevel() const noexcept
     HIPFILE_STATIC unsigned int stats_level_env{Environment::stats_level().value_or(0)};
     return stats_level_env;
 }
+
+bool
+Configuration::unsupportedFileSystems() const noexcept
+{
+    HIPFILE_STATIC bool unsupported_file_systems_env{Environment::unsupported_file_systems().value_or(false)};
+    return unsupported_file_systems_env;
+}

src/amd_detail/configuration.h

@@ -37,6 +37,11 @@ class Configuration {
     /// @brief Shows the level of detail for stats collection
     /// @return 0 if stats collection disabled, higher levels of detail as value increases
     virtual unsigned int statsLevel() const noexcept;
+
+    /// @brief Checks if unsupported file systems are allowed in the fastpath backend
+    /// @return true if unsupported file systems are allowed, false if only supported file systems are
+    /// permitted (default)
+    virtual bool unsupportedFileSystems() const noexcept;
 };
 
 }

src/amd_detail/environment.cpp

@@ -64,6 +64,12 @@ Environment::force_compat_mode()
     return Environment::get<bool>(Environment::FORCE_COMPAT_MODE);
 }
 
+optional<bool>
+Environment::unsupported_file_systems()
+{
+    return Environment::get<bool>(Environment::UNSUPPORTED_FILE_SYSTEMS);
+}
+
 optional<unsigned int>
 Environment::stats_level()
 {

src/amd_detail/environment.h

@@ -47,6 +47,19 @@ class Environment {
     static constexpr const char *const STATS_LEVEL{"HIPFILE_STATS_LEVEL"};
 
     static std::optional<unsigned int> stats_level();
+
+    /// @brief Allow unsupported file systems in the fastpath backend
+    ///
+    /// If enabled, the fastpath backend will allow I/O on file systems other than
+    /// ext4 (with ordered journaling) and xfs. If disabled (default), only supported
+    /// file systems are permitted.
+    static constexpr const char *const UNSUPPORTED_FILE_SYSTEMS{"HIPFILE_UNSUPPORTED_FILE_SYSTEMS"};
+
+    /// @brief Get the value of HIPFILE_UNSUPPORTED_FILE_SYSTEMS from the environment
+    /// @return An optional boolean value if HIPFILE_UNSUPPORTED_FILE_SYSTEMS was set,
+    /// nullopt if HIPFILE_UNSUPPORTED_FILE_SYSTEMS was unset or had a value other than
+    /// true or false.
+    static std::optional<bool> unsupported_file_systems();
 };
 
 }

src/amd_detail/file.cpp

@@ -16,6 +16,7 @@
 #include <sys/sysmacros.h>
 #include <syslog.h>
 #include <system_error>
+#include <unistd.h>
 
 using namespace std;
 
@@ -31,7 +32,13 @@ UnregisteredFile::UnregisteredFile(int fd)
 #endif
                                      )},
       flags{Context<Sys>::get()->fcntl(fd, F_GETFL, 0)},
-      mountinfo{Context<LibMountHelper>::get()->getMountInfo(makedev(stx.stx_dev_major, stx.stx_dev_minor))}
+      mountinfo{Context<LibMountHelper>::get()->getMountInfo(makedev(stx.stx_dev_major, stx.stx_dev_minor))},
+#if defined(STATX_DIOALIGN)
+      m_dio_mem_align{stx.stx_mask & STATX_DIOALIGN ? stx.stx_dio_mem_align : 4096},
+      m_dio_offset_align{stx.stx_mask & STATX_DIOALIGN ? stx.stx_dio_offset_align : 4096}
+#else
+      m_dio_mem_align{4096}, m_dio_offset_align{4096}
+#endif
 {
     std::string path = "/proc/self/fd/" + std::to_string(fd);
 
@@ -50,60 +57,86 @@ UnregisteredFile::UnregisteredFile(int fd)
             if (e.code().value() != EINVAL) {
                 throw;
             }
-            unbuffered_fd = nullopt;
+            unbuffered_fd      = nullopt;
+            m_dio_mem_align    = 0;
+            m_dio_offset_align = 0;
         }
     }
 }
 
 hipFileHandle_t
-IFile::getHandle() const
+IFile::handle() const noexcept
 {
     return reinterpret_cast<hipFileHandle_t>(const_cast<IFile *>(this));
 }
 
 File::File(UnregisteredFile &&uf, const PassKey<FileMap> &)
-    : client_fd{std::move(uf.client_fd)}, buffered_fd{std::move(uf.buffered_fd)},
-      unbuffered_fd{std::move(uf.unbuffered_fd)}, stx{uf.stx}, status_flags{uf.flags}, mountinfo{uf.mountinfo}
+    : m_client_fd{std::move(uf.client_fd)}, m_buffered_fd{std::move(uf.buffered_fd)},
+      m_unbuffered_fd{std::move(uf.unbuffered_fd)}, m_dio_mem_align{uf.m_dio_mem_align},
+      m_dio_offset_align{uf.m_dio_offset_align},
+      m_is_block_device{(uf.stx.stx_mask & STATX_TYPE) && S_ISBLK(uf.stx.stx_mode)},
+      m_is_regular_file{(uf.stx.stx_mask & STATX_TYPE) && S_ISREG(uf.stx.stx_mode)},
+      m_on_ext4_ordered{uf.mountinfo && uf.mountinfo->type == FilesystemType::ext4 &&
+                        uf.mountinfo->options.ext4.journaling_mode == ExtJournalingMode::ordered},
+      m_on_xfs{uf.mountinfo && uf.mountinfo->type == FilesystemType::xfs}
 {
 }
 
 int
-File::getClientFd() const
+File::clientFd() const noexcept
 {
-    return client_fd.get();
+    return m_client_fd.get();
 }
 
 int
-File::getBufferedFd() const
+File::bufferedFd() const noexcept
 {
-    return buffered_fd.get();
+    return m_buffered_fd.get();
 }
 
 optional<int>
-File::getUnbufferedFd() const
+File::unbufferedFd() const noexcept
 {
-    if (unbuffered_fd) {
-        return unbuffered_fd.value().get();
+    if (m_unbuffered_fd) {
+        return m_unbuffered_fd.value().get();
     }
     return nullopt;
 }
 
-const struct statx &
-File::getStatx() const noexcept
+uint32_t
+File::dioMemAlign() const noexcept
 {
-    return stx;
+    return m_dio_mem_align;
 }
 
-int
-File::getStatusFlags() const
+uint32_t
+File::dioOffsetAlign() const noexcept
+{
+    return m_dio_offset_align;
+}
+
+bool
+File::isBlockDevice() const noexcept
+{
+    return m_is_block_device;
+}
+
+bool
+File::isRegularFile() const noexcept
+{
+    return m_is_regular_file;
+}
+
+bool
+File::onExt4Ordered() const noexcept
 {
-    return status_flags;
+    return m_on_ext4_ordered;
 }
 
-optional<MountInfo>
-File::getMountInfo() const
+bool
+File::onXfs() const noexcept
 {
-    return mountinfo;
+    return m_on_xfs;
 }
 
 shared_ptr<IFile>
@@ -124,11 +157,11 @@ FileMap::registerFile(UnregisteredFile &&uf)
         throw FileAlreadyRegistered();
     }
 
-    auto file                    = std::shared_ptr<IFile>(new File(std::move(uf), PassKey<FileMap>{}));
-    from_fd[file->getClientFd()] = file;
-    from_fh[file->getHandle()]   = file;
+    auto file                 = std::shared_ptr<IFile>(new File(std::move(uf), PassKey<FileMap>{}));
+    from_fd[file->clientFd()] = file;
+    from_fh[file->handle()]   = file;
 
-    return file->getHandle();
+    return file->handle();
 }
 
 void
@@ -144,7 +177,7 @@ FileMap::deregisterFile(hipFileHandle_t fh)
         throw FileOperationsOutstanding();
     }
 
-    from_fd.erase(itr->second->getClientFd());
+    from_fd.erase(itr->second->clientFd());
     from_fh.erase(fh);
 }