diff --git a/doc/kipc.adoc b/doc/kipc.adoc index b551d5fe3..1ca9f8884 100644 --- a/doc/kipc.adoc +++ b/doc/kipc.adoc @@ -260,6 +260,13 @@ the dump region index. If the dump region index is equal to or greater than the number of dump regions for the specified task, `None` will be returned. +Passing a dump region of 0 returns the task's descriptor in kernel memory, and +will always return `Some(..)` (for a valid task). Subsequent regions are +guaranteed to be sorted by (ascending) base address. + +CAUTION: The descriptor region may not be sorted with respect to other regions; +kernel memory may be either above or below task memory. + === `read_task_dump_region` (7) For a given task and task dump region, this will read the specified region and diff --git a/sys/userlib/src/kipc.rs b/sys/userlib/src/kipc.rs index 077104a50..7e9183982 100644 --- a/sys/userlib/src/kipc.rs +++ b/sys/userlib/src/kipc.rs @@ -63,9 +63,33 @@ pub fn find_faulted_task(task: usize) -> Option { NonZeroUsize::new(response as usize) } +/// Returns a [`TaskDumpRegion`](abi::TaskDumpRegion) for this task's descriptor +/// +/// The task descriptor is located in kernel memory. +pub fn get_task_desc_region(task: usize) -> abi::TaskDumpRegion { + // It is always valid to ask the kernel for the 0th dump region + get_task_dump_region_inner(task, 0).unwrap_lite() +} + +/// Returns the `i`'th dumpable region for the given task (or `None`) +/// +/// Dumpable regions are located in task RAM, and are returned in sorted +/// (ascending) order by base address. pub fn get_task_dump_region( task: usize, region: usize, +) -> Option { + // Region 0 is the task descriptor, so we add 1 here + get_task_dump_region_inner(task, region.checked_add(1)?) +} + +/// Access to the raw [`GetTaskDumpRegion`](Kipcnum::GetTaskDumpRegion) KIPC +/// +/// Wrapped by [`get_task_desc_region`] or [`get_task_dump_region`] for +/// higher-level semantics. +fn get_task_dump_region_inner( + task: usize, + region: usize, ) -> Option { let msg = (task as u32, region as u32); let mut buf = [0; core::mem::size_of::<(u32, u32)>()]; diff --git a/task/jefe/src/dump.rs b/task/jefe/src/dump.rs index 2b3dac598..0f06c1d5a 100644 --- a/task/jefe/src/dump.rs +++ b/task/jefe/src/dump.rs @@ -247,25 +247,11 @@ pub fn dump_task(base: u32, task: usize) -> Result { let area = dump_task_setup(base, DumpTaskContents::SingleTask)?; - for ndx in 0..=usize::MAX { - // - // We need to ask the kernel which regions we should dump for this - // task, which we do by asking for each dump region by index. Note - // that get_task_dump_region is O(#regions) -- which makes this loop - // quadratic: O(#regions * #dumpable). Fortunately, these numbers are - // very small: the number of regions is generally 3 or less (and -- as - // of this writing -- tops out at 7), and the number of dumpable - // regions is generally just one (two when including the task TCB, but - // that's constant time to extract). So this isn't as bad as it - // potentially looks (and boils down to two iterations over all - // regions in a task) -- but could become so if these numbers become - // larger. - // - let Some(region) = kipc::get_task_dump_region(task, ndx) else { - break; - }; + // Helper function to add a region to the dump + let add_dump_region = |region: abi::TaskDumpRegion| { + // Skip regions which are in the space used for raw dump data if in_dump_area(region.base, region.size) { - continue; + return Ok(()); } ringbuf_entry!(Trace::DumpRegion(region)); @@ -281,8 +267,28 @@ pub fn dump_task(base: u32, task: usize) -> Result { |addr, buf| unsafe { humpty::to_mem(addr, buf) }, ) { ringbuf_entry!(Trace::DumpRegionsFailed(e)); - return Err(DumpAgentError::BadSegmentAdd); + Err(DumpAgentError::BadSegmentAdd) + } else { + Ok(()) } + }; + + // We need to ask the kernel which regions we should dump for this task, + // which we do by asking for each dump region by index. Note that + // get_task_dump_region is O(#regions) -- which makes this loop quadratic: + // O(#regions * #dumpable). Fortunately, these numbers are very small: the + // number of regions is generally 3 or less (and -- as of this writing -- + // tops out at 7), and the number of dumpable regions is generally just one + // (two when including the task TCB, but that's constant time to extract). + // So this isn't as bad as it potentially looks (and boils down to two + // iterations over all regions in a task) -- but could become so if these + // numbers become larger. + add_dump_region(kipc::get_task_desc_region(task))?; + for ndx in 0..=usize::MAX { + let Some(region) = kipc::get_task_dump_region(task, ndx) else { + break; + }; + add_dump_region(region)?; } dump_task_run(area.region.address, task)?; @@ -323,24 +329,55 @@ pub fn dump_task_region( let mem = start..end; let mut okay = false; - for ndx in 0..=usize::MAX { - // This is Accidentally Quadratic; see the note in `dump_task` - let Some(region) = kipc::get_task_dump_region(task, ndx) else { - break; - }; - - if in_dump_area(region.base, region.size) { - continue; - } - - ringbuf_entry!(Trace::DumpRegion(region)); + // Get the task descriptor region (in kernel memory) + let desc = kipc::get_task_desc_region(task); + // Note: we implicitly trust kipc won't give us a region that wraps, so + // we'll use an unchecked add here (unlike untrusted user data from the + // request that we checked above) + let desc_region = desc.base..desc.base + desc.size; + if mem.start >= desc_region.start && mem.end <= desc_region.end { + // We are reading from the kernel descriptor region, great job + okay = true; + } else { + // Otherwise, iterate over task regions. We will start with `mem` + // representing the full memory range to be dumped, then adjust + // `mem.start` as we find overlaps within the task dump regions. If + // `mem` becomes empty, then we know that it is valid (because the + // entire `mem` region has overlapped with task dump regions). + // + // Note: we also implicitly trust that kipc gives us regions which are + // in sorted order by base address. + let mut mem = start..end; + let mut started = false; + for ndx in 0..=usize::MAX { + // This is Accidentally Quadratic; see the note in `dump_task` + let Some(region) = kipc::get_task_dump_region(task, ndx) else { + break; + }; + + if in_dump_area(region.base, region.size) { + continue; + } - // Note: we implicitly trust kipc won't give us a region that wraps, - // unlike untrusted user data from the request that we checked above. - let region = region.base..region.base + region.size; - if mem.start >= region.start && mem.end <= region.end { - okay = true; - break; + ringbuf_entry!(Trace::DumpRegion(region)); + + // Slide `mem.start` based on overlap + let region = region.base..region.base + region.size; + if region.contains(&mem.start) { + mem.start = region.end.min(mem.end); + started = true; + if mem.start == mem.end { + okay = true; + break; + } + } else if region.start > mem.start + || (started && region.start < mem.start) + { + // If we are beyond the start of our `mem` region (or have + // started overlapping but this region does not overlap), then + // there are no more overlaps and we can bail out immediately. + break; + } } }