1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-26 23:17:46 +00:00

Kernel: Make ProcessGroup a ListedRefCounted and fix two races

This closes two race windows:

- ProcessGroup removed itself from the "all process groups" list in its
  destructor. It was possible to walk the list between the last unref()
  and the destructor invocation, and grab a pointer to a ProcessGroup
  that was about to get deleted.

- sys$setsid() could end up creating a process group that already
  existed, as there was a race window between checking if the PGID
  is used, and actually creating a ProcessGroup with that PGID.
This commit is contained in:
Andreas Kling 2023-04-04 16:38:46 +02:00
parent 37bfc36601
commit 3e30d9bc99
3 changed files with 29 additions and 36 deletions

View file

@ -1,6 +1,6 @@
/*
* Copyright (c) 2020, the SerenityOS developers.
* Copyright (c) 2021, Andreas Kling <kling@serenityos.org>
* Copyright (c) 2021-2023, Andreas Kling <kling@serenityos.org>
*
* SPDX-License-Identifier: BSD-2-Clause
*/
@ -10,45 +10,44 @@
namespace Kernel {
static Singleton<SpinlockProtected<ProcessGroup::List, LockRank::None>> s_process_groups;
static Singleton<SpinlockProtected<ProcessGroup::AllInstancesList, LockRank::None>> s_all_instances;
SpinlockProtected<ProcessGroup::List, LockRank::None>& process_groups()
SpinlockProtected<ProcessGroup::AllInstancesList, LockRank::None>& ProcessGroup::all_instances()
{
return *s_process_groups;
return s_all_instances;
}
ProcessGroup::~ProcessGroup()
{
process_groups().with([&](auto& groups) {
groups.remove(*this);
});
}
ProcessGroup::~ProcessGroup() = default;
ErrorOr<NonnullRefPtr<ProcessGroup>> ProcessGroup::create(ProcessGroupID pgid)
ErrorOr<NonnullRefPtr<ProcessGroup>> ProcessGroup::create_if_unused_pgid(ProcessGroupID pgid)
{
auto process_group = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) ProcessGroup(pgid)));
process_groups().with([&](auto& groups) {
groups.prepend(*process_group);
return all_instances().with([&](auto& all_instances) -> ErrorOr<NonnullRefPtr<ProcessGroup>> {
for (auto& process_group : all_instances) {
if (process_group.pgid() == pgid)
return EPERM;
}
auto process_group = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) ProcessGroup(pgid)));
all_instances.prepend(*process_group);
return process_group;
});
return process_group;
}
ErrorOr<NonnullRefPtr<ProcessGroup>> ProcessGroup::find_or_create(ProcessGroupID pgid)
{
return process_groups().with([&](auto& groups) -> ErrorOr<NonnullRefPtr<ProcessGroup>> {
for (auto& group : groups) {
return all_instances().with([&](auto& all_instances) -> ErrorOr<NonnullRefPtr<ProcessGroup>> {
for (auto& group : all_instances) {
if (group.pgid() == pgid)
return group;
}
auto process_group = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) ProcessGroup(pgid)));
groups.prepend(*process_group);
all_instances.prepend(*process_group);
return process_group;
});
}
RefPtr<ProcessGroup> ProcessGroup::from_pgid(ProcessGroupID pgid)
{
return process_groups().with([&](auto& groups) -> RefPtr<ProcessGroup> {
return all_instances().with([&](auto& groups) -> RefPtr<ProcessGroup> {
for (auto& group : groups) {
if (group.pgid() == pgid)
return &group;

View file

@ -1,15 +1,15 @@
/*
* Copyright (c) 2020, the SerenityOS developers.
* Copyright (c) 2020-2023, the SerenityOS developers.
*
* SPDX-License-Identifier: BSD-2-Clause
*/
#pragma once
#include <AK/AtomicRefCounted.h>
#include <AK/IntrusiveList.h>
#include <AK/RefPtr.h>
#include <Kernel/Forward.h>
#include <Kernel/Library/ListedRefCounted.h>
#include <Kernel/Library/LockWeakable.h>
#include <Kernel/Locking/SpinlockProtected.h>
#include <Kernel/UnixTypes.h>
@ -17,7 +17,7 @@
namespace Kernel {
class ProcessGroup
: public AtomicRefCounted<ProcessGroup>
: public ListedRefCounted<ProcessGroup, LockType::Spinlock>
, public LockWeakable<ProcessGroup> {
AK_MAKE_NONMOVABLE(ProcessGroup);
@ -26,7 +26,7 @@ class ProcessGroup
public:
~ProcessGroup();
static ErrorOr<NonnullRefPtr<ProcessGroup>> create(ProcessGroupID);
static ErrorOr<NonnullRefPtr<ProcessGroup>> create_if_unused_pgid(ProcessGroupID);
static ErrorOr<NonnullRefPtr<ProcessGroup>> find_or_create(ProcessGroupID);
static RefPtr<ProcessGroup> from_pgid(ProcessGroupID);
@ -38,13 +38,13 @@ private:
{
}
IntrusiveListNode<ProcessGroup> m_list_node;
ProcessGroupID m_pgid;
mutable IntrusiveListNode<ProcessGroup> m_list_node;
public:
using List = IntrusiveList<&ProcessGroup::m_list_node>;
using AllInstancesList = IntrusiveList<&ProcessGroup::m_list_node>;
static SpinlockProtected<AllInstancesList, LockRank::None>& all_instances();
};
SpinlockProtected<ProcessGroup::List, LockRank::None>& process_groups();
}

View file

@ -28,16 +28,10 @@ ErrorOr<FlatPtr> Process::sys$setsid()
{
VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this);
TRY(require_promise(Pledge::proc));
bool found_process_with_same_pgid_as_my_pid = false;
TRY(Process::for_each_in_pgrp_in_same_jail(pid().value(), [&](auto&) -> ErrorOr<void> {
found_process_with_same_pgid_as_my_pid = true;
return {};
}));
if (found_process_with_same_pgid_as_my_pid)
return EPERM;
// Create a new Session and a new ProcessGroup.
auto process_group = TRY(ProcessGroup::create(ProcessGroupID(pid().value())));
// NOTE: ProcessGroup::create_if_unused_pgid() will fail with EPERM
// if a process group with the same PGID already exists.
auto process_group = TRY(ProcessGroup::create_if_unused_pgid(ProcessGroupID(pid().value())));
return with_mutable_protected_data([&](auto& protected_data) -> ErrorOr<FlatPtr> {
protected_data.tty = nullptr;
protected_data.process_group = move(process_group);