> The documentation of struct pidfd_info promises that "If the structure
> size is too small to contain a field (requested or not), to avoid
> confusion the mask will not contain a bit for that field", and asks
> userspace to trust the returned mask when deciding which fields hold
> valid data.
>
> pidfd_info() does not keep that promise. It builds kinfo.mask without
> looking at the size encoded in the ioctl command, and
> copy_struct_to_user() then truncates the copy to that size. A caller
> that passes a 64-byte struct (PIDFD_INFO_SIZE_VER0) and requests
> PIDFD_INFO_COREDUMP or PIDFD_INFO_SUPPORTED_MASK gets a mask with
> PIDFD_INFO_COREDUMP, PIDFD_INFO_COREDUMP_SIGNAL, PIDFD_INFO_COREDUMP_CODE
> and PIDFD_INFO_SUPPORTED_MASK set, while the coredump_* and
> supported_mask fields behind the 64 bytes were never written. Trusting
> the mask as documented, userspace then reads whatever its own buffer
> held at those offsets. The same happens for a 72- or 80-byte struct
> and the fields added after it.
>
> Clear the bits of the fields that lie beyond the caller's size before
> the copy, in the one place that knows both the final mask and the size.
> A request for a field that cannot be returned is thereby answered the
> way the header describes: the field is simply absent from the mask.
>
> The added selftest issues PIDFD_GET_INFO with the first published size
> over a full-size, poisoned struct and checks that the mask names none
> of the fields beyond that size and that the poison is intact. It fails
> on an unpatched kernel and passes with this change.
>
> Found by formal verification of fs/pidfs.c against the kernel binary.
>
> Fixes: 1d8db6fd698d ("pidfs, coredump: add PIDFD_INFO_COREDUMP")
> Signed-off-by: Nickolai Zeldovich <[email protected]>
> Signed-off-by: Christian Brauner (Amutable) <[email protected]>
> ---
> Notes:
> The three size thresholds follow the field layout: coredump_mask and
> coredump_signal end at PIDFD_INFO_SIZE_VER1 (72), coredump_code at
> PIDFD_INFO_SIZE_VER2 (80), supported_mask at PIDFD_INFO_SIZE_VER3 (88).
> Masking the request bits early instead (before the coredump and
> supported_mask blocks run) would save a little work for such callers
> but leaves the bits set in the task->mm fallback path; clearing the
> result once before copy_struct_to_user() covers every way a bit can be
> set. No stable tag: the only callers affected pass a struct size
> smaller than the fields they request, and the consequence is a wrong
> mask bit, not a crash or corruption.
>
> fs/pidfs.c | 8 ++++
> tools/testing/selftests/pidfd/pidfd.h | 16 +++++++
> .../testing/selftests/pidfd/pidfd_info_test.c | 48 +++++++++++++++++++
Semantic changes and selftest changes in separate patches, please.
--
