linux-kernel - Re: [PATCH] coredump: reduce stack usage in vfs

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] [day] [month] [year] [list]

Message-ID: <CAJqdLrq4bwF=sjZ2umwQBCMWSXEDi8N+DiSVW5poT9KfWykqmA@mail.gmail.com>
Date: Fri, 20 Jun 2025 18:49:59 +0200
From: Alexander Mikhalitsyn <alexander@...alicyn.com>
To: Arnd Bergmann <arnd@...nel.org>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>, 
	Arnd Bergmann <arnd@...db.de>, Jan Kara <jack@...e.cz>, Jann Horn <jannh@...gle.com>, 
	Luca Boccassi <luca.boccassi@...il.com>, Jeff Layton <jlayton@...nel.org>, 
	Roman Kisel <romank@...ux.microsoft.com>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] coredump: reduce stack usage in vfs_coredump()

Am Fr., 20. Juni 2025 um 13:21 Uhr schrieb Arnd Bergmann <arnd@...nel.org>:
>
> From: Arnd Bergmann <arnd@...db.de>
>
> The newly added socket coredump code runs into some corner cases
> with KASAN that end up needing a lot of stack space:
>
> fs/coredump.c:1206:1: error: the frame size of 1680 bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
>
> Mark the socket helper function as noinline_for_stack so its stack
> usage does not leak out to the other code paths. This also seems to
> help with register pressure, and the resulting combined stack usage of
> vfs_coredump() and coredump_socket() is actually lower than the inlined
> version.
>
> Moving the core_state variable into coredump_wait() helps reduce the
> stack usage further and simplifies the code, though it is not sufficient
> to avoid the warning by itself.
>
> Fixes: 6a7a50e5f1ac ("coredump: use a single helper for the socket")
> Signed-off-by: Arnd Bergmann <arnd@...db.de>

Reviewed-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>

> ---
>  fs/coredump.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/fs/coredump.c b/fs/coredump.c
> index e2611fb1f254..c46e3996ff91 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -518,27 +518,28 @@ static int zap_threads(struct task_struct *tsk,
>         return nr;
>  }
>
> -static int coredump_wait(int exit_code, struct core_state *core_state)
> +static int coredump_wait(int exit_code)
>  {
>         struct task_struct *tsk = current;
> +       struct core_state core_state;
>         int core_waiters = -EBUSY;
>
> -       init_completion(&core_state->startup);
> -       core_state->dumper.task = tsk;
> -       core_state->dumper.next = NULL;
> +       init_completion(&core_state.startup);
> +       core_state.dumper.task = tsk;
> +       core_state.dumper.next = NULL;
>
> -       core_waiters = zap_threads(tsk, core_state, exit_code);
> +       core_waiters = zap_threads(tsk, &core_state, exit_code);
>         if (core_waiters > 0) {
>                 struct core_thread *ptr;
>
> -               wait_for_completion_state(&core_state->startup,
> +               wait_for_completion_state(&core_state.startup,
>                                           TASK_UNINTERRUPTIBLE|TASK_FREEZABLE);
>                 /*
>                  * Wait for all the threads to become inactive, so that
>                  * all the thread context (extended register state, like
>                  * fpu etc) gets copied to the memory.
>                  */
> -               ptr = core_state->dumper.next;
> +               ptr = core_state.dumper.next;
>                 while (ptr != NULL) {
>                         wait_task_inactive(ptr->task, TASK_ANY);
>                         ptr = ptr->next;
> @@ -858,7 +859,7 @@ static bool coredump_sock_request(struct core_name *cn, struct coredump_params *
>         return coredump_sock_mark(cprm->file, COREDUMP_MARK_REQACK);
>  }
>
> -static bool coredump_socket(struct core_name *cn, struct coredump_params *cprm)
> +static noinline_for_stack bool coredump_socket(struct core_name *cn, struct coredump_params *cprm)
>  {
>         if (!coredump_sock_connect(cn, cprm))
>                 return false;
> @@ -1095,7 +1096,6 @@ void vfs_coredump(const kernel_siginfo_t *siginfo)
>  {
>         struct cred *cred __free(put_cred) = NULL;
>         size_t *argv __free(kfree) = NULL;
> -       struct core_state core_state;
>         struct core_name cn;
>         struct mm_struct *mm = current->mm;
>         struct linux_binfmt *binfmt = mm->binfmt;
> @@ -1131,7 +1131,7 @@ void vfs_coredump(const kernel_siginfo_t *siginfo)
>         if (coredump_force_suid_safe(&cprm))
>                 cred->fsuid = GLOBAL_ROOT_UID;
>
> -       if (coredump_wait(siginfo->si_signo, &core_state) < 0)
> +       if (coredump_wait(siginfo->si_signo) < 0)
>                 return;
>
>         old_cred = override_creds(cred);
> --
> 2.39.5
>