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

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

[Florian Weimer]

* 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

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

[Christian Brauner]

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

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

[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.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

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

[Florian Weimer]

* 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

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

[Xi Ruoyao]

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

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

[Florian Weimer]

* 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