Re: git: bb5f10e51bd5 - main - linux: implement sched_getattr()

From: Enji Cooper (yaneurabeya) <yaneurabeya_at_gmail.com>
Date: Tue, 06 Oct 2026 19:23:49 UTC
> On Oct 6, 2026, at 11:36 AM, John Baldwin <jhb@FreeBSD.org> wrote:
> 
> On 10/6/26 10:42, Enji Cooper (yaneurabeya) wrote:
>> ...
>>>> + attr.size = MIN(args->size, sizeof(attr));
>> …
>>>> + return (copyout(&attr, args->attr, args->size));
>>> 
>>> So, this lets the caller decide how much stack data they want to copy
>>> out, independent of sizeof(attr).  The only constraint is the
>>> `args->size > PAGE_SIZE` check at the beginning of the function.
>> I eliminated a duplicate calculation in the copyout which contradicted the [Linux] documented interface [1].
> 
> .. and introduced a kernel stack disclosure (the kind of thing we normally
> have to ship an SA for).  Using attr.size for the length here is the
> correct solution.

Yeah. I figured out that I typoed `args->size` instead of `attr.size` in this commit when I was rereading the Differential: https://reviews.freebsd.org/D60214?id=188351#inline-356758 (it took me longer to figure out what @markj was originally pointing out was wrong in this change). FWIW, I agree that the unsanitized user-provided value shouldn’t have been used.

To protect against making that mistake in the future, I renamed variables to hopefully make their use clearer and make the variables a bit more unique. It helps me personally because my brain sometimes blurs details when there are a lot of commonalities with words (both written and phonetic).

...

>> The interface documented by Linux allows the user to specify the amount of data to copy out (https://www.man7.org/linux/man-pages/man2/sched_getattr.2.html):
>>>  The fields of the sched_attr structure are as follows:
>>> 
>>> size This field should be set to the size of the structure in
>> bytes, as in sizeof(struct sched_attr). If the provided
>> structure is smaller than the kernel structure, any
>> additional fields are assumed to be '0'. If the provided
>> structure is larger than the kernel structure, the kernel
>> verifies that all additional fields are 0; if they are not,
>> sched_setattr() fails with the error E2BIG and updates size
>> to contain the size of the kernel structure.
> 
> Right, so the extra bytes in the case that args->size is
>> sizeof(attr) are supposed to be all zeroes already (and you
> are supposed to check that in the syscall if you aren't already)
> so you don't need to copy them back out again, only the
> sizeof(attr) bytes including the updated size.
> 
> This is not all that weird, and is similar to how we handle
> cpuset masks which also can be differently sized than the
> kernel's representation.

Ok. I guess my earlier statement demonstrates how little experience I have in this area.

Thank you,
-Enji