From 763ef690c69cac88f489eebf2fb6873e442bb1dc Mon Sep 17 00:00:00 2001 From: Liav A Date: Fri, 30 Jun 2023 08:55:57 +0300 Subject: [PATCH] Kernel/Storage: Declare proper blocking support for StorageDevices We remove can_read() and can_write(), as both of these methods should be implemented for proper blocking support. For our case, the previous code will simply block the user if they tries to read beyond the max addressable offset, which is not a correct behavior. Instead, just do proper EOF guarding when calling read() and write() on such objects. --- Kernel/Devices/Storage/StorageDevice.cpp | 30 +++++++++++------------- Kernel/Devices/Storage/StorageDevice.h | 4 ++-- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/Kernel/Devices/Storage/StorageDevice.cpp b/Kernel/Devices/Storage/StorageDevice.cpp index 7edb89be7f..7882b6d7a6 100644 --- a/Kernel/Devices/Storage/StorageDevice.cpp +++ b/Kernel/Devices/Storage/StorageDevice.cpp @@ -85,10 +85,14 @@ StringView StorageDevice::command_set_to_string_view() const ErrorOr StorageDevice::read(OpenFileDescription&, u64 offset, UserOrKernelBuffer& outbuf, size_t len) { + // NOTE: The last available offset is actually just after the last addressable block. + if (offset >= (max_mathematical_addressable_block() * block_size())) + return 0; + size_t nread = min(static_cast((max_mathematical_addressable_block() * block_size()) - offset), len); u64 index = offset >> block_size_log(); off_t offset_within_block = 0; - size_t whole_blocks = len >> block_size_log(); - size_t remaining = len - (whole_blocks << block_size_log()); + size_t whole_blocks = nread >> block_size_log(); + size_t remaining = nread - (whole_blocks << block_size_log()); // PATAChannel will chuck a wobbly if we try to read more than PAGE_SIZE // at a time, because it uses a single page for its DMA buffer. @@ -97,7 +101,7 @@ ErrorOr StorageDevice::read(OpenFileDescription&, u64 offset, UserOrKern remaining = 0; } - if (len < block_size()) + if (nread < block_size()) offset_within_block = offset - (index << block_size_log()); dbgln_if(STORAGE_DEVICE_DEBUG, "StorageDevice::read() index={}, whole_blocks={}, remaining={}", index, whole_blocks, remaining); @@ -144,17 +148,16 @@ ErrorOr StorageDevice::read(OpenFileDescription&, u64 offset, UserOrKern return pos + remaining; } -bool StorageDevice::can_read(OpenFileDescription const&, u64 offset) const -{ - return offset < (max_addressable_block() * block_size()); -} - ErrorOr StorageDevice::write(OpenFileDescription&, u64 offset, UserOrKernelBuffer const& inbuf, size_t len) { + // NOTE: The last available offset is actually just after the last addressable block. + if (offset >= (max_mathematical_addressable_block() * block_size())) + return Error::from_errno(ENOSPC); + size_t nwrite = min(static_cast((max_mathematical_addressable_block() * block_size()) - offset), len); u64 index = offset >> block_size_log(); off_t offset_within_block = 0; - size_t whole_blocks = len >> block_size_log(); - size_t remaining = len - (whole_blocks << block_size_log()); + size_t whole_blocks = nwrite >> block_size_log(); + size_t remaining = nwrite - (whole_blocks << block_size_log()); // PATAChannel will chuck a wobbly if we try to write more than PAGE_SIZE // at a time, because it uses a single page for its DMA buffer. @@ -163,7 +166,7 @@ ErrorOr StorageDevice::write(OpenFileDescription&, u64 offset, UserOrKer remaining = 0; } - if (len < block_size()) + if (nwrite < block_size()) offset_within_block = offset - (index << block_size_log()); // We try to allocate the temporary block buffer for partial writes *before* we start any full block writes, @@ -239,11 +242,6 @@ ErrorOr StorageDevice::write(OpenFileDescription&, u64 offset, UserOrKer return pos + remaining; } -bool StorageDevice::can_write(OpenFileDescription const&, u64 offset) const -{ - return offset < (max_addressable_block() * block_size()); -} - ErrorOr StorageDevice::ioctl(OpenFileDescription&, unsigned request, Userspace arg) { switch (request) { diff --git a/Kernel/Devices/Storage/StorageDevice.h b/Kernel/Devices/Storage/StorageDevice.h index bae2c779e7..bf488882a1 100644 --- a/Kernel/Devices/Storage/StorageDevice.h +++ b/Kernel/Devices/Storage/StorageDevice.h @@ -63,9 +63,9 @@ public: // ^BlockDevice virtual ErrorOr read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override; - virtual bool can_read(OpenFileDescription const&, u64) const override; + virtual bool can_read(OpenFileDescription const&, u64) const override { return true; } virtual ErrorOr write(OpenFileDescription&, u64, UserOrKernelBuffer const&, size_t) override; - virtual bool can_write(OpenFileDescription const&, u64) const override; + virtual bool can_write(OpenFileDescription const&, u64) const override { return true; } virtual void prepare_for_unplug() { m_partitions.clear(); } Vector> const& partitions() const { return m_partitions; }