rust-vmm / vm-memory

Virtual machine's guest memory crate
Apache License 2.0
299 stars 97 forks source link

GuestMemory::try_access() Invariant not Checked #269

Closed JonathanWoollett-Light closed 9 months ago

JonathanWoollett-Light commented 9 months ago

From @y-x41

X41 found that the trait GuestMemory defined in the vm-memory crate exposes try_access(), which takes a callback function as parameter. The method expects the callback to adhere to the invariant to return a size whose value is between 0 and the also provided len parameter. However, the callback function may violate this and return lengths higher than the provided length, which is not checked for. The below listing shows the code responsible for handling the return value of the callback. The variable cur stores the current address, which after adding an arbitrary length exceeding count, may still be valid and thus result in a hole in the guest memory map.

match f(total, len as usize, start, region) {
    Ok(0) => return Ok(total),
    Ok(len) => {
        total += len;
        // `total` may exceed `count` if the callback breaks the invariant
        // The following condition is false, when total > count
        if total == count {
            break;
        }
        cur = match cur.overflowing_add(len as GuestUsize) {
            (GuestAddress(0), _) => GuestAddress(0),
            (result, false) => result,
            (_, true) => panic!("guest address overflow"),
        }
        // We my still end up with a valid `cur` address, but may have
        // skipped a hole.
    }
    e => return e,
}

X41 recommends to produce a runtime error or panic when the provided callback does not observe the invariant to let the implementers know of a bug.