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.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
kernel/sched/syscalls.c | 42 ++----------------------------------------
1 file changed, 2 insertions(+), 40 deletions(-)
diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
index ae1b42775ef9..4ccc058bae16 100644
--- a/kernel/sched/syscalls.c
+++ b/kernel/sched/syscalls.c
@@ -1147,45 +1147,6 @@ SYSCALL_DEFINE2(sched_getparam, pid_t, pid, struct sched_param __user *, param)
return copy_to_user(param, &lp, sizeof(*param)) ? -EFAULT : 0;
}
-/*
- * Copy the kernel size attribute structure (which might be larger
- * than what user-space knows about) to user-space.
- *
- * Note that all cases are valid: user-space buffer can be larger or
- * smaller than the kernel-space buffer. The usual case is that both
- * have the same size.
- */
-static int
-sched_attr_copy_to_user(struct sched_attr __user *uattr,
- struct sched_attr *kattr,
- unsigned int usize)
-{
- unsigned int ksize = sizeof(*kattr);
-
- if (!access_ok(uattr, usize))
- return -EFAULT;
-
- /*
- * sched_getattr() ABI forwards and backwards compatibility:
- *
- * If usize == ksize then we just copy everything to user-space and all is good.
- *
- * If usize < ksize then we only copy as much as user-space has space for,
- * this keeps ABI compatibility as well. We skip the rest.
- *
- * If usize > ksize then user-space is using a newer version of the ABI,
- * which part the kernel doesn't know about. Just ignore it - tooling can
- * detect the kernel's knowledge of attributes from the attr->size value
- * which is set to ksize in this case.
- */
- kattr->size = min(usize, ksize);
-
- if (copy_to_user(uattr, kattr, kattr->size))
- return -EFAULT;
-
- return 0;
-}
-
/**
* sys_sched_getattr - similar to sched_getparam, but with sched_attr
* @pid: the pid in question.
@@ -1230,7 +1191,8 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
#endif
}
- return sched_attr_copy_to_user(uattr, &kattr, usize);
+ kattr.size = min(usize, sizeof(kattr));
+ return copy_struct_to_user(uattr, usize, &kattr, sizeof(kattr), NULL);
}
#ifdef CONFIG_SMP
--
2.46.1* 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.
Thanks,
Florian
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);
}
On Wed, 2024-12-11 at 11:23 +0100, Christian Brauner wrote: > 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. Florian, So should we drop the test before Glibc-2.41 release? I'm seeing the failure during my machine test. -- Xi Ruoyao <xry111@xry111.site> School of Aerospace Science and Technology, Xidian University
* Xi Ruoyao: > On Wed, 2024-12-11 at 11:23 +0100, Christian Brauner wrote: >> 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. > > Florian, > > So should we drop the test before Glibc-2.41 release? I'm seeing the > failure during my machine test. I was waiting for a verdict from the kernel developers. I didn't expect such a change to happen given the alleged UAPI policy. Thanks, Florian
On Mon, 2025-01-20 at 06:28 +0100, Florian Weimer wrote: > * Xi Ruoyao: > > > On Wed, 2024-12-11 at 11:23 +0100, Christian Brauner wrote: > > > 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. > > > > Florian, > > > > So should we drop the test before Glibc-2.41 release? I'm seeing the > > failure during my machine test. > I was waiting for a verdict from the kernel developers. I didn't expect > such a change to happen given the alleged UAPI policy. But 6.13 is already released without reverting the behavior change now... So is this the "final" verdict? -- Xi Ruoyao <xry111@xry111.site> School of Aerospace Science and Technology, Xidian University
* Xi Ruoyao: >> > So should we drop the test before Glibc-2.41 release? I'm seeing the >> > failure during my machine test. >> I was waiting for a verdict from the kernel developers. I didn't expect >> such a change to happen given the alleged UAPI policy. > > But 6.13 is already released without reverting the behavior change > now... So is this the "final" verdict? I had originally missed the Linux 6.13 release. I've submitted a glibc patch: [PATCH] Linux: Do not check unused bytes after sched_getattr in tst-sched_setattr <https://inbox.sourceware.org/libc-alpha/87v7u9ddas.fsf@oldenburg.str.redhat.com/> Thanks, Florian
© 2016 - 2026 Red Hat, Inc.