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

From: Christian Brauner
Date: Wed Dec 11 2024 - 05:24:10 EST


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);
}