misaligned pointer dereference

[multimeric]

When I run the example code, I get:

misaligned pointer dereference: address must be a multiple of 0x8 but is 0x567953308bf4

Backtrace:

stack backtrace:
   0: rust_begin_unwind
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panicking.rs:652:5
   1: core::panicking::panic_nounwind_fmt::runtime
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/panicking.rs:110:18
   2: core::panicking::panic_nounwind_fmt
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/panicking.rs:120:5
   3: core::panicking::panic_misaligned_pointer_dereference
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/panicking.rs:287:5
   4: cnproc::parse_msg
             at /home/migwell/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cnproc-0.2.1/src/lib.rs:204:23
   5: cnproc::PidMonitor::get_events
             at /home/migwell/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cnproc-0.2.1/src/lib.rs:167:54
   6: cnproc::PidMonitor::recv
             at /home/migwell/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cnproc-0.2.1/src/lib.rs:186:19
   7: cnproc_monitor::main
             at ./src/main.rs:4:15
   8: core::ops::function::FnOnce::call_once
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/ops/function.rs:250:5

I guess this has something to do with alignment of the C structs, and my system happens to hit an edge case. In my own <linux/connector.h>, I have:

struct proc_event {
        enum proc_cn_event what;
        __u32 cpu;
        __u64 __attribute__((aligned(8))) timestamp_ns;
       union { ... }
}

However the bindgen doesn't seem to capture this:

cnproc-rs/src/binding.rs

Lines 1072 to 1079 in 3d8f058

	#[repr(C)]
	#[derive(Copy, Clone)]
	pub struct proc_event {
	pub what: proc_event_what,
	pub cpu: __u32,
	pub timestamp_ns: __u64,
	pub event_data: proc_event__bindgen_ty_1,
	}

[rustybrooks]

I get this also. This is a new check in rust that was previous undefined behavior, I think. Newly released in 1.7

I don't know a lot about it but it seems like one way to solve the problem is using read_unaligned
https://doc.rust-lang.org/std/ptr/fn.read_unaligned.html

I believe that in the function with the problem, you could possibly replace the *proc_ev with proc_ev.read_unaligned()?

I might try this if I get a minute and submit a PR

unsafe fn parse_msg(header: *const nlmsghdr) -> Option<PidEvent> {
    let msg = (header as usize + nlmsg_length(0)) as *const cn_msg;
    if (*msg).id.idx != binding::CN_IDX_PROC || (*msg).id.val != binding::CN_VAL_PROC {
        return None;
    };
    let proc_ev = (*msg).data.as_ptr() as *const binding::proc_event;
    match (*proc_ev).what {
        binding::PROC_EVENT_FORK => {
            let pid = (*proc_ev).event_data.fork.child_pid;
            Some(PidEvent::Fork(pid))
        }
        binding::PROC_EVENT_EXEC => {
            let pid = (*proc_ev).event_data.exec.process_pid;
            Some(PidEvent::Exec(pid))
        }
        binding::PROC_EVENT_EXIT => {
            let pid = (*proc_ev).event_data.exit.process_pid;
            Some(PidEvent::Exit(pid))
        }
        binding::PROC_EVENT_COREDUMP => {
            let pid = (*proc_ev).event_data.coredump.process_pid;
            Some(PidEvent::Coredump(pid))
        }
        _ => None,
    }
}

[ydirson]

I believe that in the function with the problem, you could possibly replace the *proc_ev with proc_ev.read_unaligned()?

We don't have direct control of proc_ev here, it is defined by

1. the address of the buffer allocated in get_events, which is aligned aligned on 4-byte boundary since we allocate it as a Vec<u32>
2. the offset of the data field, with cn_msg of size 20, which also makes id aligned on 4-bytes boundary but not 8.

The assertion is about being aligned to 8-bytes boundary, which makes it necessary for buffer to be specifically not aligned to 8-bytes boundary.

[rustybrooks]

`We don't have direct control of proc_ev here, it is defined by

Sure, but the actual problem happens when you try to reference *proc_ev

You can replace every instance of that with proc_ev.read_unaligned()

I haven't looked into it enough to see if there's a concern here, i.e. if the alignment problem represents some kind of underlying bug, but, doing this should in theory be the same behavior as before Rust changed their alignment rules in derefs (which also could have been incorrect, but in my (limited) experimentation seems to work OK)

I'll submit a PR after xmas and maybe some other people can try it out or let me know if something is wrong with it

[ydirson]

You can replace every instance of that with proc_ev.read_unaligned()

It does prevent the alignment problem, but replacing every pointer dereference by a bitwise copy of the entire struct, just to each time access one field, is a bit of a waste. It is a bit better to use a single let proc_ev_copy = proc_ev.read_unaligned(); and use that instead of dereferencing the pointer several times.

But then there is still an extra copy involved, which maybe we could get rid of by getting a proper alignment to start with.

Or shouldn't we rely on a proven deserialization library instead of all this manual unsafe parsing?

[ydirson]

I have a fix for the alignment issue, but I'm not satisfied with it either: it really looks like a case where we should rather want to use scatter/gather recvmsg to fill the successive header layers into naturally-aligned structs. Also it feels weird to use unsafe { libc::recv() } rather than std::net.