Skip to content

Commit 83e2e08

Browse files
committed
fix: harden x86 decode at page boundaries
Use OS-backed memory reads for instruction-width decoding so page-end patchpoints no longer crash when the next page is unreadable. Add x86 runtime tests for unreadable/readable cross-page cases and run full tests in x86 CI jobs.
1 parent bd77d4c commit 83e2e08

2 files changed

Lines changed: 186 additions & 7 deletions

File tree

.github/workflows/ci.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ jobs:
110110
run: cargo check --all-targets --target x86_64-apple-darwin
111111

112112
- name: Cargo test
113-
run: cargo test --doc --target x86_64-apple-darwin
113+
run: cargo test --target x86_64-apple-darwin
114114

115115
- name: Cargo clippy
116116
run: cargo clippy --all-targets --target x86_64-apple-darwin -- -D warnings
@@ -191,7 +191,7 @@ jobs:
191191
run: cargo check --all-targets --target x86_64-unknown-linux-gnu
192192

193193
- name: Cargo test
194-
run: cargo test --doc --target x86_64-unknown-linux-gnu
194+
run: cargo test --target x86_64-unknown-linux-gnu
195195

196196
- name: Cargo clippy
197197
run: cargo clippy --all-targets --target x86_64-unknown-linux-gnu -- -D warnings

src/memory.rs

Lines changed: 184 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,17 @@ unsafe extern "C" {
1616
) -> libc::kern_return_t;
1717
}
1818

