Skip to content

Commit 1ca50d7

Browse files
fix(task): fix fork heap inheritance and child_tid handling
Bug 1 - Fork not inheriting heap pointers: After fork, child's heap_bottom/heap_top were reset to USER_HEAP_BASE instead of inheriting parent's heap state, causing brk() to fail. Fix: Copy heap pointers from parent to child in fork path. Bug 2 - set_child_tid/clear_child_tid NULL pointer issues: When CLONE_CHILD_SETTID or CLONE_CHILD_CLEARTID flags were set but child_tid was NULL, kernel would access invalid memory. Fix: Check address is non-NULL before processing, pass as usize, write using aspace.write() in child's context. Bug 3 - execve not clearing clear_child_tid: After execve, clear_child_tid from old address space remained set, causing invalid memory access on process exit. Fix: Clear clear_child_tid after execve.
1 parent 97d7c9d commit 1ca50d7

File tree

3 files changed

+60
-25
lines changed

3 files changed

+60
-25
lines changed

api/src/syscall/task/clone.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,10 @@ pub fn sys_clone(
127127
}
128128
new_uctx.set_retval(0);
129129

130-
let set_child_tid = if flags.contains(CloneFlags::CHILD_SETTID) {
131-
Some(UserPtr::<u32>::from(child_tid).get_as_mut()?)
130+
// Pass child_tid address instead of pointer reference
131+
// Check both the flag and whether the address is non-NULL
132+
let set_child_tid = if flags.contains(CloneFlags::CHILD_SETTID) && child_tid != 0 {
133+
Some(child_tid)
132134
} else {
133135
None
134136
};
@@ -182,6 +184,9 @@ pub fn sys_clone(
182184
exit_signal,
183185
);
184186
proc_data.set_umask(old_proc_data.umask());
187+
// Inherit heap pointers from parent to ensure child's heap state is consistent after fork
188+
proc_data.set_heap_bottom(old_proc_data.get_heap_bottom());
189+
proc_data.set_heap_top(old_proc_data.get_heap_top());
185190

186191
{
187192
let mut scope = proc_data.scope.write();
@@ -215,7 +220,7 @@ pub fn sys_clone(
215220
}
216221

217222
let thr = Thread::new(tid, new_proc_data);
218-
if flags.contains(CloneFlags::CHILD_CLEARTID) {
223+
if flags.contains(CloneFlags::CHILD_CLEARTID) && child_tid != 0 {
219224
thr.set_clear_child_tid(child_tid);
220225
}
221226
*new_task.task_ext_mut() = Some(unsafe { AxTaskExt::from_impl(thr) });

api/src/syscall/task/execve.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ pub fn sys_execve(
6565

6666
*proc_data.signal.actions.lock() = Default::default();
6767

68+
// Clear set_child_tid after exec since the original address is no longer valid
69+
curr.as_thread().set_clear_child_tid(0);
70+
6871
// Close CLOEXEC file descriptors
6972
let mut fd_table = FD_TABLE.write();
7073
let cloexec_fds = fd_table

api/src/task.rs

Lines changed: 49 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ use axhal::uspace::{ExceptionKind, ReturnReason, UserContext};
55
use axtask::{TaskInner, current};
66
use bytemuck::AnyBitPattern;
77
use linux_raw_sys::general::ROBUST_LIST_LIMIT;
8+
use memory_addr::VirtAddr;
89
use starry_core::{
910
futex::FutexKey,
10-
mm::access_user_memory,
1111
shm::SHM_MANAGER,
1212
task::{
1313
AsThread, get_process_data, get_task, send_signal_to_process, send_signal_to_thread,
@@ -28,16 +28,26 @@ use crate::{
2828
pub fn new_user_task(
2929
name: &str,
3030
mut uctx: UserContext,
31-
set_child_tid: Option<&'static mut Pid>,
31+
set_child_tid: Option<usize>, // Pass address instead of pointer
3232
) -> TaskInner {
3333
TaskInner::new(
3434
move || {
3535
let curr = axtask::current();
36-
access_user_memory(|| {
37-
if let Some(tid) = set_child_tid {
38-
*tid = curr.id().as_u64() as Pid;
36+
37+
// Write set_child_tid in child task's context
38+
if let Some(tid_addr) = set_child_tid {
39+
if tid_addr != 0 {
40+
let thr = curr.as_thread();
41+
let aspace = thr.proc_data.aspace.lock();
42+
let tid_value = curr.id().as_u64() as u32;
43+
let tid_bytes = tid_value.to_ne_bytes();
44+
// Use address space's write method to directly write to user memory
45+
if let Err(e) = aspace.write(VirtAddr::from_usize(tid_addr), &tid_bytes) {
46+
warn!("Failed to write set_child_tid at {:#x}: {:?}", tid_addr, e);
47+
}
48+
drop(aspace);
3949
}
40-
});
50+
}
4151

4252
info!("Enter user space: ip={:#x}, sp={:#x}", uctx.ip(), uctx.sp());
4353

@@ -164,21 +174,31 @@ pub fn do_exit(exit_code: i32, group_exit: bool) {
164174

165175
info!("{} exit with code: {}", curr.id_name(), exit_code);
166176

167-
let clear_child_tid = thr.clear_child_tid() as *mut u32;
168-
if clear_child_tid.vm_write(0).is_ok() {
169-
let key = FutexKey::new_current(clear_child_tid as usize);
170-
let table = thr.proc_data.futex_table_for(&key);
171-
let guard = table.get(&key);
172-
if let Some(futex) = guard {
173-
futex.wq.wake(1, u32::MAX);
177+
let clear_child_tid = thr.clear_child_tid() as usize;
178+
// Try to write clear_child_tid, but first check if address is in valid address space.
179+
if clear_child_tid != 0 {
180+
let aspace = thr.proc_data.aspace.lock();
181+
// Check if address is in a valid area
182+
let addr_valid = aspace.contains_range(VirtAddr::from_usize(clear_child_tid), 4);
183+
drop(aspace);
184+
185+
if addr_valid {
186+
if let Ok(()) = (clear_child_tid as *mut u32).vm_write(0) {
187+
let key = FutexKey::new_current(clear_child_tid);
188+
let table = thr.proc_data.futex_table_for(&key);
189+
let guard = table.get(&key);
190+
if let Some(futex) = guard {
191+
futex.wq.wake(1, u32::MAX);
192+
}
193+
axtask::yield_now();
194+
}
174195
}
175-
axtask::yield_now();
176196
}
177197
let head = thr.robust_list_head() as *const RobustListHead;
178-
if !head.is_null()
179-
&& let Err(err) = exit_robust_list(head)
180-
{
181-
warn!("exit robust list failed: {err:?}");
198+
if !head.is_null() {
199+
if let Err(err) = exit_robust_list(head) {
200+
warn!("exit robust list failed: {err:?}");
201+
}
182202
}
183203

184204
let process = &thr.proc_data.proc;
@@ -213,11 +233,18 @@ pub fn raise_signal_fatal(sig: SignalInfo) -> AxResult<()> {
213233

214234
let signo = sig.signo();
215235
info!("Send fatal signal {signo:?} to the current process");
216-
if let Some(tid) = proc_data.signal.send_signal(sig)
217-
&& let Ok(task) = get_task(tid)
218-
{
219-
task.interrupt();
236+
let handled = if let Some(tid) = proc_data.signal.send_signal(sig) {
237+
if let Ok(task) = get_task(tid) {
238+
task.interrupt();
239+
true
240+
} else {
241+
false
242+
}
220243
} else {
244+
false
245+
};
246+
247+
if !handled {
221248
// No task wants to handle the signal, abort the task
222249
do_exit(signo as i32, true);
223250
}

0 commit comments

Comments
 (0)