Re: [PATCH 1/2] copy_process: Handle architectures where sizeof(unsigned long) < sizeof(u64)

[David Hildenbrand]

On 22.08.25 10:52, schuster.simon@xxxxxxxxxxxxxxxxxx wrote:
> On Thu, Aug 21, 2025 at 11:14:00PM +0200, David Hildenbrand wrote:
> > Sounds reasonable.
> >
> > But is this actually something that is already exposed before patch#2
> > on other architectures?
>
> I'm not sure, but I would assume so, as e.g., arch/arm seems to have
> support for clone3, but also seems to use 32bit unsigned longs as far as
> I can tell and, thus, should also be affected:
>
> $ cat /tmp/printulsize.c
> #include <stdio.h>
>
> int main(void) {
> 	printf("sizeof(unsigned long): %zu\n", sizeof(unsigned long));
> }
> $ arm-linux-gnueabi-gcc-12 /tmp/printulsize.c -o printulsize
> $ qemu-arm -L /usr/arm-linux-gnueabi ./printulsize
> sizeof(unsigned long): 4
>
> Is the above test enough to warrant a "Fixes: ", or do we need a
> reproduced kselftest failure on some arch for that?

It would be good to describe that this would be an issue on nios2 and 
was reproduced there without this fix. Then you can mention that this 
should be an issue on 32bit archs with clone3 support as well, like arm.

Then we should add a Fixes:

> > (I assume above output is with patch #2 but without patch #1)
>
> Yes, sorry, that one is on me; I've naturally first implemented support
> for clone3 on nios2 and then investigated the test failures, but somehow
> deemed it wise for whatever reason to switch the commit order in the
> patch submission...

Right.

I'll note that copy_process() ends up calling other functions with 
clone_flags that accept an "unsigned long", like sched_fork(), which you 
don't handle here.

$ git grep "long clone_flags"

likely is a good indication what needs changing outside of kernel/fork.c.

It should be spelled out why you don't have to handle the others. And 
likely in the fix, we should really only fix the ones that are really 
required for now.

As a follow-up, we should likely better convert *all* users of 
clone_flags to use u64 (at least the one in core code), not just the 
ones in kernel/fork.c you tried to handle here.


For now, only the following require 64bit:

	CLONE_CLEAR_SIGHAND
	CLONE_INTO_CGROUP

CLONE_CLEAR_SIGHAND is only checked against extracted flags in:

* copy_process()->copy_sighand() and
 -> We don't use u64
* copy_process()->perf_event_init_task()->perf_event_init_context()->
  inherit_task_group()
 -> We do use u64 already

CLONE_INTO_CGROUP doesn't seem to be checked against extracted flags AFAIKS.


So I suggest making this fix CLONE_CLEAR_SIGHAND-specific and fixing 
only copy_sighand(). That one should carry Fixes:

Then, have a second patch where we convert all remaining "unsigned long 
clone_flags" in the core to use u64. That one would not be a fix.

Makes sense?

--
Cheers

David / dhildenb