Kernel: Resolve lock-inversion based deadlock in MasterPTY & BlockSet

MasterPTY's double buffer unblock callback would take m_slave's
spinlock and then call evaluate_block_conditions() which would take
BlockerSet's spinlock, while on the other hand, BlockerSet's
add_blocker would take BlockerSet's spinlock, and then call
should_add_blocker, which would call unblock_if_conditions_are_met,
which would then call should_unblock, which will finally call
MasterPTY::can_read() which will take m_slave's spinlock.
Resolve this by moving the call to evaluate_block_conditions() out of
the scope of m_slave's spinlock, as there's no need to hold the lock
while calling it anyways.
This commit is contained in:
Idan Horowitz 2023-12-15 23:27:53 +02:00 committed by Andreas Kling
parent 4534a43aae
commit ab06a76920
2 changed files with 13 additions and 4 deletions

View file

@ -36,10 +36,14 @@ MasterPTY::MasterPTY(unsigned index, NonnullOwnPtr<DoubleBuffer> buffer)
, m_buffer(move(buffer))
{
m_buffer->set_unblock_callback([this]() {
m_slave.with([this](auto& slave) {
if (slave)
evaluate_block_conditions();
});
// Note that has_slave() takes and then releases the m_slave spinlock.
// Not holding the spinlock while calling evaluate_block_conditions is legal,
// as the call will trigger a check to see if waiters may be unblocked,
// and if it was called spuriously (i.e. because the slave disappeared between
// calling the unblock callback and the actual block condition evaluation),
// the waiters will simply not unblock.
if (has_slave())
evaluate_block_conditions();
});
}

View file

@ -43,6 +43,11 @@ private:
virtual ErrorOr<void> ioctl(OpenFileDescription&, unsigned request, Userspace<void*> arg) override;
virtual StringView class_name() const override { return "MasterPTY"sv; }
bool has_slave() const
{
return m_slave.with([](auto& slave) -> bool { return slave; });
}
SpinlockProtected<RefPtr<SlavePTY>, LockRank::None> m_slave;
unsigned m_index;
bool m_closed { false };