public inbox for io-uring@vger.kernel.org
 help / color / mirror / Atom feed
From: Chris Mason <mason@kernel.org>
To: Christian Brauner <brauner@kernel.org>
Cc: Chris Mason <mason@kernel.org>, Oleg Nesterov <oleg@redhat.com>,
	Jens Axboe <axboe@kernel.dk>,
	linux-fsdevel@vger.kernel.org,
	Alexander Viro <viro@zeniv.linux.org.uk>, Jan Kara <jack@suse.cz>,
	NeilBrown <neil@brown.name>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-mm@kvack.org, io-uring@vger.kernel.org
Subject: Re: [PATCH 1/6] coredump: don't switch a dumper that has no files table
Date: Wed, 16 Sep 2026 19:48:34 +0000	[thread overview]
Message-ID: <20260916194837.3732307-1-mason@kernel.org> (raw)
In-Reply-To: <20260915-work-coredump-fixes-v1-1-f354ca41780c@kernel.org>

On Tue, 15 Sep 2026 12:22:16 +0200 Christian Brauner <brauner@kernel.org> wrote:

> Tasks without an fdtable are skipped in coredump_close_files(). The
> coredump client itself doesn't use the same check. Since
> put_files_struct() doesn't tolerate NULL it will oops for such tasks
> without an fdtable. The prime suspect for this behavior are vhost
> workers. Skip the switch for a coredump client without a table.
>
> Fixes: b2b36bcb13ea ("coredump: add COREDUMP_CLOSE_FILES")
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
>  fs/coredump.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/coredump.c b/fs/coredump.c
> index 1fba3fed1a07..791a9268ed96 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -579,7 +579,11 @@ static bool coredump_close_files(struct core_state *core_state)
>  	/* Use the dumper's real creds not the overridden ones. */
>  	scoped_with_creds(current_real_cred()) {
>  		io_uring_task_cancel();
> -		switch_files_struct(current, files);
> +		/* The dumper itself may be a vhost worker without a table. */
> +		if (current->files)
> +			switch_files_struct(current, files);
> +		else
> +			put_files_struct(files);
>  	}
>
>  	coredump_wait_inactive(core_state);

When I reproduced this one, it ended up deadlocking with the fix applied.

vhost worker (the dumper)          sibling thread
=========================          ==============
get_signal() -> vfs_coredump()
  coredump_close_files()
    hands sibling a new table ---> switch_files_struct()
    coredump_wait_inactive()         put_files_struct(old table)
      waits for sibling's switch       last close of the vhost fd
                                     vhost_net_release()
                                       __vhost_worker_flush(): waits

AI suggests making the vhost thread requeue the signal for someone more
suitable?

-chris

  reply	other threads:[~2026-09-16 19:48 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-15 10:22 [PATCH 0/6] coredump & signals: an impossible affair Christian Brauner
2026-09-15 10:22 ` [PATCH 1/6] coredump: don't switch a dumper that has no files table Christian Brauner
2026-09-16 19:48   ` Chris Mason [this message]
2026-09-16 22:10     ` Christian Brauner
2026-09-17  1:51       ` NeilBrown
2026-09-17  8:49         ` user workers as coredumpers [Re: [PATCH 1/6] coredump: don't switch a dumper that has no files] table Christian Brauner
2026-09-18 12:28           ` Christian Brauner
2026-09-18 13:33             ` Christian Brauner
2026-09-20 15:15             ` Oleg Nesterov
2026-09-21 11:20               ` Christian Brauner
2026-09-15 10:22 ` [PATCH 2/6] fork: refuse new threads while a coredump is in progress Christian Brauner
2026-09-15 15:02   ` Oleg Nesterov
2026-09-16 11:30     ` Christian Brauner
2026-09-15 10:22 ` [PATCH 3/6] coredump: hold RCU while releasing parked threads Christian Brauner
2026-09-15 15:13   ` Oleg Nesterov
2026-09-16 11:30     ` Christian Brauner
2026-09-15 10:22 ` [PATCH 4/6] signal: only SIGKILL interrupts a coredumping task Christian Brauner
2026-09-15 10:22 ` [PATCH 5/6] coredump: parse a snapshot of core_pattern Christian Brauner
2026-09-15 10:22 ` [PATCH 6/6] io-wq: order the exit bit against worker creation task work Christian Brauner
2026-09-15 12:01 ` [PATCH 0/6] coredump & signals: an impossible affair Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260916194837.3732307-1-mason@kernel.org \
    --to=mason@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=brauner@kernel.org \
    --cc=io-uring@vger.kernel.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@redhat.com \
    --cc=neil@brown.name \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox