Re: [PATCH v5] coredump: Add bit 9 of coredump_filter for pre-exit files before dumping

[Xin Zhao]

On Tue, 30 Jun 2026 13:50:15 +0200 Christian Brauner <brauner@xxxxxxxxxx> wrote:

> > + * If do not dump fd list, files that are not referenced by any VMA
> > + * can be released before dumping core. Therefore, some file release
> > + * logic, such as exiting flock or releasing references to shared
> > + * buffers is executed much earlier. Note that do_coredump() often
> > + * takes several seconds or even longer to execute.
> > + */
> > +static void coredump_pre_exit(void)
> > +{
> > +	struct task_struct *tsk = current, *t;
> > +
> > +	if (mm_flags_test(MMF_DUMP_FD_LIST, tsk->mm))
> > +		return;
> 
> Why does this hanging off of mm?

Like other bits of coredump_filter, it is actually attached to the mm flags.

> Even if we wanted to do this it is fundamentally the wrong primitive for
> this. I would envision that anything that wanted to "zap" file
> descriptors synchronously would either have to do it synchronously or
> offload it to a delayed coredump list and wait for it to have drained.
> 
> I dislike both options... Without thorough anaylsis I'm not even sure if

Are you saying that the file release operation must be performed in the
context of each task_struct? I'm not quite clear on the necessity of doing
this. The execution of exit_files merely reduces the reference count of
the file_struct for the thread, rather than decreasing the reference count
of each individual file.

> taking out files out of order with other exit-related cleanup will not
> end up causing fun bugs. Meaning:
> 
> [...]
> exit_sem(tsk);
> exit_shm(tsk);
> exit_files(tsk);
> 
> anything before that might implicitly relying on files struct being
> sane or some other subtle interactions. My appetite to dig into this
> just to make this questionable patch mergeable isn't very high...

The essence of file descriptors (fd) is to allow processes to reference
specific files through an index. The fundamental purpose of exit_files()
is that when a process exits, it no longer needs to reference files by
their indices. From this perspective, I don't see it causing any issues,
because even if functions like exit_sem and exit_shm need to use file
instances, they won't use the fd index; instead, they will increase the
reference count of the file as needed in the prior logic. By the time the
core dump begins execution, user-space logic will no longer be running.
Therefore, as long as the contents of the core dump do not require the
mapping relationship between fd and file, the file can be safely put.


> > -	if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD))) {
> > +	/*
> > +	 * coredump_pre_exit() may release files before dumping core.
> > +	 * Cannot use task_work in the case, needs to release files
> > +	 * earlier."
> 
> And it does that by offloading the closing of files to a kthread that
> runs asynchronously?

yes


> Also, as was pointed out in other parts of the thread: files are shared
> _across_ thread-groups not just within thread-groups. Anything that does
> SCM_RIGHTS or has done some pidfd_getfd() dance - however unlikely -
> might still hold that file alive and you have zero chances of getting it
> out of its hands.
> 
> If this is so critical, just skip the coredump via COREDUMP_REJECT or
> similar means.

> > +	 */
> > +	if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD | PF_DUMPCORE))) {
> 
> This:
> 
> (task->flags & PF_KTHREAD | PF_DUMPCORE)
> 
> will have interesting side-effects for everyone else...

As you mentioned, other processes may potentially reference these files
across processes, so even if a core dump is not triggered, the file
release actions are not entirely executed within task_work. Because after
a process exits, other processes may still hold references to the file,
the work for releasing the file cannot be added to task_work at that
point. In this case, it still uses schedule_delayed_work().

If kernel developers do not correctly implement the file release
operation, such as relying on certain information from the task context to
free related resources, then the issues arising from such unreasonable
release operations will manifest in other scenarios, regardless of my
changes. My modifications only amplify this problem in the context of core
dumps.


Thanks
Xin Zhao