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

--- Comment #2 from CTurt <[email protected]> ---
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.
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "[email protected]"

Reply via email to