Skip to content

Commit cbe66e3

Browse files
committed
Fix partial-transfer retry window in process_rw
After a partial process_vm_readv/writev, the retry syscall resumes at iov offset `offset`, but the result-dispatch loop iterated the local/ remote iovecs and temp_meta from index 0. On every pass after the first, already-reported entries were re-reported with the wrong metadata and local slices, the entries the retry actually transferred were never reported, byte accounting used the wrong iov_lens, and out_fail flagged the wrong element. This corrupted result attribution for any batched read/write spanning an unmapped hole. Align dispatch and accounting with the syscall window by skipping the first `win` entries on all three iterators. Add a regression test that batches [valid, unmapped, valid] reads against our own PID; it fails against the previous code (first region duplicated, third dropped).
1 parent 2dac6c4 commit cbe66e3

1 file changed

Lines changed: 96 additions & 5 deletions

File tree

src/linux/mem.rs

Lines changed: 96 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,18 @@ impl ProcessVirtualMemory {
181181
let mut remaining_written =
182182
if libcret == -1 { 0 } else { libcret as usize };
183183

184-
for (liof, (_, meta)) in iov_local
185-
.iter()
186-
.take(cnt)
187-
.zip(iov_remote.iter().zip(self.temp_meta.iter()))
188-
{
184+
// The syscall above operated on the window [win, win + cnt),
185+
// so result dispatch and byte accounting must start at `win`
186+
// too. `offset` is advanced inside the loop, so snapshot it
187+
// before iterating.
188+
let win = offset;
189+
190+
for (liof, (_, meta)) in iov_local.iter().skip(win).take(cnt).zip(
191+
iov_remote
192+
.iter()
193+
.skip(win)
194+
.zip(self.temp_meta.iter().skip(win)),
195+
) {
189196
offset += 1;
190197
let to_write = remaining_written;
191198

@@ -250,3 +257,87 @@ impl MemoryView for ProcessVirtualMemory {
250257
}
251258
}
252259
}
260+
261+
#[cfg(test)]
262+
mod tests {
263+
use super::*;
264+
use memflow::cglue::CTup2;
265+
266+
fn vmem_for_pid(pid: pid_t) -> ProcessVirtualMemory {
267+
const IOV_MAX: usize = 1024;
268+
ProcessVirtualMemory {
269+
pid,
270+
temp_iov: vec![
271+
IoSendVec(iovec {
272+
iov_base: std::ptr::null_mut::<c_void>(),
273+
iov_len: 0,
274+
});
275+
IOV_MAX * 2
276+
]
277+
.into_boxed_slice(),
278+
temp_meta: vec![Address::INVALID; IOV_MAX].into_boxed_slice(),
279+
}
280+
}
281+
282+
// Regression test for the partial-transfer retry window in `process_rw`.
283+
//
284+
// A batched read of [valid, unmapped, valid] forces `process_vm_readv` to transfer
285+
// the first region, fault on the middle one, and require a retry for the third.
286+
// Before the `.skip(win)` fix the retry dispatched from index 0, re-reporting the
287+
// first region and silently dropping the third. Reading from our own PID lets us
288+
// exercise this without spawning a child.
289+
#[test]
290+
fn partial_read_across_hole_reports_each_region_once() {
291+
let src_a = [0xAAu8; 8];
292+
let src_c = [0xCCu8; 8];
293+
294+
let addr_a = Address::from(src_a.as_ptr() as u64);
295+
let addr_c = Address::from(src_c.as_ptr() as u64);
296+
// Below the default mmap_min_addr, so reliably unmapped (EFAULT on read).
297+
let addr_bad = Address::from(0x1000u64);
298+
299+
let mut dst_a = [0u8; 8];
300+
let mut dst_b = [0u8; 8];
301+
let mut dst_c = [0u8; 8];
302+
303+
let mut ok: Vec<(Address, Vec<u8>)> = Vec::new();
304+
let mut fail: Vec<Address> = Vec::new();
305+
306+
{
307+
let inp = vec![
308+
CTup2(addr_a, (&mut dst_a[..]).into()),
309+
CTup2(addr_bad, (&mut dst_b[..]).into()),
310+
CTup2(addr_c, (&mut dst_c[..]).into()),
311+
];
312+
313+
let mut ok_cb = |CTup2(a, d): ReadData| {
314+
ok.push((a, d.to_vec()));
315+
true
316+
};
317+
let mut fail_cb = |CTup2(a, _): ReadData| {
318+
fail.push(a);
319+
true
320+
};
321+
let mut ok_oc: ReadCallback = (&mut ok_cb).into();
322+
let mut fail_oc: ReadCallback = (&mut fail_cb).into();
323+
324+
let mut mem = vmem_for_pid(unsafe { libc::getpid() });
325+
mem.read_iter(inp.into_iter(), Some(&mut ok_oc), Some(&mut fail_oc))
326+
.unwrap();
327+
}
328+
329+
ok.sort_by_key(|(a, _)| a.to_umem());
330+
let mut expected = vec![(addr_a, vec![0xAAu8; 8]), (addr_c, vec![0xCCu8; 8])];
331+
expected.sort_by_key(|(a, _)| a.to_umem());
332+
333+
assert_eq!(
334+
ok, expected,
335+
"each readable region must be reported exactly once with correct data"
336+
);
337+
assert_eq!(
338+
fail,
339+
vec![addr_bad],
340+
"the unmapped region must be the only failure"
341+
);
342+
}
343+
}

0 commit comments

Comments
 (0)