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.
This commit is contained in:
Liav A 2023-06-30 08:55:57 +03:00 committed by Andrew Kaster
parent c33246235a
commit 763ef690c6
Notes: sideshowbarker 2024-07-18 00:41:35 +09:00
2 changed files with 16 additions and 18 deletions

View File

@ -85,10 +85,14 @@ StringView StorageDevice::command_set_to_string_view() const
ErrorOr<size_t> 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<size_t>((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<size_t> 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<size_t> 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<size_t> 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<size_t>((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<size_t> 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<size_t> 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<void> StorageDevice::ioctl(OpenFileDescription&, unsigned request, Userspace<void*> arg)
{
switch (request) {

View File

@ -63,9 +63,9 @@ public:
// ^BlockDevice
virtual ErrorOr<size_t> 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<size_t> 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<NonnullLockRefPtr<DiskPartition>> const& partitions() const { return m_partitions; }