19+
#[cfg(all(target_os = "macos", target_arch = "x86_64"))]
20+
unsafe extern "C" {
21+
fn mach_vm_read_overwrite(
22+
target_task: libc::vm_map_t,
23+
address: libc::mach_vm_address_t,
24+
size: libc::mach_vm_size_t,
25+
data: libc::mach_vm_address_t,
26+
outsize: *mut libc::mach_vm_size_t,
27+
) -> libc::kern_return_t;
28+
}
29+
1930
#[cfg(all(any(target_os = "macos", target_os = "ios"), target_arch = "aarch64"))]
2031
unsafe extern "C" {
2132
fn sys_icache_invalidate(start: *mut c_void, len: usize);
@@ -116,12 +127,9 @@ pub(crate) fn instruction_width(_address: u64) -> Result<u8, SigHookError> {
116127
pub(crate) fn instruction_width(address: u64) -> Result<u8, SigHookError> {
117128
use iced_x86::{Decoder, DecoderOptions};
118129

119-
let mut bytes = [0u8; 15];
120-
unsafe {
121-
std::ptr::copy_nonoverlapping(address as *const u8, bytes.as_mut_ptr(), bytes.len());
122-
}
130+
let (bytes, available_len) = read_decode_window_x86(address)?;
123131

124-
let mut decoder = Decoder::with_ip(64, &bytes, address, DecoderOptions::NONE);
132+
let mut decoder = Decoder::with_ip(64, &bytes[..available_len], address, DecoderOptions::NONE);
125133
let instruction = decoder.decode();
126134
if instruction.is_invalid() {
127135
return Err(SigHookError::DecodeFailed);
@@ -130,6 +138,84 @@ pub(crate) fn instruction_width(address: u64) -> Result<u8, SigHookError> {
130138
Ok(instruction.len() as u8)
131139
}
132140

141+
#[cfg(all(target_arch = "x86_64", any(target_os = "linux", target_os = "macos")))]
142+
fn read_decode_window_x86(address: u64) -> Result<([u8; 15], usize), SigHookError> {
143+
if address == 0 {
144+
return Err(SigHookError::InvalidAddress);
145+
}
146+
147+
let page_size = page_size()?;
148+
let addr = address as usize;
149+
let mut bytes = [0u8; 15];
150+
let page_offset = addr % page_size;
151+
let first_chunk_len = bytes.len().min(page_size - page_offset);
152+
read_memory_chunk_x86(addr, &mut bytes[..first_chunk_len])?;
153+
154+
let mut total_len = first_chunk_len;
155+
if first_chunk_len < bytes.len() {
156+
let second_chunk_addr = addr
157+
.checked_add(first_chunk_len)
158+
.ok_or(SigHookError::InvalidAddress)?;
159+
let second_chunk_len = bytes.len() - first_chunk_len;
160+
if read_memory_chunk_x86(
161+
second_chunk_addr,
162+
&mut bytes[first_chunk_len..first_chunk_len + second_chunk_len],
163+
)
164+
.is_ok()
165+
{
166+
total_len += second_chunk_len;
167+
}
168+
}
169+
170+
Ok((bytes, total_len))
171+
}
172+
173+
#[cfg(all(target_arch = "x86_64", target_os = "linux"))]
174+
fn read_memory_chunk_x86(address: usize, out: &mut [u8]) -> Result<(), SigHookError> {
175+
if out.is_empty() {
176+
return Ok(());
177+
}
178+
179+
let local = libc::iovec {
180+
iov_base: out.as_mut_ptr().cast::<c_void>(),
181+
iov_len: out.len(),
182+
};
183+
let remote = libc::iovec {
184+
iov_base: address as *mut c_void,
185+
iov_len: out.len(),
186+
};
187+
188+
let read_size = unsafe { libc::process_vm_readv(libc::getpid(), &local, 1, &remote, 1, 0) };
189+
if read_size < 0 || read_size as usize != out.len() {
190+
return Err(SigHookError::InvalidAddress);
191+
}
192+
193+
Ok(())
194+
}
195+
196+
#[cfg(all(target_arch = "x86_64", target_os = "macos"))]
197+
fn read_memory_chunk_x86(address: usize, out: &mut [u8]) -> Result<(), SigHookError> {
198+
if out.is_empty() {
199+
return Ok(());
200+
}
201+
202+
let mut out_size: libc::mach_vm_size_t = 0;
203+
let kr = unsafe {
204+
mach_vm_read_overwrite(
205+
libc::mach_task_self(),
206+
address as libc::mach_vm_address_t,
207+
out.len() as libc::mach_vm_size_t,
208+
out.as_mut_ptr() as libc::mach_vm_address_t,
209+
&mut out_size,
210+
)
211+
};
212+
if kr != 0 || out_size as usize != out.len() {
213+
return Err(SigHookError::InvalidAddress);
214+
}
215+
216+
Ok(())
217+
}
218+
133219
#[cfg(all(target_arch = "x86_64", any(target_os = "linux", target_os = "macos")))]
134220
#[inline]
135221
pub(crate) fn int3_opcode() -> u8 {
@@ -350,6 +436,29 @@ pub(crate) fn encode_absolute_jump(to_address: u64) -> [u8; 14] {
350436
mod tests {
351437
use super::protect_range_start_len;
352438

439+
#[cfg(all(target_arch = "x86_64", any(target_os = "linux", target_os = "macos")))]
440+
use super::{instruction_width, page_size};
441+
442+
#[cfg(all(target_arch = "x86_64", any(target_os = "linux", target_os = "macos")))]
443+
use crate::error::SigHookError;
444+
445+
#[cfg(all(target_arch = "x86_64", any(target_os = "linux", target_os = "macos")))]
446+
use libc::c_void;
447+
448+
#[cfg(all(target_arch = "x86_64", any(target_os = "linux", target_os = "macos")))]
449+
fn map_two_pages(page_size: usize) -> *mut c_void {
450+
unsafe {
451+
libc::mmap(
452+
std::ptr::null_mut(),
453+
page_size * 2,
454+
libc::PROT_READ | libc::PROT_WRITE,
455+
libc::MAP_PRIVATE | libc::MAP_ANON,
456+
-1,
457+
0,
458+
)
459+
}
460+
}
461+
353462
#[test]
354463
fn protect_range_single_page() {
355464
let page_size = 0x1000;
@@ -366,6 +475,76 @@ mod tests {
366475
assert_eq!(len, 0x2000);
367476
}
368477

478+
#[cfg(all(target_arch = "x86_64", any(target_os = "linux", target_os = "macos")))]
479+
#[test]
480+
fn instruction_width_page_end_with_unmapped_next_page() {
481+
let page_size = page_size().expect("page size should be available");
482+
let mapping = map_two_pages(page_size);
483+
assert_ne!(mapping, libc::MAP_FAILED);
484+
485+
let first_page_base = mapping as *mut u8;
486+
let second_page = unsafe { first_page_base.add(page_size) } as *mut c_void;
487+
assert_eq!(
488+
unsafe { libc::mprotect(second_page, page_size, libc::PROT_NONE) },
489+
0
490+
);
491+
492+
let patchpoint = unsafe { first_page_base.add(page_size - 1) };
493+
unsafe {
494+
std::ptr::write_volatile(patchpoint, 0x90);
495+
}
496+
497+
let len = instruction_width(patchpoint as u64).expect("decode should succeed");
498+
assert_eq!(len, 1);
499+
500+
assert_eq!(unsafe { libc::munmap(mapping, page_size * 2) }, 0);
501+
}
502+
503+
#[cfg(all(target_arch = "x86_64", any(target_os = "linux", target_os = "macos")))]
504+
#[test]
505+
fn instruction_width_page_end_incomplete_instruction_returns_error() {
506+
let page_size = page_size().expect("page size should be available");
507+
let mapping = map_two_pages(page_size);
508+
assert_ne!(mapping, libc::MAP_FAILED);
509+
510+
let first_page_base = mapping as *mut u8;
511+
let second_page = unsafe { first_page_base.add(page_size) } as *mut c_void;
512+
assert_eq!(
513+
unsafe { libc::mprotect(second_page, page_size, libc::PROT_NONE) },
514+
0
515+
);
516+
517+
let patchpoint = unsafe { first_page_base.add(page_size - 1) };
518+
unsafe {
519+
std::ptr::write_volatile(patchpoint, 0x0F);
520+
}
521+
522+
let err = instruction_width(patchpoint as u64).expect_err("decode should fail");
523+
assert_eq!(err, SigHookError::DecodeFailed);
524+
525+
assert_eq!(unsafe { libc::munmap(mapping, page_size * 2) }, 0);
526+
}
527+
528+
#[cfg(all(target_arch = "x86_64", any(target_os = "linux", target_os = "macos")))]
529+
#[test]
530+
fn instruction_width_cross_page_when_next_page_readable() {
531+
let page_size = page_size().expect("page size should be available");
532+
let mapping = map_two_pages(page_size);
533+
assert_ne!(mapping, libc::MAP_FAILED);
534+
535+
let first_page_base = mapping as *mut u8;
536+
let patchpoint = unsafe { first_page_base.add(page_size - 2) };
537+
let insn = [0xE9, 0x01, 0x00, 0x00, 0x00];
538+
unsafe {
539+
std::ptr::copy_nonoverlapping(insn.as_ptr(), patchpoint, insn.len());
540+
}
541+
542+
let len = instruction_width(patchpoint as u64).expect("decode should succeed");
543+
assert_eq!(len, 5);
544+
545+
assert_eq!(unsafe { libc::munmap(mapping, page_size * 2) }, 0);
546+
}
547+
369548
#[cfg(any(target_os = "macos", target_os = "ios"))]
370549
#[test]
371550
fn executable_restore_protections_match_target() {

0 commit comments

Comments
 (0)