Skip to content

Commit 31d2262

Browse files
committed
fix(process): serialize child ownership handoff
Keep the old leader's children index and every child parent link in one tasklist-like relationship transaction during non-leader exec. Make release select the current real parent and remove the matching children entry under the same lock, preventing stale virtual PIDs when child reap races the handoff. Reuse the locked reparent helper and remove the now-redundant wrapper. Signed-off-by: longjin <longjin@dragonos.org>
1 parent 2ef2c6d commit 31d2262

3 files changed

Lines changed: 39 additions & 32 deletions

File tree

kernel/src/process/exec.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -510,9 +510,12 @@ fn de_thread(pcb: &Arc<ProcessControlBlock>) -> Result<(), SystemError> {
510510
}
511511
drop(membership_guard);
512512

513-
// Transfer the old leader's children list to the new leader.
514-
let moved_children = {
515-
let leader_pid_ns = leader.active_pid_ns();
513+
// Transfer the old leader's children list and parent links as one
514+
// tasklist-like transaction. A concurrent release must observe both
515+
// the new owner and its index entry, or neither.
516+
let _relation_guard = PTRACE_RELATION_LOCK.lock_irqsave();
517+
let leader_pid_ns = leader.active_pid_ns();
518+
let moved_child_pids = {
516519
let (first, second) =
517520
if (Arc::as_ptr(&current) as usize) <= (Arc::as_ptr(leader) as usize) {
518521
(current.clone(), leader.clone())
@@ -532,12 +535,13 @@ fn de_thread(pcb: &Arc<ProcessControlBlock>) -> Result<(), SystemError> {
532535
first_children.extend(moved.iter().copied());
533536
moved
534537
}
538+
};
539+
let moved_children = moved_child_pids
535540
.into_iter()
536541
.filter_map(|pid| ProcessManager::find_task_by_pid_ns(pid, &leader_pid_ns))
537-
.collect::<Vec<_>>()
538-
};
542+
.collect::<Vec<_>>();
539543
for child in moved_children {
540-
ProcessControlBlock::reparent_child_links_from_thread_group(
544+
ProcessControlBlock::reparent_child_links_from_thread_group_locked(
541545
&child,
542546
current.tgid,
543547
&current,
@@ -546,13 +550,13 @@ fn de_thread(pcb: &Arc<ProcessControlBlock>) -> Result<(), SystemError> {
546550

547551
// Inherit parent process relationships from the old leader.
548552
{
549-
let _relation_guard = PTRACE_RELATION_LOCK.lock_irqsave();
550553
let leader_parent = leader.real_parent_pcb.read_irqsave().clone();
551554
*current.parent_pcb.write_irqsave() = leader_parent.clone();
552555
*current.real_parent_pcb.write_irqsave() = leader_parent.clone();
553556
*current.wait_parent_pcb.write_irqsave() = leader_parent.clone();
554557
*current.fork_parent_pcb.write_irqsave() = leader_parent;
555558
}
559+
drop(_relation_guard);
556560

557561
// log::info!("de_thread: reparented current to old leader's parent");
558562

kernel/src/process/manager/exit.rs

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -540,22 +540,28 @@ impl ProcessManager {
540540
ProcessManager::exit_ptrace(pcb);
541541
ProcessManager::ptrace_unlink_tracee(pcb);
542542

543-
let parent_child_vpid = pcb.real_parent_pcb().and_then(|parent| {
544-
// A concurrently exiting parent may already have unhashed its
545-
// PID before this nonleader is autoreaped. Its children list
546-
// has then been consumed by the adoption transaction, so
547-
// there is no attached namespace/list entry left to clean.
548-
let parent_ns = parent
549-
.task_pid_ptr(PidType::PID)
550-
.and_then(|pid| pid.try_ns_of_pid())?;
551-
pcb.task_pid_nr_ns(PidType::PID, Some(parent_ns))
552-
.map(|vpid| (parent, vpid))
553-
});
543+
{
544+
// Pair parent discovery with children-index removal under the
545+
// same tasklist-like transaction used by reparent/de_thread.
546+
// Otherwise a concurrent identity handoff can move the list
547+
// entry after this release selected the former parent.
548+
let _relation_guard = crate::process::PTRACE_RELATION_LOCK.lock_irqsave();
549+
let parent_child_vpid = pcb.real_parent_pcb().and_then(|parent| {
550+
// A concurrently exiting parent may already have unhashed
551+
// its PID before this nonleader is autoreaped. Its children
552+
// list has then been consumed by the adoption transaction,
553+
// so no attached namespace/list entry remains to clean.
554+
let parent_ns = parent
555+
.task_pid_ptr(PidType::PID)
556+
.and_then(|pid| pid.try_ns_of_pid())?;
557+
pcb.task_pid_nr_ns(PidType::PID, Some(parent_ns))
558+
.map(|vpid| (parent, vpid))
559+
});
554560

555-
// Remove from the parent's children list.
556-
if let Some((parent, vpid)) = parent_child_vpid {
557-
let mut children = parent.children.write();
558-
children.retain(|&p| p != vpid);
561+
if let Some((parent, vpid)) = parent_child_vpid {
562+
let mut children = parent.children.write();
563+
children.retain(|&p| p != vpid);
564+
}
559565
}
560566

561567
// Revoke the old PCB's global numeric lookup before __exit_signal()

kernel/src/process/task.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,14 +1013,6 @@ impl ProcessControlBlock {
10131013
}
10141014
}
10151015

1016-
pub(crate) fn reparent_child_to(
1017-
child: &Arc<ProcessControlBlock>,
1018-
new_parent: &Arc<ProcessControlBlock>,
1019-
) {
1020-
let _relation_guard = PTRACE_RELATION_LOCK.lock_irqsave();
1021-
ProcessControlBlock::reparent_child_to_locked(child, new_parent);
1022-
}
1023-
10241016
fn reparent_child_to_locked(
10251017
child: &Arc<ProcessControlBlock>,
10261018
new_parent: &Arc<ProcessControlBlock>,
@@ -1045,7 +1037,12 @@ impl ProcessControlBlock {
10451037
}
10461038
}
10471039

1048-
pub(crate) fn reparent_child_links_from_thread_group(
1040+
/// Update a child's parent links during a thread-group identity handoff.
1041+
///
1042+
/// The caller must hold `PTRACE_RELATION_LOCK`, DragonOS's tasklist-like
1043+
/// relationship lock, so the parent pointers and the owning `children`
1044+
/// index change as one transaction with concurrent release/reparent.
1045+
pub(crate) fn reparent_child_links_from_thread_group_locked(
10491046
child: &Arc<ProcessControlBlock>,
10501047
old_tgid: RawPid,
10511048
new_parent: &Arc<ProcessControlBlock>,
@@ -1067,7 +1064,7 @@ impl ProcessControlBlock {
10671064
.unwrap_or(false);
10681065

10691066
if should_reparent {
1070-
ProcessControlBlock::reparent_child_to(child, new_parent);
1067+
ProcessControlBlock::reparent_child_to_locked(child, new_parent);
10711068
}
10721069
}
10731070

0 commit comments

Comments
 (0)