Re: [PATCH RFC v3 02/10] sched_getattr: port to copy_struct_to_user

[Christian Brauner <brauner@kernel.org>]

On Tue, Dec 10, 2024 at 07:14:07PM +0100, Florian Weimer wrote:
> * Aleksa Sarai:
> 
> > sched_getattr(2) doesn't care about trailing non-zero bytes in the
> > (ksize > usize) case, so just use copy_struct_to_user() without checking
> > ignored_trailing.
> 
> I think this is what causes glibc's misc/tst-sched_setattr test to fail
> on recent kernels.  The previous non-modifying behavior was documented
> in the manual page:
> 
>        If the caller-provided attr buffer is larger than the kernel's
>        sched_attr structure, the additional bytes in the user-space
>        structure are not touched.
> 
> I can just drop this part of the test if the kernel deems both behaviors
> valid.

I think in general both behaviors are valid but I would consider zeroing
the unknown parts of the provided buffer to be the safer option. And all
newer extensible struct system calls do that.

But if sched_getattr(2) wants to keep its old behavior it wouldn't be a
problem to just handle this case:

diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
index 0d71fcbaf1e3..46140ec449ba 100644
--- a/kernel/sched/syscalls.c
+++ b/kernel/sched/syscalls.c
@@ -1126,6 +1126,15 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
        }

        kattr.size = min(usize, sizeof(kattr));
+       /*
+        * If userspace passed a larger structure than the kernel knows
+        * we historically didn't zero the unknown bits but
+        * copy_struct_to_user() will. Retain the old behavior by
+        * limiting the copy_to_user() to the size the kernel knows
+        * about.
+        */
+       if (usize > sizeof(kattr))
+               usize = sizeof(kattr);
        return copy_struct_to_user(uattr, usize, &kattr, sizeof(kattr), NULL);
 }