mirror of
https://github.com/SerenityOS/serenity
synced 2024-09-06 17:06:31 +00:00
Kernel: Acquire reference to waitee before trying to block in sys$waitid
Previously, we would try to acquire a reference to the all processes lock or other contended resources while holding both the scheduler lock and the thread's blocker lock. This could lead to a deadlock if we actually have to block on those other resources.
This commit is contained in:
parent
dea62fe93c
commit
54161bf5b4
|
@ -14,6 +14,7 @@
|
|||
#include <AK/OwnPtr.h>
|
||||
#include <AK/String.h>
|
||||
#include <AK/Userspace.h>
|
||||
#include <AK/Variant.h>
|
||||
#include <AK/WeakPtr.h>
|
||||
#include <AK/Weakable.h>
|
||||
#include <Kernel/API/Syscall.h>
|
||||
|
@ -541,7 +542,7 @@ private:
|
|||
KResult do_killall(int signal);
|
||||
KResult do_killself(int signal);
|
||||
|
||||
KResultOr<siginfo_t> do_waitid(idtype_t idtype, int id, int options);
|
||||
KResultOr<siginfo_t> do_waitid(Variant<Empty, NonnullRefPtr<Process>, NonnullRefPtr<ProcessGroup>> waitee, int options);
|
||||
|
||||
KResultOr<NonnullOwnPtr<KString>> get_syscall_path_argument(Userspace<const char*> user_path, size_t path_length) const;
|
||||
KResultOr<NonnullOwnPtr<KString>> get_syscall_path_argument(const Syscall::StringArgument&) const;
|
||||
|
|
|
@ -4,15 +4,16 @@
|
|||
* SPDX-License-Identifier: BSD-2-Clause
|
||||
*/
|
||||
|
||||
#include <AK/Variant.h>
|
||||
#include <Kernel/Debug.h>
|
||||
#include <Kernel/Process.h>
|
||||
|
||||
namespace Kernel {
|
||||
|
||||
KResultOr<siginfo_t> Process::do_waitid(idtype_t idtype, int id, int options)
|
||||
KResultOr<siginfo_t> Process::do_waitid(Variant<Empty, NonnullRefPtr<Process>, NonnullRefPtr<ProcessGroup>> waitee, int options)
|
||||
{
|
||||
KResultOr<siginfo_t> result = KResult(KSuccess);
|
||||
if (Thread::current()->block<Thread::WaitBlocker>({}, options, idtype, id, result).was_interrupted())
|
||||
if (Thread::current()->block<Thread::WaitBlocker>({}, options, move(waitee), result).was_interrupted())
|
||||
return EINTR;
|
||||
VERIFY(!result.is_error() || (options & WNOHANG) || result.error() != KSuccess);
|
||||
return result;
|
||||
|
@ -27,18 +28,34 @@ KResultOr<FlatPtr> Process::sys$waitid(Userspace<const Syscall::SC_waitid_params
|
|||
if (!copy_from_user(¶ms, user_params))
|
||||
return EFAULT;
|
||||
|
||||
Variant<Empty, NonnullRefPtr<Process>, NonnullRefPtr<ProcessGroup>> waitee = Empty {};
|
||||
switch (params.idtype) {
|
||||
case P_ALL:
|
||||
case P_PID:
|
||||
case P_PGID:
|
||||
waitee = Empty {};
|
||||
break;
|
||||
case P_PID: {
|
||||
auto waitee_process = Process::from_pid(params.id);
|
||||
if (!waitee_process || waitee_process->ppid() != Process::current().pid()) {
|
||||
return ECHILD;
|
||||
}
|
||||
waitee = waitee_process.release_nonnull();
|
||||
break;
|
||||
}
|
||||
case P_PGID: {
|
||||
auto waitee_group = ProcessGroup::from_pgid(params.id);
|
||||
if (!waitee_group) {
|
||||
return ECHILD;
|
||||
}
|
||||
waitee = waitee_group.release_nonnull();
|
||||
break;
|
||||
}
|
||||
default:
|
||||
return EINVAL;
|
||||
}
|
||||
|
||||
dbgln_if(PROCESS_DEBUG, "sys$waitid({}, {}, {}, {})", params.idtype, params.id, params.infop, params.options);
|
||||
|
||||
auto siginfo_or_error = do_waitid(static_cast<idtype_t>(params.idtype), params.id, params.options);
|
||||
auto siginfo_or_error = do_waitid(move(waitee), params.options);
|
||||
if (siginfo_or_error.is_error())
|
||||
return siginfo_or_error.error();
|
||||
|
||||
|
|
|
@ -15,6 +15,7 @@
|
|||
#include <AK/String.h>
|
||||
#include <AK/TemporaryChange.h>
|
||||
#include <AK/Time.h>
|
||||
#include <AK/Variant.h>
|
||||
#include <AK/Vector.h>
|
||||
#include <AK/WeakPtr.h>
|
||||
#include <AK/Weakable.h>
|
||||
|
@ -705,7 +706,7 @@ public:
|
|||
Disowned
|
||||
};
|
||||
|
||||
WaitBlocker(int wait_options, idtype_t id_type, pid_t id, KResultOr<siginfo_t>& result);
|
||||
WaitBlocker(int wait_options, Variant<Empty, NonnullRefPtr<Process>, NonnullRefPtr<ProcessGroup>> waitee, KResultOr<siginfo_t>& result);
|
||||
virtual StringView state_string() const override { return "Waiting"sv; }
|
||||
virtual Type blocker_type() const override { return Type::Wait; }
|
||||
virtual void will_unblock_immediately_without_blocking(UnblockImmediatelyReason) override;
|
||||
|
@ -720,13 +721,9 @@ public:
|
|||
void do_set_result(const siginfo_t&);
|
||||
|
||||
const int m_wait_options;
|
||||
const idtype_t m_id_type;
|
||||
const pid_t m_waitee_id;
|
||||
KResultOr<siginfo_t>& m_result;
|
||||
RefPtr<Process> m_waitee;
|
||||
RefPtr<ProcessGroup> m_waitee_group;
|
||||
Variant<Empty, NonnullRefPtr<Process>, NonnullRefPtr<ProcessGroup>> m_waitee;
|
||||
bool m_did_unblock { false };
|
||||
bool m_error { false };
|
||||
bool m_got_sigchild { false };
|
||||
};
|
||||
|
||||
|
|
|
@ -601,55 +601,23 @@ void Thread::WaitBlockerSet::finalize()
|
|||
}
|
||||
}
|
||||
|
||||
Thread::WaitBlocker::WaitBlocker(int wait_options, idtype_t id_type, pid_t id, KResultOr<siginfo_t>& result)
|
||||
Thread::WaitBlocker::WaitBlocker(int wait_options, Variant<Empty, NonnullRefPtr<Process>, NonnullRefPtr<ProcessGroup>> waitee, KResultOr<siginfo_t>& result)
|
||||
: m_wait_options(wait_options)
|
||||
, m_id_type(id_type)
|
||||
, m_waitee_id(id)
|
||||
, m_result(result)
|
||||
, m_waitee(move(waitee))
|
||||
{
|
||||
}
|
||||
|
||||
bool Thread::WaitBlocker::setup_blocker()
|
||||
{
|
||||
switch (m_id_type) {
|
||||
case P_PID: {
|
||||
m_waitee = Process::from_pid(m_waitee_id);
|
||||
if (!m_waitee || m_waitee->ppid() != Process::current().pid()) {
|
||||
m_result = ECHILD;
|
||||
m_error = true;
|
||||
}
|
||||
break;
|
||||
}
|
||||
case P_PGID: {
|
||||
m_waitee_group = ProcessGroup::from_pgid(m_waitee_id);
|
||||
if (!m_waitee_group) {
|
||||
m_result = ECHILD;
|
||||
m_error = true;
|
||||
}
|
||||
break;
|
||||
}
|
||||
case P_ALL:
|
||||
break;
|
||||
default:
|
||||
VERIFY_NOT_REACHED();
|
||||
}
|
||||
|
||||
if (m_error)
|
||||
return false;
|
||||
|
||||
if (m_wait_options & WNOHANG)
|
||||
return false;
|
||||
|
||||
// NOTE: unblock may be called within add_to_blocker_set, in which
|
||||
// case it means that we already have a match without having to block.
|
||||
// In that case add_to_blocker_set will return false.
|
||||
return add_to_blocker_set(Process::current().wait_blocker_set());
|
||||
}
|
||||
|
||||
void Thread::WaitBlocker::will_unblock_immediately_without_blocking(UnblockImmediatelyReason)
|
||||
{
|
||||
if (!m_error)
|
||||
Process::current().wait_blocker_set().try_unblock(*this);
|
||||
Process::current().wait_blocker_set().try_unblock(*this);
|
||||
}
|
||||
|
||||
void Thread::WaitBlocker::was_unblocked(bool)
|
||||
|
@ -700,26 +668,20 @@ bool Thread::WaitBlocker::unblock(Process& process, UnblockFlags flags, u8 signa
|
|||
{
|
||||
VERIFY(flags != UnblockFlags::Terminated || signal == 0); // signal argument should be ignored for Terminated
|
||||
|
||||
switch (m_id_type) {
|
||||
case P_PID:
|
||||
VERIFY(m_waitee);
|
||||
if (process.pid() != m_waitee_id)
|
||||
return false;
|
||||
break;
|
||||
case P_PGID:
|
||||
VERIFY(m_waitee_group);
|
||||
if (process.pgid() != m_waitee_group->pgid())
|
||||
return false;
|
||||
break;
|
||||
case P_ALL:
|
||||
if (flags == UnblockFlags::Disowned) {
|
||||
bool do_not_unblock = m_waitee.visit(
|
||||
[&](NonnullRefPtr<Process> const& waitee_process) {
|
||||
return &process != waitee_process;
|
||||
},
|
||||
[&](NonnullRefPtr<ProcessGroup> const& waitee_process_group) {
|
||||
return waitee_process_group->pgid() != process.pgid();
|
||||
},
|
||||
[&](Empty const&) {
|
||||
// Generic waiter won't be unblocked by disown
|
||||
return false;
|
||||
}
|
||||
break;
|
||||
default:
|
||||
VERIFY_NOT_REACHED();
|
||||
}
|
||||
return flags == UnblockFlags::Disowned;
|
||||
});
|
||||
|
||||
if (do_not_unblock)
|
||||
return false;
|
||||
|
||||
switch (flags) {
|
||||
case UnblockFlags::Terminated:
|
||||
|
|
Loading…
Reference in a new issue