1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-26 19:27:45 +00:00

Kernel/TTY: Protect SlavePTY pointer with proper spinlock

Instead of using a LockRefPtr, we could easily use SpinlockProtected to
ensure proper locking of this pointer.
This commit is contained in:
Liav A 2023-07-08 14:41:00 +03:00 committed by Andrew Kaster
parent b55199c227
commit 82428e2a05
2 changed files with 50 additions and 34 deletions

View file

@ -22,7 +22,9 @@ ErrorOr<NonnullLockRefPtr<MasterPTY>> MasterPTY::try_create(unsigned int index)
auto master_pty = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) MasterPTY(index, move(buffer)))); auto master_pty = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) MasterPTY(index, move(buffer))));
auto credentials = Process::current().credentials(); auto credentials = Process::current().credentials();
auto slave_pty = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) SlavePTY(*master_pty, credentials->uid(), credentials->gid(), index))); auto slave_pty = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) SlavePTY(*master_pty, credentials->uid(), credentials->gid(), index)));
master_pty->m_slave = slave_pty; master_pty->m_slave.with([&slave_pty](auto& slave) {
slave = *slave_pty;
});
TRY(master_pty->after_inserting()); TRY(master_pty->after_inserting());
TRY(slave_pty->after_inserting()); TRY(slave_pty->after_inserting());
return master_pty; return master_pty;
@ -34,9 +36,11 @@ MasterPTY::MasterPTY(unsigned index, NonnullOwnPtr<DoubleBuffer> buffer)
, m_buffer(move(buffer)) , m_buffer(move(buffer))
{ {
m_buffer->set_unblock_callback([this]() { m_buffer->set_unblock_callback([this]() {
if (m_slave) m_slave.with([this](auto& slave) {
if (slave)
evaluate_block_conditions(); evaluate_block_conditions();
}); });
});
} }
MasterPTY::~MasterPTY() MasterPTY::~MasterPTY()
@ -47,24 +51,30 @@ MasterPTY::~MasterPTY()
ErrorOr<size_t> MasterPTY::read(OpenFileDescription&, u64, UserOrKernelBuffer& buffer, size_t size) ErrorOr<size_t> MasterPTY::read(OpenFileDescription&, u64, UserOrKernelBuffer& buffer, size_t size)
{ {
if (!m_slave && m_buffer->is_empty()) return m_slave.with([this, &buffer, size](auto& slave) -> ErrorOr<size_t> {
if (!slave && m_buffer->is_empty())
return 0; return 0;
return m_buffer->read(buffer, size); return m_buffer->read(buffer, size);
});
} }
ErrorOr<size_t> MasterPTY::write(OpenFileDescription&, u64, UserOrKernelBuffer const& buffer, size_t size) ErrorOr<size_t> MasterPTY::write(OpenFileDescription&, u64, UserOrKernelBuffer const& buffer, size_t size)
{ {
if (!m_slave) return m_slave.with([&](auto& slave) -> ErrorOr<size_t> {
if (!slave)
return EIO; return EIO;
m_slave->on_master_write(buffer, size); slave->on_master_write(buffer, size);
return size; return size;
});
} }
bool MasterPTY::can_read(OpenFileDescription const&, u64) const bool MasterPTY::can_read(OpenFileDescription const&, u64) const
{ {
if (!m_slave) return m_slave.with([this](auto& slave) -> bool {
if (!slave)
return true; return true;
return !m_buffer->is_empty(); return !m_buffer->is_empty();
});
} }
bool MasterPTY::can_write(OpenFileDescription const&, u64) const bool MasterPTY::can_write(OpenFileDescription const&, u64) const
@ -74,11 +84,13 @@ bool MasterPTY::can_write(OpenFileDescription const&, u64) const
void MasterPTY::notify_slave_closed(Badge<SlavePTY>) void MasterPTY::notify_slave_closed(Badge<SlavePTY>)
{ {
dbgln_if(MASTERPTY_DEBUG, "MasterPTY({}): slave closed, my retains: {}, slave retains: {}", m_index, ref_count(), m_slave->ref_count()); m_slave.with([this](auto& slave) {
dbgln_if(MASTERPTY_DEBUG, "MasterPTY({}): slave closed, my retains: {}, slave retains: {}", m_index, ref_count(), slave->ref_count());
// +1 ref for my MasterPTY::m_slave // +1 ref for my MasterPTY::m_slave
// +1 ref for OpenFileDescription::m_device // +1 ref for OpenFileDescription::m_device
if (m_slave->ref_count() == 2) if (slave->ref_count() == 2)
m_slave = nullptr; slave = nullptr;
});
} }
ErrorOr<size_t> MasterPTY::on_slave_write(UserOrKernelBuffer const& data, size_t size) ErrorOr<size_t> MasterPTY::on_slave_write(UserOrKernelBuffer const& data, size_t size)
@ -102,8 +114,10 @@ ErrorOr<void> MasterPTY::close()
// From this point, let's consider ourselves closed. // From this point, let's consider ourselves closed.
m_closed = true; m_closed = true;
if (m_slave) m_slave.with([](auto& slave) {
m_slave->hang_up(); if (slave)
slave->hang_up();
});
return {}; return {};
} }
@ -111,7 +125,8 @@ ErrorOr<void> MasterPTY::close()
ErrorOr<void> MasterPTY::ioctl(OpenFileDescription& description, unsigned request, Userspace<void*> arg) ErrorOr<void> MasterPTY::ioctl(OpenFileDescription& description, unsigned request, Userspace<void*> arg)
{ {
TRY(Process::current().require_promise(Pledge::tty)); TRY(Process::current().require_promise(Pledge::tty));
if (!m_slave) return m_slave.with([&](auto& slave) -> ErrorOr<void> {
if (!slave)
return EIO; return EIO;
switch (request) { switch (request) {
case TIOCGPTN: { case TIOCGPTN: {
@ -120,10 +135,11 @@ ErrorOr<void> MasterPTY::ioctl(OpenFileDescription& description, unsigned reques
} }
case TIOCSWINSZ: case TIOCSWINSZ:
case TIOCGPGRP: case TIOCGPGRP:
return m_slave->ioctl(description, request, arg); return slave->ioctl(description, request, arg);
default: default:
return EINVAL; return EINVAL;
} }
});
} }
ErrorOr<NonnullOwnPtr<KString>> MasterPTY::pseudo_path(OpenFileDescription const&) const ErrorOr<NonnullOwnPtr<KString>> MasterPTY::pseudo_path(OpenFileDescription const&) const

View file

@ -43,7 +43,7 @@ private:
virtual ErrorOr<void> ioctl(OpenFileDescription&, unsigned request, Userspace<void*> arg) override; virtual ErrorOr<void> ioctl(OpenFileDescription&, unsigned request, Userspace<void*> arg) override;
virtual StringView class_name() const override { return "MasterPTY"sv; } virtual StringView class_name() const override { return "MasterPTY"sv; }
LockRefPtr<SlavePTY> m_slave; SpinlockProtected<RefPtr<SlavePTY>, LockRank::None> m_slave;
unsigned m_index; unsigned m_index;
bool m_closed { false }; bool m_closed { false };
NonnullOwnPtr<DoubleBuffer> m_buffer; NonnullOwnPtr<DoubleBuffer> m_buffer;