linux-kernel - Re: [PATCH 1/2] copy_process: Handle architectures where sizeof(unsigned long)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <233952f3-fdee-4ad6-8ad4-8f690b036f68@lucifer.local>
Date: Fri, 22 Aug 2025 12:22:56 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: schuster.simon@...mens-energy.com
Cc: Dinh Nguyen <dinguyen@...nel.org>, Christian Brauner <brauner@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        David Hildenbrand <david@...hat.com>,
        "Liam R. Howlett" <Liam.Howlett@...cle.com>,
        Vlastimil Babka <vbabka@...e.cz>, Mike Rapoport <rppt@...nel.org>,
        Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
        Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>,
        Juri Lelli <juri.lelli@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>,
        Mel Gorman <mgorman@...e.de>, Valentin Schneider <vschneid@...hat.com>,
        Kees Cook <kees@...nel.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] copy_process: Handle architectures where
 sizeof(unsigned long) < sizeof(u64)

On Thu, Aug 21, 2025 at 01:27:37PM +0200, Simon Schuster via B4 Relay wrote:
> From: Simon Schuster <schuster.simon@...mens-energy.com>
>
> With the introduction of clone3 in commit 7f192e3cd316 ("fork: add
> clone3") the effective bit width of clone_flags on all architectures was
> increased from 32bit to 64bit. However, the signature of the copy_*
> helper functions (e.g., copy_sighand) used by copy_process was not
> adapted, as such, they potentially truncate the flags on architectures
> such as nios2, where unsigned long is a 32bit unsigned integer type.
>
> This can, for instance, be observed via failures of kernel selftest
> clone3_clear_sighand, which attempts to trigger the conditional
>
> 	if (clone_flags & CLONE_CLEAR_SIGHAND)
>
> in function copy_sighand within fork.c that will always fail given:
>
> 	unsigned long /* == uint32_t */ clone_flags
> 	#define CLONE_CLEAR_SIGHAND 0x100000000ULL
>
> This commit fixes the bug by always passing clone_flags via their
> declared u64 type, invariant of architecture-dependent integer sizes.
>
> Signed-off-by: Simon Schuster <schuster.simon@...mens-energy.com>

Ah this is a change after my own heart :) as I have worked to make mm flags
at a fixed size per architecture (and indeed, expandable in the future)
[0], and plan to do so for VMA flags also.

It'd be nice to go further and make this an opaque type etc. etc. but not
sure if worth it.

In any case for a backportable fix (I agree with others that indeed this
needs a fixes and backporting as this is a bug fundamentally) this is fine.

So,

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>

[0]: https://lore.kernel.org/linux-mm/cover.1755012943.git.lorenzo.stoakes@oracle.com/

> ---
>  kernel/fork.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 5115be549234..0e9b2dd6c365 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1510,7 +1510,7 @@ static struct mm_struct *dup_mm(struct task_struct *tsk,
>  	return NULL;
>  }
>
> -static int copy_mm(unsigned long clone_flags, struct task_struct *tsk)
> +static int copy_mm(u64 clone_flags, struct task_struct *tsk)
>  {
>  	struct mm_struct *mm, *oldmm;
>
> @@ -1548,7 +1548,7 @@ static int copy_mm(unsigned long clone_flags, struct task_struct *tsk)
>  	return 0;
>  }
>
> -static int copy_fs(unsigned long clone_flags, struct task_struct *tsk)
> +static int copy_fs(u64 clone_flags, struct task_struct *tsk)
>  {
>  	struct fs_struct *fs = current->fs;
>  	if (clone_flags & CLONE_FS) {
> @@ -1569,7 +1569,7 @@ static int copy_fs(unsigned long clone_flags, struct task_struct *tsk)
>  	return 0;
>  }
>
> -static int copy_files(unsigned long clone_flags, struct task_struct *tsk,
> +static int copy_files(u64 clone_flags, struct task_struct *tsk,
>  		      int no_files)
>  {
>  	struct files_struct *oldf, *newf;
> @@ -1599,7 +1599,7 @@ static int copy_files(unsigned long clone_flags, struct task_struct *tsk,
>  	return 0;
>  }
>
> -static int copy_sighand(unsigned long clone_flags, struct task_struct *tsk)
> +static int copy_sighand(u64 clone_flags, struct task_struct *tsk)
>  {
>  	struct sighand_struct *sig;
>
> @@ -1648,7 +1648,7 @@ static void posix_cpu_timers_init_group(struct signal_struct *sig)
>  	posix_cputimers_group_init(pct, cpu_limit);
>  }
>
> -static int copy_signal(unsigned long clone_flags, struct task_struct *tsk)
> +static int copy_signal(u64 clone_flags, struct task_struct *tsk)
>  {
>  	struct signal_struct *sig;
>
>
> --
> 2.39.5
>
>