Kernel: Stop using *LockRefPtr for ProcessGroup

Had to wrap Process::m_pg in a SpinlockProtected for this to be safe.
This commit is contained in:
Andreas Kling 2023-04-02 21:57:12 +02:00
parent ed1253ab90
commit 83b409083b
Notes: sideshowbarker 2024-07-17 00:37:23 +09:00
9 changed files with 31 additions and 23 deletions

View file

@ -236,7 +236,10 @@ public:
return with_protected_data([](auto& protected_data) { return protected_data.sid; });
}
bool is_session_leader() const { return sid().value() == pid().value(); }
ProcessGroupID pgid() const { return m_pg ? m_pg->pgid() : 0; }
ProcessGroupID pgid() const
{
return m_pg.with([&](auto& pg) { return pg ? pg->pgid() : 0; });
}
bool is_group_leader() const { return pgid().value() == pid().value(); }
ProcessID ppid() const
{
@ -622,7 +625,7 @@ private:
ErrorOr<void> do_killall(int signal);
ErrorOr<void> do_killself(int signal);
ErrorOr<siginfo_t> do_waitid(Variant<Empty, NonnullRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee, int options);
ErrorOr<siginfo_t> do_waitid(Variant<Empty, NonnullRefPtr<Process>, NonnullRefPtr<ProcessGroup>> waitee, int options);
static ErrorOr<NonnullOwnPtr<KString>> get_syscall_path_argument(Userspace<char const*> user_path, size_t path_length);
static ErrorOr<NonnullOwnPtr<KString>> get_syscall_path_argument(Syscall::StringArgument const&);
@ -674,7 +677,7 @@ private:
SpinlockProtected<OwnPtr<Memory::AddressSpace>, LockRank::None> m_space;
LockRefPtr<ProcessGroup> m_pg;
SpinlockProtected<RefPtr<ProcessGroup>, LockRank::None> m_pg;
RecursiveSpinlock<LockRank::None> mutable m_protected_data_lock;
AtomicEdgeAction<u32> m_protected_data_refs;

View file

@ -24,31 +24,31 @@ ProcessGroup::~ProcessGroup()
});
}
ErrorOr<NonnullLockRefPtr<ProcessGroup>> ProcessGroup::try_create(ProcessGroupID pgid)
ErrorOr<NonnullRefPtr<ProcessGroup>> ProcessGroup::create(ProcessGroupID pgid)
{
auto process_group = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) ProcessGroup(pgid)));
auto process_group = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) ProcessGroup(pgid)));
process_groups().with([&](auto& groups) {
groups.prepend(*process_group);
});
return process_group;
}
ErrorOr<NonnullLockRefPtr<ProcessGroup>> ProcessGroup::try_find_or_create(ProcessGroupID pgid)
ErrorOr<NonnullRefPtr<ProcessGroup>> ProcessGroup::find_or_create(ProcessGroupID pgid)
{
return process_groups().with([&](auto& groups) -> ErrorOr<NonnullLockRefPtr<ProcessGroup>> {
return process_groups().with([&](auto& groups) -> ErrorOr<NonnullRefPtr<ProcessGroup>> {
for (auto& group : groups) {
if (group.pgid() == pgid)
return group;
}
auto process_group = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) ProcessGroup(pgid)));
auto process_group = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) ProcessGroup(pgid)));
groups.prepend(*process_group);
return process_group;
});
}
LockRefPtr<ProcessGroup> ProcessGroup::from_pgid(ProcessGroupID pgid)
RefPtr<ProcessGroup> ProcessGroup::from_pgid(ProcessGroupID pgid)
{
return process_groups().with([&](auto& groups) -> LockRefPtr<ProcessGroup> {
return process_groups().with([&](auto& groups) -> RefPtr<ProcessGroup> {
for (auto& group : groups) {
if (group.pgid() == pgid)
return &group;

View file

@ -8,6 +8,7 @@
#include <AK/AtomicRefCounted.h>
#include <AK/IntrusiveList.h>
#include <AK/RefPtr.h>
#include <Kernel/Forward.h>
#include <Kernel/Library/LockWeakable.h>
#include <Kernel/Locking/SpinlockProtected.h>
@ -25,9 +26,9 @@ class ProcessGroup
public:
~ProcessGroup();
static ErrorOr<NonnullLockRefPtr<ProcessGroup>> try_create(ProcessGroupID);
static ErrorOr<NonnullLockRefPtr<ProcessGroup>> try_find_or_create(ProcessGroupID);
static LockRefPtr<ProcessGroup> from_pgid(ProcessGroupID);
static ErrorOr<NonnullRefPtr<ProcessGroup>> create(ProcessGroupID);
static ErrorOr<NonnullRefPtr<ProcessGroup>> find_or_create(ProcessGroupID);
static RefPtr<ProcessGroup> from_pgid(ProcessGroupID);
ProcessGroupID const& pgid() const { return m_pgid; }

View file

@ -96,7 +96,9 @@ ErrorOr<FlatPtr> Process::sys$fork(RegisterState& regs)
});
}));
child->m_pg = m_pg;
child->m_pg.with([&](auto& child_pg) {
child_pg = m_pg.with([&](auto& pg) { return pg; });
});
with_protected_data([&](auto& my_protected_data) {
child->with_mutable_protected_data([&](auto& child_protected_data) {

View file

@ -38,7 +38,8 @@ ErrorOr<FlatPtr> Process::sys$setsid()
return EPERM;
// Create a new Session and a new ProcessGroup.
m_pg = TRY(ProcessGroup::try_create(ProcessGroupID(pid().value())));
auto process_group = TRY(ProcessGroup::create(ProcessGroupID(pid().value())));
m_pg.with([&](auto& pg) { pg = move(process_group); });
m_tty = nullptr;
return with_mutable_protected_data([&](auto& protected_data) -> ErrorOr<FlatPtr> {
protected_data.sid = pid().value();
@ -119,7 +120,8 @@ ErrorOr<FlatPtr> Process::sys$setpgid(pid_t specified_pid, pid_t specified_pgid)
return EPERM;
}
// FIXME: There are more EPERM conditions to check for here..
process->m_pg = TRY(ProcessGroup::try_find_or_create(new_pgid));
auto process_group = TRY(ProcessGroup::find_or_create(new_pgid));
process->m_pg.with([&](auto& pg) { pg = move(process_group); });
return with_mutable_protected_data([&](auto& protected_data) -> ErrorOr<FlatPtr> {
auto credentials = this->credentials();

View file

@ -10,7 +10,7 @@
namespace Kernel {
ErrorOr<siginfo_t> Process::do_waitid(Variant<Empty, NonnullRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee, int options)
ErrorOr<siginfo_t> Process::do_waitid(Variant<Empty, NonnullRefPtr<Process>, NonnullRefPtr<ProcessGroup>> waitee, int options)
{
ErrorOr<siginfo_t> result = siginfo_t {};
if (Thread::current()->block<Thread::WaitBlocker>({}, options, move(waitee), result).was_interrupted())
@ -25,7 +25,7 @@ ErrorOr<FlatPtr> Process::sys$waitid(Userspace<Syscall::SC_waitid_params const*>
TRY(require_promise(Pledge::proc));
auto params = TRY(copy_typed_from_user(user_params));
Variant<Empty, NonnullRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee;
Variant<Empty, NonnullRefPtr<Process>, NonnullRefPtr<ProcessGroup>> waitee;
switch (params.idtype) {
case P_ALL:
break;

View file

@ -500,7 +500,7 @@ ErrorOr<void> TTY::ioctl(OpenFileDescription&, unsigned request, Userspace<void*
return EPERM;
if (process && pgid != process->pgid())
return EPERM;
m_pg = process_group;
m_pg = TRY(process_group->try_make_weak_ptr());
if (process) {
if (auto parent = Process::from_pid_ignoring_jails(process->ppid())) {

View file

@ -644,7 +644,7 @@ public:
Disowned
};
WaitBlocker(int wait_options, Variant<Empty, NonnullRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee, ErrorOr<siginfo_t>& result);
WaitBlocker(int wait_options, Variant<Empty, NonnullRefPtr<Process>, NonnullRefPtr<ProcessGroup>> waitee, ErrorOr<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;
@ -660,7 +660,7 @@ public:
int const m_wait_options;
ErrorOr<siginfo_t>& m_result;
Variant<Empty, NonnullRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> const m_waitee;
Variant<Empty, NonnullRefPtr<Process>, NonnullRefPtr<ProcessGroup>> const m_waitee;
bool m_did_unblock { false };
bool m_got_sigchild { false };
};

View file

@ -663,7 +663,7 @@ void Thread::WaitBlockerSet::finalize()
}
}
Thread::WaitBlocker::WaitBlocker(int wait_options, Variant<Empty, NonnullRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee, ErrorOr<siginfo_t>& result)
Thread::WaitBlocker::WaitBlocker(int wait_options, Variant<Empty, NonnullRefPtr<Process>, NonnullRefPtr<ProcessGroup>> waitee, ErrorOr<siginfo_t>& result)
: m_wait_options(wait_options)
, m_result(result)
, m_waitee(move(waitee))
@ -734,7 +734,7 @@ bool Thread::WaitBlocker::unblock(Process& process, UnblockFlags flags, u8 signa
[&](NonnullRefPtr<Process> const& waitee_process) {
return &process != waitee_process;
},
[&](NonnullLockRefPtr<ProcessGroup> const& waitee_process_group) {
[&](NonnullRefPtr<ProcessGroup> const& waitee_process_group) {
return waitee_process_group->pgid() != process.pgid();
},
[&](Empty const&) {