[Bug 219525] Multiple bugs in mpr ioctl handler

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219525

--- Comment #2 from CTurt <ecturt at gmail.com> ---
Thanks for taking a look.

To clarify the point you asked about:

`recorded_events` is a fixed size, it looks like this in `struct mpr_softc`:

mpr_event_entry_t               recorded_events[MPR_EVENT_QUEUE_SIZE];

The kernel should copy out only `sizeof(sc->recored_events)` bytes at most from
here, or else it will copy out of bounds memory to userland.

Going back to the code:

        size = data->Size;
        if ((size >= sizeof(sc->recorded_events)) && (status == 0)) {
            mpr_unlock(sc);
            if (copyout((void *)sc->recorded_events,
                PTRIN(data->PtrEvents), size) != 0)
                status = EFAULT;
            mpr_lock(sc);
        }

Notice that it passes `size` to `copyout`, which can be greater than
`sizeof(sc->recorded_events)` (see the check).

Instead, the `copyout` call should be passed `sizeof(sc->recorded_events)` so
that it won't read more than the array contains.

So, you are correct when you say that it "is just checking if the user supplied
size is enough to hold sizeof(sc->recored_events)". But this check won't
prevent copying out of bounds memory to userland.

-- 
You are receiving this mail because:
You are the assignee for the bug.