mirror of
https://github.com/RGBCube/serenity
synced 2025-05-14 08:44:58 +00:00
Kernel: Get RefPtr<Device> from the DeviceManagement::get_device method
Instead of returning a raw pointer, which could be technically invalid when using it in the caller function, we return a valid RefPtr of such device. This ensures that the code in DevPtsFS is now safe from a rare race condition in which the SlavePTY device is gone but we still have a pointer to it.
This commit is contained in:
parent
5dcf03ad9a
commit
11ead5c84f
6 changed files with 24 additions and 16 deletions
|
@ -42,13 +42,13 @@ DeviceManagement& DeviceManagement::the()
|
||||||
return *s_the;
|
return *s_the;
|
||||||
}
|
}
|
||||||
|
|
||||||
Device* DeviceManagement::get_device(MajorNumber major, MinorNumber minor)
|
RefPtr<Device> DeviceManagement::get_device(MajorNumber major, MinorNumber minor)
|
||||||
{
|
{
|
||||||
return m_devices.with([&](auto& map) -> Device* {
|
return m_devices.with([&](auto& map) -> RefPtr<Device> {
|
||||||
auto it = map.find(encoded_device(major.value(), minor.value()));
|
auto it = map.find(encoded_device(major.value(), minor.value()));
|
||||||
if (it == map.end())
|
if (it == map.end())
|
||||||
return nullptr;
|
return nullptr;
|
||||||
return it->value;
|
return *it->value;
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -41,7 +41,7 @@ public:
|
||||||
|
|
||||||
void for_each(Function<void(Device&)>);
|
void for_each(Function<void(Device&)>);
|
||||||
ErrorOr<void> try_for_each(Function<ErrorOr<void>(Device&)>);
|
ErrorOr<void> try_for_each(Function<ErrorOr<void>(Device&)>);
|
||||||
Device* get_device(MajorNumber major, MinorNumber minor);
|
RefPtr<Device> get_device(MajorNumber major, MinorNumber minor);
|
||||||
|
|
||||||
NullDevice const& null_device() const;
|
NullDevice const& null_device() const;
|
||||||
NullDevice& null_device();
|
NullDevice& null_device();
|
||||||
|
|
|
@ -353,9 +353,9 @@ UNMAP_AFTER_INIT void StorageManagement::determine_block_boot_device()
|
||||||
// Note: We simply fetch the corresponding BlockDevice with the major and minor parameters.
|
// Note: We simply fetch the corresponding BlockDevice with the major and minor parameters.
|
||||||
// We don't try to accept and resolve a partition number as it will make this code much more
|
// We don't try to accept and resolve a partition number as it will make this code much more
|
||||||
// complicated. This rule is also explained in the boot_device_addressing(7) manual page.
|
// complicated. This rule is also explained in the boot_device_addressing(7) manual page.
|
||||||
LockRefPtr<Device> device = DeviceManagement::the().get_device(parameters_view[0], parameters_view[1]);
|
auto device = DeviceManagement::the().get_device(parameters_view[0], parameters_view[1]);
|
||||||
if (device && device->is_block_device())
|
if (device && device->is_block_device())
|
||||||
m_boot_block_device = static_ptr_cast<BlockDevice>(device);
|
m_boot_block_device = *static_ptr_cast<BlockDevice>(device);
|
||||||
}
|
}
|
||||||
|
|
||||||
UNMAP_AFTER_INIT void StorageManagement::determine_boot_device_with_logical_unit_number()
|
UNMAP_AFTER_INIT void StorageManagement::determine_boot_device_with_logical_unit_number()
|
||||||
|
|
|
@ -23,7 +23,7 @@ DevPtsFS::~DevPtsFS() = default;
|
||||||
|
|
||||||
ErrorOr<void> DevPtsFS::initialize()
|
ErrorOr<void> DevPtsFS::initialize()
|
||||||
{
|
{
|
||||||
m_root_inode = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) DevPtsFSInode(*this, 1, nullptr)));
|
m_root_inode = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) DevPtsFSInode(*this)));
|
||||||
m_root_inode->m_metadata.inode = { fsid(), 1 };
|
m_root_inode->m_metadata.inode = { fsid(), 1 };
|
||||||
m_root_inode->m_metadata.mode = 0040555;
|
m_root_inode->m_metadata.mode = 0040555;
|
||||||
m_root_inode->m_metadata.uid = 0;
|
m_root_inode->m_metadata.uid = 0;
|
||||||
|
@ -55,15 +55,15 @@ ErrorOr<NonnullRefPtr<Inode>> DevPtsFS::get_inode(InodeIdentifier inode_id) cons
|
||||||
return *m_root_inode;
|
return *m_root_inode;
|
||||||
|
|
||||||
unsigned pty_index = inode_index_to_pty_index(inode_id.index());
|
unsigned pty_index = inode_index_to_pty_index(inode_id.index());
|
||||||
auto* device = DeviceManagement::the().get_device(201, pty_index);
|
auto device = DeviceManagement::the().get_device(201, pty_index);
|
||||||
VERIFY(device);
|
VERIFY(device);
|
||||||
|
|
||||||
auto* pts_device = static_cast<SlavePTY*>(device);
|
auto& pts_device = static_cast<SlavePTY&>(*device);
|
||||||
auto inode = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) DevPtsFSInode(const_cast<DevPtsFS&>(*this), inode_id.index(), pts_device)));
|
auto inode = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) DevPtsFSInode(const_cast<DevPtsFS&>(*this), inode_id.index(), pts_device)));
|
||||||
inode->m_metadata.inode = inode_id;
|
inode->m_metadata.inode = inode_id;
|
||||||
inode->m_metadata.size = 0;
|
inode->m_metadata.size = 0;
|
||||||
inode->m_metadata.uid = pts_device->uid();
|
inode->m_metadata.uid = pts_device.uid();
|
||||||
inode->m_metadata.gid = pts_device->gid();
|
inode->m_metadata.gid = pts_device.gid();
|
||||||
inode->m_metadata.mode = 0020600;
|
inode->m_metadata.mode = 0020600;
|
||||||
inode->m_metadata.major_device = device->major();
|
inode->m_metadata.major_device = device->major();
|
||||||
inode->m_metadata.minor_device = device->minor();
|
inode->m_metadata.minor_device = device->minor();
|
||||||
|
|
|
@ -16,11 +16,16 @@ static InodeIndex pty_index_to_inode_index(unsigned pty_index)
|
||||||
return pty_index + 2;
|
return pty_index + 2;
|
||||||
}
|
}
|
||||||
|
|
||||||
DevPtsFSInode::DevPtsFSInode(DevPtsFS& fs, InodeIndex index, SlavePTY* pty)
|
// NOTE: This constructor is used for the root inode only.
|
||||||
: Inode(fs, index)
|
DevPtsFSInode::DevPtsFSInode(DevPtsFS& fs)
|
||||||
|
: Inode(fs, 1)
|
||||||
|
{
|
||||||
|
}
|
||||||
|
|
||||||
|
DevPtsFSInode::DevPtsFSInode(DevPtsFS& fs, InodeIndex index, SlavePTY& pty)
|
||||||
|
: Inode(fs, index)
|
||||||
|
, m_pty(pty)
|
||||||
{
|
{
|
||||||
if (pty)
|
|
||||||
m_pty = *pty;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
DevPtsFSInode::~DevPtsFSInode() = default;
|
DevPtsFSInode::~DevPtsFSInode() = default;
|
||||||
|
|
|
@ -23,7 +23,10 @@ public:
|
||||||
DevPtsFS const& fs() const { return static_cast<DevPtsFS const&>(Inode::fs()); }
|
DevPtsFS const& fs() const { return static_cast<DevPtsFS const&>(Inode::fs()); }
|
||||||
|
|
||||||
private:
|
private:
|
||||||
DevPtsFSInode(DevPtsFS&, InodeIndex, SlavePTY*);
|
DevPtsFSInode(DevPtsFS&, InodeIndex, SlavePTY&);
|
||||||
|
|
||||||
|
// NOTE: This constructor is used for the root inode only.
|
||||||
|
DevPtsFSInode(DevPtsFS&);
|
||||||
|
|
||||||
// ^Inode
|
// ^Inode
|
||||||
virtual ErrorOr<size_t> read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override;
|
virtual ErrorOr<size_t> read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override;
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue