On Thu, 10 Oct 2024 at 10:46, Christian Brauner <brauner@xxxxxxxxxx> wrote: > > On Thu, Oct 10, 2024 at 07:50:36AM +1100, Aleksa Sarai wrote: > > On 2024-10-08, luca.boccassi@xxxxxxxxx <luca.boccassi@xxxxxxxxx> wrote: > > > From: Luca Boccassi <luca.boccassi@xxxxxxxxx> > > > + /* > > > + * If userspace and the kernel have the same struct size it can just > > > + * be copied. If userspace provides an older struct, only the bits that > > > + * userspace knows about will be copied. If userspace provides a new > > > + * struct, only the bits that the kernel knows about will be copied and > > > + * the size value will be set to the size the kernel knows about. > > > + */ > > > + if (copy_to_user(uinfo, &kinfo, min(usize, sizeof(kinfo)))) > > > + return -EFAULT; > > > > If usize > ksize, we also want to clear_user() the trailing bytes to > > avoid userspace thinking that any garbage bytes they had are valid. > > > > Also, you mention "the size value" but there is no size in pidfd_info. I > > don't think it's actually necessary to include such a field (especially > > when you have a statx-like request_mask), but it means you really should > > clear the trailing bytes to avoid userspace bugs. > > > > I implemented all of these semantics as copy_struct_to_user() in the > > CHECK_FIELDS patch I sent a few weeks ago (I just sent v3[1]). Maybe you > > can cherry-pick this patch and use it? The semantics when we extend this > > pidfd_info to accept new request_mask values with larger structures is > > going to get a little ugly and copy_struct_to_user() makes this a little > > easier to deal with. > > > > [1]: https://lore.kernel.org/all/20241010-extensible-structs-check_fields-v3-1-d2833dfe6edd@xxxxxxxxxx/ > > I agree. @Luca, you can either send the two patches together or I can > just port the patch to it. I don't mind. I've updated for the latter, given that series is not merged yet, thanks.
