[PATCH RFC v2 00/16] coredump, files: exit files on request

[PATCH RFC v2 00/16] coredump, files: exit files on request

[Christian Brauner]

We've had quite a few proposal for exiting files before generating the
coredump (see [1]-[8]). The various proposed solution were quite
unacceptable. So I took some time to look what a _remotely_ acceptable
version of this could look like. Here it is.

The gist is that we allow the coredump socket to request the
thread-group shed the various fdtables closed synchronously before
generating the coredump.

The intricate task is that this requires us to add a primitive to close
fdtables synchronously. Coredump is not enough to justify this but
performance numbers for this look pretty convincing. For tasks exiting
with a large file descriptor table this means getting rid of _a lot_ of
cmpxchg()s and hammering on task->pi_lock. The cost of this was
mentioned in various threads over the years. I found at least recent
comments by Oleg and Neil.

The obvious problematic case are kernel threads. fput() punts their
final __fput() to a workqueue. kthreads never return to userspace to run
task work and some of them may need to finish a umount and so cannot
->release() inline. No path we change is taken by a kthread. They don't
own a descriptor table and kthreadd is created with CLONE_FILES and
every kthread inherits it. So all of them share init_files and init_task
pins that forever. Their exit_files() never drops the last reference.

kernel_execve() refuses PF_KTHREAD outright. Usermodehelpers are spawned
without the kthread flag. close_range() is a syscall. The one indirect
way is a failing copy_process() calling exit_files() on a 
child from a kthread parent. A dup_fd() in copy_process() cannot hold
the last reference to any of its files while the source table is alive.

Worker threads such as io_uring workers are user threads without
PF_KTHREAD that share the fdtable. One of them can be the last to put
the fdtable at exit. Today fput() hands their final __fput()s to task
work. The worker runs that itself in exit_task_work() a few lines later.
Now the same thread does the same work in exit_files() instead. vhost
workers are created without a descriptor table at all, so they never
even put an fdtable. Neither kind ever execs or calls close_range().

Now the performance numbers. I measured this on a 64 vCPU KVM guest (AMD
EPYC 9754) on top of vfs-7.4.coredump with and without the series and
the same x86_64_defconfig for both. The benchmark opens N files in a
forked worker and times the teardown:

- process exit
- execve() with N close-on-exec descriptors
- close_range() with and without CLOSE_RANGE_UNSHARE

Here are the medians over two or three boots which agree within 2%. With
/dev/null as the file and 4096, 65536 and 1M descriptors:

                         4096 fds        65536 fds        1M fds
  ----------------------------------------------------------------------
  exit                    890 -> 826 us  13.5 -> 12.4 ms  221 -> 203 ms
                          -7.2%          -7.9%            -8.3%
  ----------------------------------------------------------------------
  execve, close-on-exec  1215 -> 1113 us 15.9 -> 14.2 ms  247 -> 224 ms
                          -8.4%          -10.1%           -9.0%
  ----------------------------------------------------------------------
  close_range()           984 -> 909 us  17.3 -> 15.3 ms  268 -> 245 ms
                          -7.6%          -11.5%           -8.7%
  ----------------------------------------------------------------------
  close_range(UNSHARE)    990 -> 912 us  17.1 -> 15.3 ms  267 -> 245 ms
                          -7.9%          -10.3%           -8.4%

So that's a 7-11% win or a constant 15-25 ns per closed file. This sheds
init_task_work() and task_work_add() per file with its cmpxchg() and the
TIF_NOTIFY_RESUME test-and-set, the list walk with the indirect call
in the path our, and then a second pass over every struct file that
isn't cache-hot anymore by then.

The performance boost is linear in the number of descriptors from 256 to
1M. Note the thread count doesn't matter since one thread does the close
anyway.

So with eight processes tearing down 65536 descriptors each at the same
time the win grows to 13-15% (20.1 -> 17.4 ms for exit, 20.5 -> 17.4 ms
for execve). The old code walks the table once to queue the task work
and then walks all 16 MB of struct file a second time from
task_work_run().

So once eight CPUs compete for the cache that second pass gets
very expensive. With 32 or 64 processes at once both kernels are bound
by pushing millions of files through slab and RCU. So then the
difference shrinks to 1-4%. Still though...

Files whose own release dominates the cleanup gain a little less. 4% for
eventfds, 7% for pipes, about 1% for AF_UNIX sockets and distinct tmpfs
inodes. For them a release costs 1.25 us per descriptor anyway.

Using function profiling a 65536 descriptor exit spends 4.0 ms in
exit_files() and 10.4 ms in task_work_run() before and now 12.5 ms in
exit_files() and nothing in task_work_run() after.

close(2) is untouched and measures the same. will-it-scale (open1,
open3, dup1, eventfd1, unix1, pipe1, signal1, processes and threads)
is within 2% either way. dup1 a few percent better which I'd put down
to layout.

So, back to coredumps. With COREDUMP_CLOSE_FILES coredumps switch to an
empty file descriptor table before generating the coredump. That
requires a bit of synchronization but I think I got the basics down.

The coredumping thread allocates a new empty file descriptor table. It
then wakes all threads in the thread-group and tells them to switch to
the empty file descriptor table and get rid of the old one. They report
back once they're done.

So that handles most cases where locks would be held for an unreasonable
time until the coredump is generated but since files can be shared
between completely unrelated processes that's not a guarantee and we
can't give one. But it solves the reported issue without resorting to
even grosser hacks.

Link: https://lore.kernel.org/20260618030700.2511668-1-jackzxcui1989@163.com [1]
Link: https://lore.kernel.org/20260618150301.3226517-1-jackzxcui1989@163.com [2]
Link: https://lore.kernel.org/20260619122419.3954581-1-jackzxcui1989@163.com [3]
Link: https://lore.kernel.org/20260624145552.70143-1-jackzxcui1989@163.com [4]
Link: https://lore.kernel.org/20260630075604.52533-1-jackzxcui1989@163.com [5]
Link: https://lore.kernel.org/20260804001703.1340667-1-jackzxcui1989@163.com [6]
Link: https://lore.kernel.org/20260807040124.1706927-1-jackzxcui1989@163.com [7]
Link: https://lore.kernel.org/20260808052732.2589657-1-jackzxcui1989@163.com [8]

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Changes in v2:
- Revamp.
- Link to v1: https://patch.msgid.link/20260824-work-coredump-unlock-self-v1-0-48fd1cca7eda@kernel.org

---
Christian Brauner (16):
      fs: don't open-code file_close_fd() in close_fd()
      fs: add fput_close_list() and fput_list()
      fs: rename do_close_on_exec() to close_cloexec_files()
      fs: defer the final fput of close-on-exec files past the exec locks
      fs: add switch_files_struct()
      fs: move unshare_fd() to fs/file.c
      fs: remove unshare_files()
      exec: release fdtable after exec locks drop
      fs: make close_files() synchronous
      fs: make close_range() synchronous
      coredump: s/startup/done/g
      coredump: factor out coredump_wait_inactive()
      fs: add alloc_files_struct()
      coredump: add COREDUMP_CLOSE_FILES
      tools: sync coredump.h header
      selftests/coredump: test COREDUMP_CLOSE_FILES

 fs/coredump.c                                      |  91 +++-
 fs/exec.c                                          |  11 +-
 fs/file.c                                          | 124 +++--
 fs/file_table.c                                    |  25 +-
 fs/internal.h                                      |   3 +
 fs/open.c                                          |   2 +-
 include/linux/binfmts.h                            |   6 +
 include/linux/fdtable.h                            |   7 +-
 include/linux/sched/signal.h                       |   5 +-
 include/uapi/linux/coredump.h                      |   8 +
 kernel/exit.c                                      |  15 +-
 kernel/fork.c                                      |  48 +-
 tools/include/uapi/linux/coredump.h                |   8 +
 tools/testing/selftests/coredump/Makefile          |   4 +-
 .../selftests/coredump/coredump_close_files_test.c | 592 +++++++++++++++++++++
 .../coredump/coredump_socket_protocol_test.c       |   6 +
 .../selftests/coredump/coredump_test_helpers.c     |   3 +-
 17 files changed, 847 insertions(+), 111 deletions(-)
---
base-commit: 582e5e0d99ac356fac9656767070269bb4ffc1b2
change-id: 20260824-work-coredump-unlock-self-63a898912870

[PATCH RFC v2 01/16] fs: don't open-code file_close_fd() in close_fd()

[Christian Brauner]

close_fd() takes the lock, calls file_close_fd_locked() and drops the
lock, which is exactly what file_close_fd() does. Use it.

No functional changes.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/file.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index 628ca07dc4b1..59673547de90 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -732,16 +732,13 @@ struct file *file_close_fd_locked(struct files_struct *files, unsigned fd)
 
 int close_fd(unsigned fd)
 {
-	struct files_struct *files = current->files;
 	struct file *file;
 
-	spin_lock(&files->file_lock);
-	file = file_close_fd_locked(files, fd);
-	spin_unlock(&files->file_lock);
+	file = file_close_fd(fd);
 	if (!file)
 		return -EBADF;
 
-	return filp_close(file, files);
+	return filp_close(file, current->files);
 }
 EXPORT_SYMBOL(close_fd);
 

-- 
2.53.0

[PATCH RFC v2 02/16] fs: add fput_close_list() and fput_list()

[Christian Brauner]

Add helpers for paths that close many files in one go. They collect the
last references during the walk and drain them right after

fput_list() runs the final __fput() for everything on the list.
delayed_fput() shares that loop.

We'll use that in the next patches.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/file_table.c | 25 +++++++++++++++++++++----
 fs/internal.h   |  3 +++
 fs/open.c       |  2 +-
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/fs/file_table.c b/fs/file_table.c
index c68b8c0a4097..617912ca4e0a 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -528,11 +528,10 @@ static void __fput(struct file *file)
 static LLIST_HEAD(delayed_fput_list);
 static void delayed_fput(struct work_struct *unused)
 {
-	struct llist_node *node = llist_del_all(&delayed_fput_list);
-	struct file *f, *t;
+	/* Detach the shared list once, then drain it like a private one. */
+	struct llist_head list = { .first = llist_del_all(&delayed_fput_list) };
 
-	llist_for_each_entry_safe(f, t, node, f_llist)
-		__fput(f);
+	fput_list(&list);
 }
 
 static void ____fput(struct callback_head *work)
@@ -629,6 +628,24 @@ void fput_close(struct file *file)
 		__fput_deferred(file);
 }
 
+/* Like fput_close(), but the last reference goes on the caller's @list. */
+void fput_close_list(struct file *file, struct llist_head *list)
+{
+	if (file_ref_put_close(&file->f_ref))
+		__llist_add(&file->f_llist, list);
+}
+
+/* Run the final __fput() for everything on the private @list, right here. */
+void fput_list(struct llist_head *list)
+{
+	struct file *f, *t;
+
+	llist_for_each_entry_safe(f, t, __llist_del_all(list), f_llist) {
+		__fput(f);
+		cond_resched();
+	}
+}
+
 void __init files_init(void)
 {
 	struct kmem_cache_args args = {
diff --git a/fs/internal.h b/fs/internal.h
index c658c8a5ebd5..619eaeb23fd9 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -129,6 +129,8 @@ static inline void put_file_access(struct file *file)
 
 void fput_close_sync(struct file *);
 void fput_close(struct file *);
+void fput_close_list(struct file *file, struct llist_head *list);
+void fput_list(struct llist_head *list);
 
 /*
  * super.c
@@ -198,6 +200,7 @@ extern struct file *do_file_open_root(const struct path *,
 extern struct open_how build_open_how(int flags, umode_t mode);
 extern int build_open_flags(const struct open_how *how, struct open_flags *op);
 struct file *file_close_fd_locked(struct files_struct *files, unsigned fd);
+int filp_flush(struct file *filp, fl_owner_t id);
 
 int do_ftruncate(struct file *file, loff_t length, unsigned int flags);
 int chmod_common(const struct path *path, umode_t mode);
diff --git a/fs/open.c b/fs/open.c
index 6b1c14e684a9..7181665c3af5 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1506,7 +1506,7 @@ SYSCALL_DEFINE2(creat, const char __user *, pathname, umode_t, mode)
  * "id" is the POSIX thread ID. We use the
  * files pointer for this..
  */
-static int filp_flush(struct file *filp, fl_owner_t id)
+int filp_flush(struct file *filp, fl_owner_t id)
 {
 	int retval = 0;
 

-- 
2.53.0

Re: [PATCH RFC v2 02/16] fs: add fput_close_list() and fput_list()

[Mateusz Guzik]

On Wed, Sep 2, 2026 at 5:58 PM Christian Brauner <brauner@kernel.org> wrote:
>
> Add helpers for paths that close many files in one go. They collect the
> last references during the walk and drain them right after
>
> fput_list() runs the final __fput() for everything on the list.
> delayed_fput() shares that loop.

nit: I think fput_list is a highly misleading name here, suggesting it
grabs a list of files to unref.

I don't have a proposal for a name I'm happy with and I'm not going to
argue about this, but to throw some ideas: filp_close_list,
fput_finish_list or similar

Regardless of what happens with the above:
Reviewed-by: Mateusz Guzik <mjguzik@gmail.com>

>
> We'll use that in the next patches.
>
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
>  fs/file_table.c | 25 +++++++++++++++++++++----
>  fs/internal.h   |  3 +++
>  fs/open.c       |  2 +-
>  3 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/fs/file_table.c b/fs/file_table.c
> index c68b8c0a4097..617912ca4e0a 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -528,11 +528,10 @@ static void __fput(struct file *file)
>  static LLIST_HEAD(delayed_fput_list);
>  static void delayed_fput(struct work_struct *unused)
>  {
> -       struct llist_node *node = llist_del_all(&delayed_fput_list);
> -       struct file *f, *t;
> +       /* Detach the shared list once, then drain it like a private one. */
> +       struct llist_head list = { .first = llist_del_all(&delayed_fput_list) };
>
> -       llist_for_each_entry_safe(f, t, node, f_llist)
> -               __fput(f);
> +       fput_list(&list);
>  }
>
>  static void ____fput(struct callback_head *work)
> @@ -629,6 +628,24 @@ void fput_close(struct file *file)
>                 __fput_deferred(file);
>  }
>
> +/* Like fput_close(), but the last reference goes on the caller's @list. */
> +void fput_close_list(struct file *file, struct llist_head *list)
> +{
> +       if (file_ref_put_close(&file->f_ref))
> +               __llist_add(&file->f_llist, list);
> +}
> +
> +/* Run the final __fput() for everything on the private @list, right here. */
> +void fput_list(struct llist_head *list)
> +{
> +       struct file *f, *t;
> +
> +       llist_for_each_entry_safe(f, t, __llist_del_all(list), f_llist) {
> +               __fput(f);
> +               cond_resched();
> +       }
> +}
> +
>  void __init files_init(void)
>  {
>         struct kmem_cache_args args = {
> diff --git a/fs/internal.h b/fs/internal.h
> index c658c8a5ebd5..619eaeb23fd9 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -129,6 +129,8 @@ static inline void put_file_access(struct file *file)
>
>  void fput_close_sync(struct file *);
>  void fput_close(struct file *);
> +void fput_close_list(struct file *file, struct llist_head *list);
> +void fput_list(struct llist_head *list);
>
>  /*
>   * super.c
> @@ -198,6 +200,7 @@ extern struct file *do_file_open_root(const struct path *,
>  extern struct open_how build_open_how(int flags, umode_t mode);
>  extern int build_open_flags(const struct open_how *how, struct open_flags *op);
>  struct file *file_close_fd_locked(struct files_struct *files, unsigned fd);
> +int filp_flush(struct file *filp, fl_owner_t id);
>
>  int do_ftruncate(struct file *file, loff_t length, unsigned int flags);
>  int chmod_common(const struct path *path, umode_t mode);
> diff --git a/fs/open.c b/fs/open.c
> index 6b1c14e684a9..7181665c3af5 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -1506,7 +1506,7 @@ SYSCALL_DEFINE2(creat, const char __user *, pathname, umode_t, mode)
>   * "id" is the POSIX thread ID. We use the
>   * files pointer for this..
>   */
> -static int filp_flush(struct file *filp, fl_owner_t id)
> +int filp_flush(struct file *filp, fl_owner_t id)
>  {
>         int retval = 0;
>
>
> --
> 2.53.0
>

Re: [PATCH RFC v2 02/16] fs: add fput_close_list() and fput_list()

[Mateusz Guzik]

On Wed, Sep 2, 2026 at 7:41 PM Mateusz Guzik <mjguzik@gmail.com> wrote:
>
> On Wed, Sep 2, 2026 at 5:58 PM Christian Brauner <brauner@kernel.org> wrote:
> >
> > Add helpers for paths that close many files in one go. They collect the
> > last references during the walk and drain them right after
> >
> > fput_list() runs the final __fput() for everything on the list.
> > delayed_fput() shares that loop.
>
> nit: I think fput_list is a highly misleading name here, suggesting it
> grabs a list of files to unref.
>
> I don't have a proposal for a name I'm happy with and I'm not going to
> argue about this, but to throw some ideas: filp_close_list,
> fput_finish_list or similar

The more I'm looking at this, the more I'm convinced I'm whatever name
it should follow the convention of ending with _sync.

>
> Regardless of what happens with the above:
> Reviewed-by: Mateusz Guzik <mjguzik@gmail.com>
>
> >
> > We'll use that in the next patches.
> >
> > Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> > ---
> >  fs/file_table.c | 25 +++++++++++++++++++++----
> >  fs/internal.h   |  3 +++
> >  fs/open.c       |  2 +-
> >  3 files changed, 25 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/file_table.c b/fs/file_table.c
> > index c68b8c0a4097..617912ca4e0a 100644
> > --- a/fs/file_table.c
> > +++ b/fs/file_table.c
> > @@ -528,11 +528,10 @@ static void __fput(struct file *file)
> >  static LLIST_HEAD(delayed_fput_list);
> >  static void delayed_fput(struct work_struct *unused)
> >  {
> > -       struct llist_node *node = llist_del_all(&delayed_fput_list);
> > -       struct file *f, *t;
> > +       /* Detach the shared list once, then drain it like a private one. */
> > +       struct llist_head list = { .first = llist_del_all(&delayed_fput_list) };
> >
> > -       llist_for_each_entry_safe(f, t, node, f_llist)
> > -               __fput(f);
> > +       fput_list(&list);
> >  }
> >
> >  static void ____fput(struct callback_head *work)
> > @@ -629,6 +628,24 @@ void fput_close(struct file *file)
> >                 __fput_deferred(file);
> >  }
> >
> > +/* Like fput_close(), but the last reference goes on the caller's @list. */
> > +void fput_close_list(struct file *file, struct llist_head *list)
> > +{
> > +       if (file_ref_put_close(&file->f_ref))
> > +               __llist_add(&file->f_llist, list);
> > +}
> > +
> > +/* Run the final __fput() for everything on the private @list, right here. */
> > +void fput_list(struct llist_head *list)
> > +{
> > +       struct file *f, *t;
> > +
> > +       llist_for_each_entry_safe(f, t, __llist_del_all(list), f_llist) {
> > +               __fput(f);
> > +               cond_resched();
> > +       }
> > +}
> > +
> >  void __init files_init(void)
> >  {
> >         struct kmem_cache_args args = {
> > diff --git a/fs/internal.h b/fs/internal.h
> > index c658c8a5ebd5..619eaeb23fd9 100644
> > --- a/fs/internal.h
> > +++ b/fs/internal.h
> > @@ -129,6 +129,8 @@ static inline void put_file_access(struct file *file)
> >
> >  void fput_close_sync(struct file *);
> >  void fput_close(struct file *);
> > +void fput_close_list(struct file *file, struct llist_head *list);
> > +void fput_list(struct llist_head *list);
> >
> >  /*
> >   * super.c
> > @@ -198,6 +200,7 @@ extern struct file *do_file_open_root(const struct path *,
> >  extern struct open_how build_open_how(int flags, umode_t mode);
> >  extern int build_open_flags(const struct open_how *how, struct open_flags *op);
> >  struct file *file_close_fd_locked(struct files_struct *files, unsigned fd);
> > +int filp_flush(struct file *filp, fl_owner_t id);
> >
> >  int do_ftruncate(struct file *file, loff_t length, unsigned int flags);
> >  int chmod_common(const struct path *path, umode_t mode);
> > diff --git a/fs/open.c b/fs/open.c
> > index 6b1c14e684a9..7181665c3af5 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -1506,7 +1506,7 @@ SYSCALL_DEFINE2(creat, const char __user *, pathname, umode_t, mode)
> >   * "id" is the POSIX thread ID. We use the
> >   * files pointer for this..
> >   */
> > -static int filp_flush(struct file *filp, fl_owner_t id)
> > +int filp_flush(struct file *filp, fl_owner_t id)
> >  {
> >         int retval = 0;
> >
> >
> > --
> > 2.53.0
> >

Re: [PATCH RFC v2 02/16] fs: add fput_close_list() and fput_list()

[Christian Brauner]

On Wed, Sep 02, 2026 at 07:58:46PM +0200, Mateusz Guzik wrote:
> On Wed, Sep 2, 2026 at 7:41 PM Mateusz Guzik <mjguzik@gmail.com> wrote:
> >
> > On Wed, Sep 2, 2026 at 5:58 PM Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > Add helpers for paths that close many files in one go. They collect the
> > > last references during the walk and drain them right after
> > >
> > > fput_list() runs the final __fput() for everything on the list.
> > > delayed_fput() shares that loop.
> >
> > nit: I think fput_list is a highly misleading name here, suggesting it
> > grabs a list of files to unref.
> >
> > I don't have a proposal for a name I'm happy with and I'm not going to
> > argue about this, but to throw some ideas: filp_close_list,
> > fput_finish_list or similar
> 
> The more I'm looking at this, the more I'm convinced I'm whatever name
> it should follow the convention of ending with _sync.

Ok, might change.

[PATCH RFC v2 03/16] fs: rename do_close_on_exec() to close_cloexec_files()

[Christian Brauner]

Rename the helper and align it with close_files().

No functional changes.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/exec.c               | 2 +-
 fs/file.c               | 2 +-
 include/linux/fdtable.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 745f6eb5279e..92a02bce11d5 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1220,7 +1220,7 @@ int begin_new_exec(struct linux_binprm * bprm)
 	 * trying to access the should-be-closed file descriptors of a process
 	 * undergoing exec(2).
 	 */
-	do_close_on_exec(me->files);
+	close_cloexec_files(me->files);
 
 	if (bprm->secureexec) {
 		/* Make sure parent cannot signal privileged process. */
diff --git a/fs/file.c b/fs/file.c
index 59673547de90..be06e09df41c 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -884,7 +884,7 @@ struct file *file_close_fd(unsigned int fd)
 	return file;
 }
 
-void do_close_on_exec(struct files_struct *files)
+void close_cloexec_files(struct files_struct *files)
 {
 	unsigned i;
 	struct fdtable *fdt;
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index c45306a9f007..e6b641d9eb91 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -105,7 +105,7 @@ struct fd_range {
 	unsigned int from, to;
 };
 struct files_struct *dup_fd(struct files_struct *, struct fd_range *) __latent_entropy;
-void do_close_on_exec(struct files_struct *);
+void close_cloexec_files(struct files_struct *);
 int iterate_fd(struct files_struct *, unsigned,
 		int (*)(const void *, struct file *, unsigned),
 		const void *);

-- 
2.53.0

Re: [PATCH RFC v2 03/16] fs: rename do_close_on_exec() to close_cloexec_files()

[Mateusz Guzik]

On Wed, Sep 2, 2026 at 5:58 PM Christian Brauner <brauner@kernel.org> wrote:
>
> Rename the helper and align it with close_files().
>
> No functional changes.
>
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
>  fs/exec.c               | 2 +-
>  fs/file.c               | 2 +-
>  include/linux/fdtable.h | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index 745f6eb5279e..92a02bce11d5 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1220,7 +1220,7 @@ int begin_new_exec(struct linux_binprm * bprm)
>          * trying to access the should-be-closed file descriptors of a process
>          * undergoing exec(2).
>          */
> -       do_close_on_exec(me->files);
> +       close_cloexec_files(me->files);
>
>         if (bprm->secureexec) {
>                 /* Make sure parent cannot signal privileged process. */
> diff --git a/fs/file.c b/fs/file.c
> index 59673547de90..be06e09df41c 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -884,7 +884,7 @@ struct file *file_close_fd(unsigned int fd)
>         return file;
>  }
>
> -void do_close_on_exec(struct files_struct *files)
> +void close_cloexec_files(struct files_struct *files)
>  {
>         unsigned i;
>         struct fdtable *fdt;
> diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
> index c45306a9f007..e6b641d9eb91 100644
> --- a/include/linux/fdtable.h
> +++ b/include/linux/fdtable.h
> @@ -105,7 +105,7 @@ struct fd_range {
>         unsigned int from, to;
>  };
>  struct files_struct *dup_fd(struct files_struct *, struct fd_range *) __latent_entropy;
> -void do_close_on_exec(struct files_struct *);
> +void close_cloexec_files(struct files_struct *);
>  int iterate_fd(struct files_struct *, unsigned,
>                 int (*)(const void *, struct file *, unsigned),
>                 const void *);
>
> --
> 2.53.0
>

Reviewed-by: Mateusz Guzik <mjguzik@gmail.com>

[PATCH RFC v2 04/16] fs: defer the final fput of close-on-exec files past the exec locks

[Christian Brauner]

Taks with lots of file descriptors pay a hefy price for offloading to
task work. Port them to the fput_list() mechanism instead and waste
files in free_binprm().

Link: https://lore.kernel.org/CAGudoHGRvxd-EwwDasqoVhk5VbEvhtjjM7kCXBfFyQ8ykeC9CQ@mail.gmail.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/exec.c               | 4 +++-
 fs/file.c               | 6 ++++--
 include/linux/binfmts.h | 4 ++++
 include/linux/fdtable.h | 2 +-
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 92a02bce11d5..b1f449dba258 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1220,7 +1220,7 @@ int begin_new_exec(struct linux_binprm * bprm)
 	 * trying to access the should-be-closed file descriptors of a process
 	 * undergoing exec(2).
 	 */
-	close_cloexec_files(me->files);
+	close_cloexec_files(me->files, &bprm->cloexec_files);
 
 	if (bprm->secureexec) {
 		/* Make sure parent cannot signal privileged process. */
@@ -1469,6 +1469,8 @@ static void free_bprm(struct linux_binprm *bprm)
 		mutex_unlock(&current->signal->cred_guard_mutex);
 		abort_creds(bprm->cred);
 	}
+	/* The exec locks are out of the way now, see close_cloexec_files(). */
+	fput_list(&bprm->cloexec_files);
 	/* exec swapped the mm but failed before setup_new_exec() freed it */
 	if (bprm->old_mm)
 		exec_mm_put_old(bprm->old_mm);
diff --git a/fs/file.c b/fs/file.c
index be06e09df41c..88ede1262b3e 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -884,7 +884,8 @@ struct file *file_close_fd(unsigned int fd)
 	return file;
 }
 
-void close_cloexec_files(struct files_struct *files)
+/* The caller drains @list with fput_list() once it dropped its locks. */
+void close_cloexec_files(struct files_struct *files, struct llist_head *list)
 {
 	unsigned i;
 	struct fdtable *fdt;
@@ -911,7 +912,8 @@ void close_cloexec_files(struct files_struct *files)
 			rcu_assign_pointer(fdt->fd[fd], NULL);
 			__put_unused_fd(files, fd);
 			spin_unlock(&files->file_lock);
-			filp_close(file, files);
+			filp_flush(file, files);
+			fput_close_list(file, list);
 			cond_resched();
 			spin_lock(&files->file_lock);
 		}
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 2e87faf9a8c2..be935320f1c7 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -4,6 +4,7 @@
 
 #include <linux/sched.h>
 #include <linux/unistd.h>
+#include <linux/llist.h>
 #include <asm/exec.h>
 #include <uapi/linux/binfmts.h>
 
@@ -81,6 +82,9 @@ struct linux_binprm {
 	int execfd;		/* File descriptor of the executable */
 	unsigned long exec;
 
+	/* Close-on-exec files awaiting their final __fput(), see free_bprm(). */
+	struct llist_head cloexec_files;
+
 	struct rlimit rlim_stack; /* Saved RLIMIT_STACK used during exec. */
 
 	char buf[BINPRM_BUF_SIZE];
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index e6b641d9eb91..050f4a04157e 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -105,7 +105,7 @@ struct fd_range {
 	unsigned int from, to;
 };
 struct files_struct *dup_fd(struct files_struct *, struct fd_range *) __latent_entropy;
-void close_cloexec_files(struct files_struct *);
+void close_cloexec_files(struct files_struct *, struct llist_head *);
 int iterate_fd(struct files_struct *, unsigned,
 		int (*)(const void *, struct file *, unsigned),
 		const void *);

-- 
2.53.0

Re: [PATCH RFC v2 04/16] fs: defer the final fput of close-on-exec files past the exec locks

[Mateusz Guzik]

On Wed, Sep 2, 2026 at 5:58 PM Christian Brauner <brauner@kernel.org> wrote:
>
> Taks with lots of file descriptors pay a hefy price for offloading to
> task work. Port them to the fput_list() mechanism instead and waste
> files in free_binprm().
>
> Link: https://lore.kernel.org/CAGudoHGRvxd-EwwDasqoVhk5VbEvhtjjM7kCXBfFyQ8ykeC9CQ@mail.gmail.com
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
>  fs/exec.c               | 4 +++-
>  fs/file.c               | 6 ++++--
>  include/linux/binfmts.h | 4 ++++
>  include/linux/fdtable.h | 2 +-
>  4 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index 92a02bce11d5..b1f449dba258 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1220,7 +1220,7 @@ int begin_new_exec(struct linux_binprm * bprm)
>          * trying to access the should-be-closed file descriptors of a process
>          * undergoing exec(2).
>          */
> -       close_cloexec_files(me->files);
> +       close_cloexec_files(me->files, &bprm->cloexec_files);
>
>         if (bprm->secureexec) {
>                 /* Make sure parent cannot signal privileged process. */
> @@ -1469,6 +1469,8 @@ static void free_bprm(struct linux_binprm *bprm)
>                 mutex_unlock(&current->signal->cred_guard_mutex);
>                 abort_creds(bprm->cred);
>         }
> +       /* The exec locks are out of the way now, see close_cloexec_files(). */
> +       fput_list(&bprm->cloexec_files);
>         /* exec swapped the mm but failed before setup_new_exec() freed it */
>         if (bprm->old_mm)
>                 exec_mm_put_old(bprm->old_mm);
> diff --git a/fs/file.c b/fs/file.c
> index be06e09df41c..88ede1262b3e 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -884,7 +884,8 @@ struct file *file_close_fd(unsigned int fd)
>         return file;
>  }
>
> -void close_cloexec_files(struct files_struct *files)
> +/* The caller drains @list with fput_list() once it dropped its locks. */
> +void close_cloexec_files(struct files_struct *files, struct llist_head *list)
>  {
>         unsigned i;
>         struct fdtable *fdt;
> @@ -911,7 +912,8 @@ void close_cloexec_files(struct files_struct *files)
>                         rcu_assign_pointer(fdt->fd[fd], NULL);
>                         __put_unused_fd(files, fd);
>                         spin_unlock(&files->file_lock);
> -                       filp_close(file, files);
> +                       filp_flush(file, files);
> +                       fput_close_list(file, list);
>                         cond_resched();
>                         spin_lock(&files->file_lock);
>                 }
> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
> index 2e87faf9a8c2..be935320f1c7 100644
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -4,6 +4,7 @@
>
>  #include <linux/sched.h>
>  #include <linux/unistd.h>
> +#include <linux/llist.h>
>  #include <asm/exec.h>
>  #include <uapi/linux/binfmts.h>
>
> @@ -81,6 +82,9 @@ struct linux_binprm {
>         int execfd;             /* File descriptor of the executable */
>         unsigned long exec;
>
> +       /* Close-on-exec files awaiting their final __fput(), see free_bprm(). */
> +       struct llist_head cloexec_files;
> +
>         struct rlimit rlim_stack; /* Saved RLIMIT_STACK used during exec. */
>
>         char buf[BINPRM_BUF_SIZE];
> diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
> index e6b641d9eb91..050f4a04157e 100644
> --- a/include/linux/fdtable.h
> +++ b/include/linux/fdtable.h
> @@ -105,7 +105,7 @@ struct fd_range {
>         unsigned int from, to;
>  };
>  struct files_struct *dup_fd(struct files_struct *, struct fd_range *) __latent_entropy;
> -void close_cloexec_files(struct files_struct *);
> +void close_cloexec_files(struct files_struct *, struct llist_head *);
>  int iterate_fd(struct files_struct *, unsigned,
>                 int (*)(const void *, struct file *, unsigned),
>                 const void *);
>
> --
> 2.53.0
>

The patch looks fine in its own right, but there is a pre-existing
deadlock reported and I think this will need adjustment to take care
of it:
https://lore.kernel.org/linux-fsdevel/f5e8166a-88be-46c5-8939-1e5227ffe4c2@app.fastmail.com/T/#u
* deadlock from exec_update_lock and reading procfs

[PATCH RFC v2 05/16] fs: add switch_files_struct()

[Christian Brauner]

Add switch_files_struct() to install another table on a task. It
consumes the reference to the new table and hands the old one back for
the caller to put. Convert every place that switches a descriptor table.

No functional changes.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/file.c               | 28 ++++++++++++++++------------
 include/linux/fdtable.h |  2 ++
 kernel/fork.c           |  6 ++----
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index 88ede1262b3e..aec2adf59031 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -515,16 +515,23 @@ void put_files_struct(struct files_struct *files)
 	}
 }
 
-void exit_files(struct task_struct *tsk)
+/* Install @files on @tsk, consuming the reference; returns the old table. */
+struct files_struct *switch_files_struct(struct task_struct *tsk,
+					 struct files_struct *files)
 {
-	struct files_struct * files = tsk->files;
+	struct files_struct *old;
 
-	if (files) {
-		task_lock(tsk);
-		tsk->files = NULL;
-		task_unlock(tsk);
-		put_files_struct(files);
-	}
+	task_lock(tsk);
+	old = tsk->files;
+	tsk->files = files;
+	task_unlock(tsk);
+	return old;
+}
+
+void exit_files(struct task_struct *tsk)
+{
+	if (tsk->files)
+		put_files_struct(switch_files_struct(tsk, NULL));
 }
 
 struct files_struct init_files = {
@@ -855,10 +862,7 @@ SYSCALL_DEFINE3(close_range, unsigned int, fd, unsigned int, max_fd,
 		 * We're done closing the files we were supposed to. Time to install
 		 * the new file descriptor table and drop the old one.
 		 */
-		task_lock(me);
-		me->files = cur_fds;
-		task_unlock(me);
-		put_files_struct(fds);
+		put_files_struct(switch_files_struct(me, cur_fds));
 	}
 
 	return 0;
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index 050f4a04157e..80f192412a5a 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -100,6 +100,8 @@ static inline bool close_on_exec(unsigned int fd, const struct files_struct *fil
 struct task_struct;
 
 void put_files_struct(struct files_struct *fs);
+struct files_struct *switch_files_struct(struct task_struct *tsk,
+					 struct files_struct *files);
 int unshare_files(void);
 struct fd_range {
 	unsigned int from, to;
diff --git a/kernel/fork.c b/kernel/fork.c
index 416758c8a3d4..b959191edf52 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -3320,10 +3320,8 @@ int ksys_unshare(unsigned long unshare_flags)
 		if (new_fs)
 			new_fs = switch_fs_struct(new_fs);
 
-		if (new_fd) {
-			guard(task_lock)(current);
-			swap(current->files, new_fd);
-		}
+		if (new_fd)
+			new_fd = switch_files_struct(current, new_fd);
 
 		if (new_cred) {
 			/* Install the new user namespace */

-- 
2.53.0

[PATCH RFC v2 06/16] fs: move unshare_fd() to fs/file.c

[Christian Brauner]

Move dup_fd() and switch_files_struct() where the rest of the
descriptor table lifecycle helpers live.

No functional changes.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/file.c               | 18 ++++++++++++++++++
 include/linux/fdtable.h |  1 +
 kernel/fork.c           | 18 ------------------
 3 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index aec2adf59031..185de8deb39d 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -471,6 +471,24 @@ struct files_struct *dup_fd(struct files_struct *oldf, struct fd_range *punch_ho
 	return newf;
 }
 
+/*
+ * Unshare file descriptor table if it is being shared
+ */
+int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp)
+{
+	struct files_struct *fd = current->files;
+
+	if ((unshare_flags & CLONE_FILES) &&
+	    (fd && atomic_read(&fd->count) > 1)) {
+		fd = dup_fd(fd, NULL);
+		if (IS_ERR(fd))
+			return PTR_ERR(fd);
+		*new_fdp = fd;
+	}
+
+	return 0;
+}
+
 static struct fdtable *close_files(struct files_struct * files)
 {
 	/*
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index 80f192412a5a..102fe4c20b7d 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -103,6 +103,7 @@ void put_files_struct(struct files_struct *fs);
 struct files_struct *switch_files_struct(struct task_struct *tsk,
 					 struct files_struct *files);
 int unshare_files(void);
+int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp);
 struct fd_range {
 	unsigned int from, to;
 };
diff --git a/kernel/fork.c b/kernel/fork.c
index b959191edf52..c456958c51f4 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -3207,24 +3207,6 @@ static int unshare_fs(unsigned long unshare_flags, struct fs_struct **new_fsp)
 	return 0;
 }
 
-/*
- * Unshare file descriptor table if it is being shared
- */
-static int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp)
-{
-	struct files_struct *fd = current->files;
-
-	if ((unshare_flags & CLONE_FILES) &&
-	    (fd && atomic_read(&fd->count) > 1)) {
-		fd = dup_fd(fd, NULL);
-		if (IS_ERR(fd))
-			return PTR_ERR(fd);
-		*new_fdp = fd;
-	}
-
-	return 0;
-}
-
 /*
  * unshare allows a process to 'unshare' part of the process
  * context which was originally shared using clone.  copy_*

-- 
2.53.0

Re: [PATCH RFC v2 06/16] fs: move unshare_fd() to fs/file.c

[Mateusz Guzik]

On Wed, Sep 2, 2026 at 5:58 PM Christian Brauner <brauner@kernel.org> wrote:
>
> Move dup_fd() and switch_files_struct() where the rest of the
> descriptor table lifecycle helpers live.
>
> No functional changes.
>
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
>  fs/file.c               | 18 ++++++++++++++++++
>  include/linux/fdtable.h |  1 +
>  kernel/fork.c           | 18 ------------------
>  3 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/fs/file.c b/fs/file.c
> index aec2adf59031..185de8deb39d 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -471,6 +471,24 @@ struct files_struct *dup_fd(struct files_struct *oldf, struct fd_range *punch_ho
>         return newf;
>  }
>
> +/*
> + * Unshare file descriptor table if it is being shared
> + */
> +int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp)
> +{
> +       struct files_struct *fd = current->files;
> +
> +       if ((unshare_flags & CLONE_FILES) &&
> +           (fd && atomic_read(&fd->count) > 1)) {
> +               fd = dup_fd(fd, NULL);
> +               if (IS_ERR(fd))
> +                       return PTR_ERR(fd);
> +               *new_fdp = fd;
> +       }
> +
> +       return 0;
> +}
> +
>  static struct fdtable *close_files(struct files_struct * files)
>  {
>         /*
> diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
> index 80f192412a5a..102fe4c20b7d 100644
> --- a/include/linux/fdtable.h
> +++ b/include/linux/fdtable.h
> @@ -103,6 +103,7 @@ void put_files_struct(struct files_struct *fs);
>  struct files_struct *switch_files_struct(struct task_struct *tsk,
>                                          struct files_struct *files);
>  int unshare_files(void);
> +int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp);
>  struct fd_range {
>         unsigned int from, to;
>  };
> diff --git a/kernel/fork.c b/kernel/fork.c
> index b959191edf52..c456958c51f4 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -3207,24 +3207,6 @@ static int unshare_fs(unsigned long unshare_flags, struct fs_struct **new_fsp)
>         return 0;
>  }
>
> -/*
> - * Unshare file descriptor table if it is being shared
> - */
> -static int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp)
> -{
> -       struct files_struct *fd = current->files;
> -
> -       if ((unshare_flags & CLONE_FILES) &&
> -           (fd && atomic_read(&fd->count) > 1)) {
> -               fd = dup_fd(fd, NULL);
> -               if (IS_ERR(fd))
> -                       return PTR_ERR(fd);
> -               *new_fdp = fd;
> -       }
> -
> -       return 0;
> -}
> -
>  /*
>   * unshare allows a process to 'unshare' part of the process
>   * context which was originally shared using clone.  copy_*
>
> --
> 2.53.0
>

Reviewed-by: Mateusz Guzik <mjguzik@gmail.com>

[PATCH RFC v2 07/16] fs: remove unshare_files()

[Christian Brauner]

exec is the only caller left since commit 433967cab51e ("coredump: stop
unsharing the file descriptor table"). All it does is call unshare_fd()
with CLONE_FILES and install the copy. Kill the pointless helper and
open-code it.

No functional changes.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/exec.c               |  5 ++++-
 include/linux/fdtable.h |  1 -
 kernel/fork.c           | 24 ------------------------
 3 files changed, 4 insertions(+), 26 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index b1f449dba258..713819244d23 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1124,6 +1124,7 @@ static struct file *bprm_identity_file(const struct linux_binprm *bprm)
 int begin_new_exec(struct linux_binprm * bprm)
 {
 	struct task_struct *me = current;
+	struct files_struct *files = NULL;
 	int retval;
 
 	/* A pending PT_INTERP substitution this format cannot consume. */
@@ -1160,9 +1161,11 @@ int begin_new_exec(struct linux_binprm * bprm)
 	io_uring_task_cancel();
 
 	/* Ensure the files table is not shared. */
-	retval = unshare_files();
+	retval = unshare_fd(CLONE_FILES, &files);
 	if (retval)
 		goto out;
+	if (files)
+		put_files_struct(switch_files_struct(me, files));
 
 	/*
 	 * Must be called _before_ exec_mmap() as bprm->mm is
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index 102fe4c20b7d..15ee6292e0e3 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -102,7 +102,6 @@ struct task_struct;
 void put_files_struct(struct files_struct *fs);
 struct files_struct *switch_files_struct(struct task_struct *tsk,
 					 struct files_struct *files);
-int unshare_files(void);
 int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp);
 struct fd_range {
 	unsigned int from, to;
diff --git a/kernel/fork.c b/kernel/fork.c
index c456958c51f4..ba74705ca191 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -3336,30 +3336,6 @@ SYSCALL_DEFINE1(unshare, unsigned long, unshare_flags)
 	return ksys_unshare(unshare_flags);
 }
 
-/*
- *	Helper to unshare the files of the current task.
- *	We don't want to expose copy_files internals to
- *	the exec layer of the kernel.
- */
-
-int unshare_files(void)
-{
-	struct task_struct *task = current;
-	struct files_struct *old, *copy = NULL;
-	int error;
-
-	error = unshare_fd(CLONE_FILES, &copy);
-	if (error || !copy)
-		return error;
-
-	old = task->files;
-	task_lock(task);
-	task->files = copy;
-	task_unlock(task);
-	put_files_struct(old);
-	return 0;
-}
-
 static int sysctl_max_threads(const struct ctl_table *table, int write,
 		       void *buffer, size_t *lenp, loff_t *ppos)
 {

-- 
2.53.0

Re: [PATCH RFC v2 07/16] fs: remove unshare_files()

[Mateusz Guzik]

On Wed, Sep 2, 2026 at 5:58 PM Christian Brauner <brauner@kernel.org> wrote:
>
> exec is the only caller left since commit 433967cab51e ("coredump: stop
> unsharing the file descriptor table"). All it does is call unshare_fd()
> with CLONE_FILES and install the copy. Kill the pointless helper and
> open-code it.
>
> No functional changes.
>
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
>  fs/exec.c               |  5 ++++-
>  include/linux/fdtable.h |  1 -
>  kernel/fork.c           | 24 ------------------------
>  3 files changed, 4 insertions(+), 26 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index b1f449dba258..713819244d23 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1124,6 +1124,7 @@ static struct file *bprm_identity_file(const struct linux_binprm *bprm)
>  int begin_new_exec(struct linux_binprm * bprm)
>  {
>         struct task_struct *me = current;
> +       struct files_struct *files = NULL;
>         int retval;
>
>         /* A pending PT_INTERP substitution this format cannot consume. */
> @@ -1160,9 +1161,11 @@ int begin_new_exec(struct linux_binprm * bprm)
>         io_uring_task_cancel();
>
>         /* Ensure the files table is not shared. */
> -       retval = unshare_files();
> +       retval = unshare_fd(CLONE_FILES, &files);
>         if (retval)
>                 goto out;
> +       if (files)
> +               put_files_struct(switch_files_struct(me, files));
>
>         /*
>          * Must be called _before_ exec_mmap() as bprm->mm is
> diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
> index 102fe4c20b7d..15ee6292e0e3 100644
> --- a/include/linux/fdtable.h
> +++ b/include/linux/fdtable.h
> @@ -102,7 +102,6 @@ struct task_struct;
>  void put_files_struct(struct files_struct *fs);
>  struct files_struct *switch_files_struct(struct task_struct *tsk,
>                                          struct files_struct *files);
> -int unshare_files(void);
>  int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp);
>  struct fd_range {
>         unsigned int from, to;
> diff --git a/kernel/fork.c b/kernel/fork.c
> index c456958c51f4..ba74705ca191 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -3336,30 +3336,6 @@ SYSCALL_DEFINE1(unshare, unsigned long, unshare_flags)
>         return ksys_unshare(unshare_flags);
>  }
>
> -/*
> - *     Helper to unshare the files of the current task.
> - *     We don't want to expose copy_files internals to
> - *     the exec layer of the kernel.
> - */
> -
> -int unshare_files(void)
> -{
> -       struct task_struct *task = current;
> -       struct files_struct *old, *copy = NULL;
> -       int error;
> -
> -       error = unshare_fd(CLONE_FILES, &copy);
> -       if (error || !copy)
> -               return error;
> -
> -       old = task->files;
> -       task_lock(task);
> -       task->files = copy;
> -       task_unlock(task);
> -       put_files_struct(old);
> -       return 0;
> -}
> -
>  static int sysctl_max_threads(const struct ctl_table *table, int write,
>                        void *buffer, size_t *lenp, loff_t *ppos)
>  {
>
> --
> 2.53.0
>

Reviewed-by: Mateusz Guzik <mjguzik@gmail.com>

[PATCH RFC v2 08/16] exec: release fdtable after exec locks drop

[Christian Brauner]

During exec the task gets a private fdtable and releases the shared
fdtable. This all happens under cred_guard_mutex. If the exec'ing task
has the last reference on the fdtable close_files() closes all files.
Today this happens to be deferred to task work. Following patches will
make that part synchronous so the final put needs to move out of the
cred_guard_mutex.

The shared table now lives until free_bprm() instead of dying in
begin_new_exec(). This doesn't matter as nothing can get access to the
old table anymore and closing of files would've been deferred to task
work anyway.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/exec.c               | 6 ++++--
 include/linux/binfmts.h | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 713819244d23..be8601d9c691 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1165,7 +1165,7 @@ int begin_new_exec(struct linux_binprm * bprm)
 	if (retval)
 		goto out;
 	if (files)
-		put_files_struct(switch_files_struct(me, files));
+		bprm->old_files = switch_files_struct(me, files);
 
 	/*
 	 * Must be called _before_ exec_mmap() as bprm->mm is
@@ -1472,7 +1472,9 @@ static void free_bprm(struct linux_binprm *bprm)
 		mutex_unlock(&current->signal->cred_guard_mutex);
 		abort_creds(bprm->cred);
 	}
-	/* The exec locks are out of the way now, see close_cloexec_files(). */
+	/* The exec locks are gone, release what begin_new_exec() closed. */
+	if (bprm->old_files)
+		put_files_struct(bprm->old_files);
 	fput_list(&bprm->cloexec_files);
 	/* exec swapped the mm but failed before setup_new_exec() freed it */
 	if (bprm->old_mm)
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index be935320f1c7..0f0fcf859b4f 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -37,6 +37,8 @@ struct linux_binprm {
 #endif
 	struct mm_struct *mm;
 	struct mm_struct *old_mm;	/* replaced address space, freed by setup_new_exec() */
+	/* Descriptor table displaced by begin_new_exec(), put by free_bprm(). */
+	struct files_struct *old_files;
 	/* user_ns published to task->exec_state at execve, narrowed by would_dump(). */
 	struct user_namespace *user_ns;
 	unsigned long p; /* current top of mem */

-- 
2.53.0

[PATCH RFC v2 09/16] fs: make close_files() synchronous

[Christian Brauner]

When the last reference to a descriptor table is dropped close_files()
closes every file but punts the actual work to task work. For an exiting
task that task work only runs in exit_task_work().

Before commit 4a9d4b024a31 ("switch fput to task_work_add") fput() was
synchronous everywhere and exit released its files in exit_files().

The deferral made fput() safe from any context. And exit_files()
offloaded to task work as a side-effect. And that has downsides.

Oleg and Neil noticed that some time ago. A task that exits with a big
descriptor table ends up queueing a very large number of files on task
work. That walks the task work list under ->pi_lock and costs a lot of
atomics too.

Let close_files() close right away. The walk puts the last references
through filp_close_list(). If it's the last reference it places them on
a private list and drains it through fput_list(). Note, this keeps keeps
the exact order the deferred path had. Every filp_flush() first and then
the final __fput()s in the reverse order task_work_run() giving the same
ordering even.

Every put of a dying table is synchronous now:

- exit_files()
- copy_process()
- close_range(CLOSE_RANGE_UNSHARE)
- unshare(2)
- exec

Kernel threads don't own a file descriptor table and exec already splats
where they to exec. kthreadd and every kthread share init_files and
init_task pins that forever.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/file.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index 185de8deb39d..47ce844be95b 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -489,7 +489,7 @@ int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp)
 	return 0;
 }
 
-static struct fdtable *close_files(struct files_struct * files)
+static struct fdtable *close_files(struct files_struct *files)
 {
 	/*
 	 * It is safe to dereference the fd table without RCU or
@@ -498,6 +498,7 @@ static struct fdtable *close_files(struct files_struct * files)
 	 */
 	struct fdtable *fdt = rcu_dereference_raw(files->fdt);
 	unsigned int i, j = 0;
+	LLIST_HEAD(to_close);
 
 	for (;;) {
 		unsigned long set;
@@ -509,7 +510,8 @@ static struct fdtable *close_files(struct files_struct * files)
 			if (set & 1) {
 				struct file *file = fdt->fd[i];
 				if (file) {
-					filp_close(file, files);
+					filp_flush(file, files);
+					fput_close_list(file, &to_close);
 					cond_resched();
 				}
 			}
@@ -518,6 +520,7 @@ static struct fdtable *close_files(struct files_struct * files)
 		}
 	}
 
+	fput_list(&to_close);
 	return fdt;
 }
 

-- 
2.53.0

[PATCH RFC v2 10/16] fs: make close_range() synchronous

[Christian Brauner]

__range_close() closes through filp_close() so every file the caller
held the last reference to is punted to task work. That costs one
cmpxchg per file plus a list entry for any later task_work_cancel() to
search under ->pi_lock. close_range(2) exists to close many descriptors
in one go fast. So convert it to the same synchronous treatment as
close(2) and close_files().

We collect the last references through filp_close_list() and run any
final __fput()s through fput_list() right before returning.
close_range(2) now behaves like close(2).

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/file.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index 47ce844be95b..608e6a1fa69b 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -798,7 +798,7 @@ static inline void __range_cloexec(struct files_struct *cur_fds,
 }
 
 static inline void __range_close(struct files_struct *files, unsigned int fd,
-				 unsigned int max_fd)
+				 unsigned int max_fd, struct llist_head *list)
 {
 	struct file *file;
 	struct fdtable *fdt;
@@ -815,7 +815,8 @@ static inline void __range_close(struct files_struct *files, unsigned int fd,
 		file = file_close_fd_locked(files, fd);
 		if (file) {
 			spin_unlock(&files->file_lock);
-			filp_close(file, files);
+			filp_flush(file, files);
+			fput_close_list(file, list);
 			cond_resched();
 			spin_lock(&files->file_lock);
 			fdt = files_fdtable(files);
@@ -845,6 +846,7 @@ SYSCALL_DEFINE3(close_range, unsigned int, fd, unsigned int, max_fd,
 {
 	struct task_struct *me = current;
 	struct files_struct *cur_fds = me->files, *fds = NULL;
+	LLIST_HEAD(to_close);
 
 	if (flags & ~(CLOSE_RANGE_UNSHARE | CLOSE_RANGE_CLOEXEC))
 		return -EINVAL;
@@ -876,7 +878,7 @@ SYSCALL_DEFINE3(close_range, unsigned int, fd, unsigned int, max_fd,
 	if (flags & CLOSE_RANGE_CLOEXEC)
 		__range_cloexec(cur_fds, fd, max_fd);
 	else
-		__range_close(cur_fds, fd, max_fd);
+		__range_close(cur_fds, fd, max_fd, &to_close);
 
 	if (fds) {
 		/*
@@ -886,6 +888,7 @@ SYSCALL_DEFINE3(close_range, unsigned int, fd, unsigned int, max_fd,
 		put_files_struct(switch_files_struct(me, cur_fds));
 	}
 
+	fput_list(&to_close);
 	return 0;
 }
 

-- 
2.53.0

[PATCH RFC v2 11/16] coredump: s/startup/done/g

[Christian Brauner]

We're going to use that in a bit. Rename it to something easier to
understand.

No functional changes.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/coredump.c                | 4 ++--
 include/linux/sched/signal.h | 3 ++-
 kernel/exit.c                | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index 9addd2d59b7b..545a6df48ea8 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -523,7 +523,7 @@ static int coredump_wait(int exit_code, struct core_state *core_state)
 	struct task_struct *tsk = current;
 	int core_waiters = -EBUSY;
 
-	init_completion(&core_state->startup);
+	init_completion(&core_state->done);
 	core_state->dumper.task = tsk;
 	core_state->dumper.next = NULL;
 
@@ -531,7 +531,7 @@ static int coredump_wait(int exit_code, struct core_state *core_state)
 	if (core_waiters > 0) {
 		struct core_thread *ptr;
 
-		wait_for_completion_state(&core_state->startup,
+		wait_for_completion_state(&core_state->done,
 					  TASK_UNINTERRUPTIBLE|TASK_FREEZABLE);
 		/*
 		 * Wait for all the threads to become inactive, so that
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 584ae88b435e..ed9e2d49d166 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -81,7 +81,8 @@ struct core_thread {
 struct core_state {
 	atomic_t nr_threads;
 	struct core_thread dumper;
-	struct completion startup;
+	/* Completed by the last thread to park. */
+	struct completion done;
 };
 
 /*
diff --git a/kernel/exit.c b/kernel/exit.c
index 97686af89501..8702e3ead7dd 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -440,7 +440,7 @@ static void coredump_task_exit(struct task_struct *tsk,
 	 * to core_state->dumper.
 	 */
 	if (atomic_dec_and_test(&core_state->nr_threads))
-		complete(&core_state->startup);
+		complete(&core_state->done);
 
 	for (;;) {
 		set_current_state(TASK_IDLE|TASK_FREEZABLE);

-- 
2.53.0

[PATCH RFC v2 12/16] coredump: factor out coredump_wait_inactive()

[Christian Brauner]

Factor out a new  coredump_wait_inactive() helper that
COREDUMP_CLOSE_FILES can consume in a bit.

No functional changes.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/coredump.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index 545a6df48ea8..d0796536e03f 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -518,6 +518,24 @@ static int zap_threads(struct task_struct *tsk,
 	return nr;
 }
 
+static void coredump_wait_inactive(struct core_state *core_state)
+{
+	struct core_thread *ptr;
+
+	wait_for_completion_state(&core_state->done,
+				  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;
+	while (ptr != NULL) {
+		wait_task_inactive(ptr->task, TASK_ANY);
+		ptr = ptr->next;
+	}
+}
+
 static int coredump_wait(int exit_code, struct core_state *core_state)
 {
 	struct task_struct *tsk = current;
@@ -528,22 +546,8 @@ static int coredump_wait(int exit_code, struct core_state *core_state)
 	core_state->dumper.next = NULL;
 
 	core_waiters = zap_threads(tsk, core_state, exit_code);
-	if (core_waiters > 0) {
-		struct core_thread *ptr;
-
-		wait_for_completion_state(&core_state->done,
-					  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;
-		while (ptr != NULL) {
-			wait_task_inactive(ptr->task, TASK_ANY);
-			ptr = ptr->next;
-		}
-	}
+	if (core_waiters > 0)
+		coredump_wait_inactive(core_state);
 
 	return core_waiters;
 }

-- 
2.53.0

[PATCH RFC v2 13/16] fs: add alloc_files_struct()

[Christian Brauner]

Add a way to get an empty descriptor table with one reference. Let
dup_fd() use that helper. The coredump code will use it too in a bit.

No functional changes.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/file.c               | 49 ++++++++++++++++++++++++++++++++++++++-----------
 include/linux/fdtable.h |  1 +
 2 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index 608e6a1fa69b..ccbaa71dfbe4 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -375,21 +375,15 @@ static unsigned int sane_fdtable_size(struct fdtable *fdt, struct fd_range *punc
 	return ALIGN(last + 1, BITS_PER_LONG);
 }
 
-/*
- * Allocate a new descriptor table and copy contents from the passed in
- * instance.  Returns a pointer to cloned table on success, ERR_PTR()
- * on failure.  For 'punch_hole' see sane_fdtable_size().
- */
-struct files_struct *dup_fd(struct files_struct *oldf, struct fd_range *punch_hole)
+/* A table with one reference and the embedded fdtable, nothing copied yet. */
+static struct files_struct *alloc_files(gfp_t gfp)
 {
 	struct files_struct *newf;
-	struct file **old_fds, **new_fds;
-	unsigned int open_files, i;
-	struct fdtable *old_fdt, *new_fdt;
+	struct fdtable *new_fdt;
 
-	newf = kmem_cache_alloc(files_cachep, GFP_KERNEL);
+	newf = kmem_cache_alloc(files_cachep, gfp);
 	if (!newf)
-		return ERR_PTR(-ENOMEM);
+		return NULL;
 
 	atomic_set(&newf->count, 1);
 
@@ -404,6 +398,39 @@ struct files_struct *dup_fd(struct files_struct *oldf, struct fd_range *punch_ho
 	new_fdt->full_fds_bits = newf->full_fds_bits_init;
 	new_fdt->fd = &newf->fd_array[0];
 
+	return newf;
+}
+
+/* An empty descriptor table with one reference. */
+struct files_struct *alloc_files_struct(void)
+{
+	struct files_struct *newf;
+
+	newf = alloc_files(GFP_KERNEL | __GFP_ZERO);
+	if (!newf)
+		return NULL;
+
+	rcu_assign_pointer(newf->fdt, &newf->fdtab);
+	return newf;
+}
+
+/*
+ * Allocate a new descriptor table and copy contents from the passed in
+ * instance.  Returns a pointer to cloned table on success, ERR_PTR()
+ * on failure.  For 'punch_hole' see sane_fdtable_size().
+ */
+struct files_struct *dup_fd(struct files_struct *oldf, struct fd_range *punch_hole)
+{
+	struct files_struct *newf;
+	struct file **old_fds, **new_fds;
+	unsigned int open_files, i;
+	struct fdtable *old_fdt, *new_fdt;
+
+	newf = alloc_files(GFP_KERNEL);
+	if (!newf)
+		return ERR_PTR(-ENOMEM);
+	new_fdt = &newf->fdtab;
+
 	spin_lock(&oldf->file_lock);
 	old_fdt = files_fdtable(oldf);
 	open_files = sane_fdtable_size(old_fdt, punch_hole);
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index 15ee6292e0e3..811994b16bed 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -100,6 +100,7 @@ static inline bool close_on_exec(unsigned int fd, const struct files_struct *fil
 struct task_struct;
 
 void put_files_struct(struct files_struct *fs);
+struct files_struct *alloc_files_struct(void);
 struct files_struct *switch_files_struct(struct task_struct *tsk,
 					 struct files_struct *files);
 int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp);

-- 
2.53.0

[PATCH RFC v2 14/16] coredump: add COREDUMP_CLOSE_FILES

[Christian Brauner]

Add COREDUMP_CLOSE_FILES and allow a coredump server to request that the
thread-group closes all files before creating the coredump.

There have been several attempts to let the dumping process decide
through a new fcntl() flag, a new procfs file or a new coredump_filter
bit that its descriptors go away early. That's just broken imho.

Tools like systemd-coredump walk /proc/<pid>/fd and /proc/<pid>/fdinfo
and some use pidfd_getfd() to preserve files of the crashing process.
Only the coredump server knows whether it still needs the descriptors.

So let the coredump server ask for it. Add a new COREDUMP_CLOSE_FILES
feature bit. If the coredump server raises it the kernel drops the
descriptor tables of the thread group right after the handshake and
before it generates the coredump.

COREDUMP_CLOSE_FILES doesn't work with COREDUMP_REJECT. A rejected task
exits and closes everything right away anyway.

Reported-by: Xin Zhao <jackzxcui1989@163.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/coredump.c                 | 55 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/sched/signal.h  |  4 +++-
 include/uapi/linux/coredump.h |  8 +++++++
 kernel/exit.c                 | 13 ++++++++++
 4 files changed, 78 insertions(+), 2 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index d0796536e03f..506410132e30 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -552,6 +552,48 @@ static int coredump_wait(int exit_code, struct core_state *core_state)
 	return core_waiters;
 }
 
+/*
+ * Allocate a new empty fdtable and switch the whole thread-group to it.
+ * Put all the old fdtables freeing up resources and locks before writing the
+ * coredump.
+ */
+static bool coredump_close_files(struct core_state *core_state)
+{
+	struct files_struct *files;
+	struct core_thread *ct;
+	int nr = 0;
+
+	files = alloc_files_struct();
+	if (!files)
+		return false;
+
+	/* Tasks without a table such as vhost workers can be skipped. */
+	for (ct = core_state->dumper.next; ct; ct = ct->next)
+		if (ct->task->files)
+			nr++;
+	atomic_set(&core_state->nr_threads, nr);
+	reinit_completion(&core_state->done);
+
+	for (ct = core_state->dumper.next; ct; ct = ct->next) {
+		if (!ct->task->files)
+			continue;
+		/* ct->files holds a reference until the thread switches to it. */
+		atomic_inc(&files->count);
+		/* Pairs with the acquire in coredump_task_exit(). */
+		smp_store_release(&ct->files, files);
+		wake_up_process(ct->task);
+	}
+
+	/* Use the dumper's real creds not the overriden ones. */
+	scoped_with_creds(current_real_cred())
+		put_files_struct(switch_files_struct(current, files));
+
+	if (nr)
+		coredump_wait_inactive(core_state);
+
+	return true;
+}
+
 static void coredump_finish(enum coredump_state state)
 {
 	struct core_thread *curr, *next;
@@ -841,7 +883,8 @@ static bool coredump_sock_request(struct core_name *cn, struct coredump_params *
 		.mask			= COREDUMP_KERNEL | COREDUMP_USERSPACE |
 					  COREDUMP_REJECT | COREDUMP_WAIT |
 					  COREDUMP_RECORDS | COREDUMP_SPARSE |
-					  COREDUMP_MEMORY_TYPES,
+					  COREDUMP_MEMORY_TYPES |
+					  COREDUMP_CLOSE_FILES,
 		.size_ack		= sizeof(struct coredump_ack),
 		.memory_types		= cprm->memory_types,
 		.memory_types_mask	= COREDUMP_MEMORY_ALL,
@@ -909,6 +952,12 @@ static bool coredump_sock_request(struct core_name *cn, struct coredump_params *
 		return false;
 	}
 
+	/* A rejected task exits right away and closes everything anyway. */
+	if ((ack.mask & COREDUMP_CLOSE_FILES) && (ack.mask & COREDUMP_REJECT)) {
+		coredump_sock_mark(cprm->file, COREDUMP_MARK_CONFLICTING);
+		return false;
+	}
+
 	if (ack.mask & COREDUMP_MEMORY_TYPES) {
 		/* The memory types need the whole field. */
 		if (usize < COREDUMP_ACK_SIZE_VER1) {
@@ -1224,6 +1273,10 @@ static void do_coredump(struct core_name *cn, struct coredump_params *cprm,
 	if (cprm->mask & COREDUMP_REJECT)
 		return;
 
+	if ((cprm->mask & COREDUMP_CLOSE_FILES) &&
+	    !coredump_close_files(current->signal->core_state))
+		return;
+
 	if ((cprm->mask & COREDUMP_KERNEL) && !coredump_write(cprm, binfmt))
 		return;
 
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index ed9e2d49d166..fed02a427de3 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -76,12 +76,14 @@ struct multiprocess_signals {
 struct core_thread {
 	struct task_struct *task;
 	struct core_thread *next;
+	/* The empty table to switch to, published by the dumping thread. */
+	struct files_struct *files;
 };
 
 struct core_state {
 	atomic_t nr_threads;
 	struct core_thread dumper;
-	/* Completed by the last thread to park. */
+	/* Completed by the last thread to park, reused for the table switch. */
 	struct completion done;
 };
 
diff --git a/include/uapi/linux/coredump.h b/include/uapi/linux/coredump.h
index 6d0c53b534ea..ec09d7ab0131 100644
--- a/include/uapi/linux/coredump.h
+++ b/include/uapi/linux/coredump.h
@@ -19,6 +19,9 @@
  * @COREDUMP_MEMORY_TYPES: dump the memory types in
  *                          coredump_ack->memory_types instead of the ones
  *                          the task selected; requires COREDUMP_KERNEL
+ * @COREDUMP_CLOSE_FILES: close all file descriptors of the task before the
+ *                        coredump is generated; incompatible with
+ *                        COREDUMP_REJECT
  */
 enum {
 	COREDUMP_KERNEL		= (1ULL << 0),
@@ -28,6 +31,7 @@ enum {
 	COREDUMP_RECORDS	= (1ULL << 4),
 	COREDUMP_SPARSE		= (1ULL << 5),
 	COREDUMP_MEMORY_TYPES	= (1ULL << 6),
+	COREDUMP_CLOSE_FILES	= (1ULL << 7),
 };
 
 /**
@@ -137,6 +141,10 @@ enum {
  * Note that @memory_types must be zero if COREDUMP_MEMORY_TYPES isn't
  * raised. COREDUMP_MEMORY_TYPES requires COREDUMP_KERNEL and an ack of
  * at least COREDUMP_ACK_SIZE_VER1 bytes.
+ *
+ * If COREDUMP_CLOSE_FILES is raised in @mask the kernel closes the file
+ * descriptors of the coredumping task before it generates the coredump.
+ * The task ends up with an empty descriptor table.
  */
 struct coredump_ack {
 	__u32 size;
diff --git a/kernel/exit.c b/kernel/exit.c
index 8702e3ead7dd..70d9a862e38c 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -25,6 +25,7 @@
 #include <linux/acct.h>
 #include <linux/tsacct_kern.h>
 #include <linux/file.h>
+#include <linux/fdtable.h>
 #include <linux/freezer.h>
 #include <linux/binfmts.h>
 #include <linux/nsproxy.h>
@@ -428,9 +429,11 @@ kill_orphaned_pgrp(struct task_struct *tsk, struct task_struct *parent)
 static void coredump_task_exit(struct task_struct *tsk,
 			       struct core_state *core_state)
 {
+	struct files_struct *files;
 	struct core_thread self;
 
 	self.task = tsk;
+	self.files = NULL;
 	if (self.task->flags & PF_SIGNALED)
 		self.next = xchg(&core_state->dumper.next, &self);
 	else
@@ -446,6 +449,16 @@ static void coredump_task_exit(struct task_struct *tsk,
 		set_current_state(TASK_IDLE|TASK_FREEZABLE);
 		if (!self.task) /* see coredump_finish() */
 			break;
+		/* Pairs with the release in coredump_close_files(). */
+		files = smp_load_acquire(&self.files);
+		if (files) {
+			__set_current_state(TASK_RUNNING);
+			self.files = NULL;
+			put_files_struct(switch_files_struct(tsk, files));
+			if (atomic_dec_and_test(&core_state->nr_threads))
+				complete(&core_state->done);
+			continue;
+		}
 		schedule();
 	}
 	__set_current_state(TASK_RUNNING);

-- 
2.53.0

[PATCH RFC v2 15/16] tools: sync coredump.h header

[Christian Brauner]

Sync the headers for the selftests.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 tools/include/uapi/linux/coredump.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/include/uapi/linux/coredump.h b/tools/include/uapi/linux/coredump.h
index 6d0c53b534ea..ec09d7ab0131 100644
--- a/tools/include/uapi/linux/coredump.h
+++ b/tools/include/uapi/linux/coredump.h
@@ -19,6 +19,9 @@
  * @COREDUMP_MEMORY_TYPES: dump the memory types in
  *                          coredump_ack->memory_types instead of the ones
  *                          the task selected; requires COREDUMP_KERNEL
+ * @COREDUMP_CLOSE_FILES: close all file descriptors of the task before the
+ *                        coredump is generated; incompatible with
+ *                        COREDUMP_REJECT
  */
 enum {
 	COREDUMP_KERNEL		= (1ULL << 0),
@@ -28,6 +31,7 @@ enum {
 	COREDUMP_RECORDS	= (1ULL << 4),
 	COREDUMP_SPARSE		= (1ULL << 5),
 	COREDUMP_MEMORY_TYPES	= (1ULL << 6),
+	COREDUMP_CLOSE_FILES	= (1ULL << 7),
 };
 
 /**
@@ -137,6 +141,10 @@ enum {
  * Note that @memory_types must be zero if COREDUMP_MEMORY_TYPES isn't
  * raised. COREDUMP_MEMORY_TYPES requires COREDUMP_KERNEL and an ack of
  * at least COREDUMP_ACK_SIZE_VER1 bytes.
+ *
+ * If COREDUMP_CLOSE_FILES is raised in @mask the kernel closes the file
+ * descriptors of the coredumping task before it generates the coredump.
+ * The task ends up with an empty descriptor table.
  */
 struct coredump_ack {
 	__u32 size;

-- 
2.53.0

[PATCH RFC v2 16/16] selftests/coredump: test COREDUMP_CLOSE_FILES

[Christian Brauner]

Test COREDUMP_CLOSE_FILES.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 tools/testing/selftests/coredump/Makefile          |   4 +-
 .../selftests/coredump/coredump_close_files_test.c | 592 +++++++++++++++++++++
 .../coredump/coredump_socket_protocol_test.c       |   6 +
 .../selftests/coredump/coredump_test_helpers.c     |   3 +-
 4 files changed, 603 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/coredump/Makefile b/tools/testing/selftests/coredump/Makefile
index dece1a31d561..dc8489d40618 100644
--- a/tools/testing/selftests/coredump/Makefile
+++ b/tools/testing/selftests/coredump/Makefile
@@ -3,7 +3,8 @@ CFLAGS += -Wall -O0 -g $(KHDR_INCLUDES) $(TOOLS_INCLUDES)
 
 TEST_GEN_PROGS := stackdump_test \
 		  coredump_socket_test \
-		  coredump_socket_protocol_test
+		  coredump_socket_protocol_test \
+		  coredump_close_files_test
 TEST_FILES := stackdump
 
 include ../lib.mk
@@ -11,3 +12,4 @@ include ../lib.mk
 $(OUTPUT)/stackdump_test: coredump_test_helpers.c
 $(OUTPUT)/coredump_socket_test: coredump_test_helpers.c
 $(OUTPUT)/coredump_socket_protocol_test: coredump_test_helpers.c
+$(OUTPUT)/coredump_close_files_test: coredump_test_helpers.c
diff --git a/tools/testing/selftests/coredump/coredump_close_files_test.c b/tools/testing/selftests/coredump/coredump_close_files_test.c
new file mode 100644
index 000000000000..7a41906b4695
--- /dev/null
+++ b/tools/testing/selftests/coredump/coredump_close_files_test.c
@@ -0,0 +1,592 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <dirent.h>
+#include <fcntl.h>
+#include <pthread.h>
+#include <sys/file.h>
+#include <sys/stat.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+
+#include "coredump_test.h"
+
+#define LOCK_FILE "/tmp/coredump.lock"
+
+/* Idle threads the multi-threaded crashing child spawns. */
+#define NUM_CLOSE_THREADS 4
+
+/* How the crashing child locks LOCK_FILE. */
+enum lock_kind {
+	LOCK_KIND_POSIX,
+	LOCK_KIND_OFD,
+	LOCK_KIND_FLOCK,
+};
+
+/* Who else has a handle on the lock when the child crashes. */
+enum lock_share {
+	LOCK_SHARE_NONE,
+	LOCK_SHARE_DUP,		/* a second slot in the child's own table */
+	LOCK_SHARE_FORK,	/* a forked process keeps the fd open */
+	LOCK_SHARE_FILES,	/* a CLONE_FILES process shares the table */
+};
+
+struct close_test {
+	enum lock_kind kind;
+	enum lock_share share;
+	bool threads;
+	bool close;		/* ack with COREDUMP_CLOSE_FILES */
+	bool userspace;		/* COREDUMP_USERSPACE instead of COREDUMP_KERNEL */
+	bool released;		/* the lock is gone once the kernel is past the close */
+};
+
+FIXTURE_SETUP(coredump)
+{
+	FILE *file;
+	int ret;
+
+	self->pid_coredump_server = -ESRCH;
+	self->fd_tmpfs_detached = -1;
+	file = fopen("/proc/sys/kernel/core_pattern", "r");
+	ASSERT_NE(NULL, file);
+
+	ret = fread(self->original_core_pattern, 1, sizeof(self->original_core_pattern), file);
+	ASSERT_TRUE(ret || feof(file));
+	ASSERT_LT(ret, sizeof(self->original_core_pattern));
+
+	self->original_core_pattern[ret] = '\0';
+	self->fd_tmpfs_detached = create_detached_tmpfs();
+	ASSERT_GE(self->fd_tmpfs_detached, 0);
+
+	ret = fclose(file);
+	ASSERT_EQ(0, ret);
+}
+
+FIXTURE_TEARDOWN(coredump)
+{
+	const char *reason;
+	FILE *file;
+	int ret, status;
+
+	if (self->pid_coredump_server > 0) {
+		kill(self->pid_coredump_server, SIGTERM);
+		waitpid(self->pid_coredump_server, &status, 0);
+	}
+	unlink(LOCK_FILE);
+	unlink("/tmp/coredump.socket");
+
+	file = fopen("/proc/sys/kernel/core_pattern", "w");
+	if (!file) {
+		reason = "Unable to open core_pattern";
+		goto fail;
+	}
+
+	ret = fprintf(file, "%s", self->original_core_pattern);
+	if (ret < 0) {
+		reason = "Unable to write to core_pattern";
+		goto fail;
+	}
+
+	ret = fclose(file);
+	if (ret) {
+		reason = "Unable to close core_pattern";
+		goto fail;
+	}
+
+	if (self->fd_tmpfs_detached >= 0) {
+		ret = close(self->fd_tmpfs_detached);
+		if (ret < 0) {
+			reason = "Unable to close detached tmpfs";
+			goto fail;
+		}
+		self->fd_tmpfs_detached = -1;
+	}
+
+	return;
+fail:
+	/* This should never happen */
+	fprintf(stderr, "Failed to cleanup coredump test: %s\n", reason);
+}
+
+/* Write-lock @fd the way @kind says. */
+static int take_lock(int fd, enum lock_kind kind)
+{
+	struct flock fl = {
+		.l_type = F_WRLCK,
+		.l_whence = SEEK_SET,
+	};
+
+	switch (kind) {
+	case LOCK_KIND_POSIX:
+		return fcntl(fd, F_SETLK, &fl);
+	case LOCK_KIND_OFD:
+		return fcntl(fd, F_OFD_SETLK, &fl);
+	case LOCK_KIND_FLOCK:
+		return flock(fd, LOCK_EX);
+	}
+
+	return -1;
+}
+
+/* Does anyone else hold a write lock on @fd? 1 if so, 0 if not, -1 on error. */
+static int lock_held(int fd, enum lock_kind kind)
+{
+	struct flock fl = {
+		.l_type = F_WRLCK,
+		.l_whence = SEEK_SET,
+	};
+
+	if (kind == LOCK_KIND_FLOCK) {
+		if (flock(fd, LOCK_EX | LOCK_NB) == 0) {
+			flock(fd, LOCK_UN);
+			return 0;
+		}
+		return errno == EWOULDBLOCK ? 1 : -1;
+	}
+
+	/* F_GETLK reports conflicting OFD locks too. */
+	if (fcntl(fd, F_GETLK, &fl) < 0)
+		return -1;
+	return fl.l_type != F_UNLCK;
+}
+
+/* Number of entries in /proc/@pid/fd, the lowest one in @first. */
+static int count_fds(pid_t pid, int *first)
+{
+	char path[64];
+	struct dirent *de;
+	DIR *dir;
+	int nr = 0;
+
+	snprintf(path, sizeof(path), "/proc/%d/fd", pid);
+	dir = opendir(path);
+	if (!dir)
+		return -1;
+
+	*first = -1;
+	while ((de = readdir(dir))) {
+		int fd;
+
+		if (de->d_name[0] == '.')
+			continue;
+		fd = atoi(de->d_name);
+		if (*first < 0 || fd < *first)
+			*first = fd;
+		nr++;
+	}
+	closedir(dir);
+	return nr;
+}
+
+/*
+ * Block until the test hangs up @fd_release, keeping every inherited fd
+ * open, then report through @fd_result whether @fd is still open.
+ */
+static void hold_until_released(int fd, int fd_release, int fd_result)
+{
+	char c;
+
+	read_nointr(fd_release, &c, 1);
+	c = fcntl(fd, F_GETFD) < 0 ? 'C' : 'O';
+	write_nointr(fd_result, &c, 1);
+	_exit(EXIT_SUCCESS);
+}
+
+/* Lock LOCK_FILE, share it as requested, then crash. */
+static void crashing_child_locked(const struct close_test *t, int fd_release,
+				  int fd_result)
+{
+	pthread_t thread;
+	int fd, pidfd, i;
+	pid_t pid;
+
+	fd = open(LOCK_FILE, O_RDWR | O_CLOEXEC);
+	if (fd < 0)
+		_exit(EXIT_FAILURE);
+
+	if (take_lock(fd, t->kind))
+		_exit(EXIT_FAILURE);
+
+	switch (t->share) {
+	case LOCK_SHARE_NONE:
+		break;
+	case LOCK_SHARE_DUP:
+		if (dup(fd) < 0)
+			_exit(EXIT_FAILURE);
+		break;
+	case LOCK_SHARE_FORK:
+		pid = fork();
+		if (pid < 0)
+			_exit(EXIT_FAILURE);
+		if (pid == 0)
+			hold_until_released(fd, fd_release, fd_result);
+		break;
+	case LOCK_SHARE_FILES:
+		pid = create_child(&pidfd, CLONE_FILES);
+		if (pid < 0)
+			_exit(EXIT_FAILURE);
+		if (pid == 0)
+			hold_until_released(fd, fd_release, fd_result);
+		break;
+	}
+
+	if (t->threads)
+		for (i = 0; i < NUM_CLOSE_THREADS; i++)
+			pthread_create(&thread, NULL, do_nothing, NULL);
+
+	/* crash on purpose */
+	*(volatile int *)NULL = 0;
+}
+
+/*
+ * Serve one coredump and look at the task on the way. Before the ack the
+ * lock is held and the descriptors are there. Once the kernel is past the
+ * point where it closes them, which is before the first byte of the dump
+ * or before the hangup in userspace mode, they are gone if we asked for
+ * it and the lock is in the expected state.
+ */
+static int close_server(const struct close_test *t, int fd_ipc)
+{
+	struct coredump_req req = {};
+	struct pidfd_info info = {};
+	int fd_server = -1, fd_coredump = -1, fd_peer_pidfd = -1, fd_lock = -1;
+	int exit_code = EXIT_FAILURE;
+	int fd, first_fd, nr_fds;
+	__u64 mask;
+	ssize_t bytes;
+	char c;
+
+	fd_lock = open(LOCK_FILE, O_RDWR | O_CLOEXEC);
+	if (fd_lock < 0) {
+		fprintf(stderr, "%s: open lock file failed: %m\n", __func__);
+		goto out;
+	}
+
+	fd_server = create_and_listen_unix_socket("/tmp/coredump.socket");
+	if (fd_server < 0) {
+		fprintf(stderr, "%s: create_and_listen_unix_socket failed: %m\n", __func__);
+		goto out;
+	}
+
+	if (write_nointr(fd_ipc, "1", 1) < 0) {
+		fprintf(stderr, "%s: write_nointr to ipc socket failed: %m\n", __func__);
+		goto out;
+	}
+	close(fd_ipc);
+
+	fd_coredump = accept4(fd_server, NULL, NULL, SOCK_CLOEXEC);
+	if (fd_coredump < 0) {
+		fprintf(stderr, "%s: accept4 failed: %m\n", __func__);
+		goto out;
+	}
+
+	fd_peer_pidfd = get_peer_pidfd(fd_coredump);
+	if (fd_peer_pidfd < 0) {
+		fprintf(stderr, "%s: get_peer_pidfd failed\n", __func__);
+		goto out;
+	}
+
+	if (!get_pidfd_info(fd_peer_pidfd, &info)) {
+		fprintf(stderr, "%s: get_pidfd_info failed\n", __func__);
+		goto out;
+	}
+
+	if (!read_coredump_req(fd_coredump, &req)) {
+		fprintf(stderr, "%s: read_coredump_req failed\n", __func__);
+		goto out;
+	}
+
+	if (!check_coredump_req(&req)) {
+		fprintf(stderr, "%s: check_coredump_req failed\n", __func__);
+		goto out;
+	}
+
+	/* The task waits for our answer with everything still in place. */
+	if (lock_held(fd_lock, t->kind) != 1) {
+		fprintf(stderr, "%s: lock not held during the handshake\n", __func__);
+		goto out;
+	}
+
+	nr_fds = count_fds(info.pid, &first_fd);
+	if (nr_fds <= 0) {
+		fprintf(stderr, "%s: no descriptors during the handshake\n", __func__);
+		goto out;
+	}
+
+	fd = sys_pidfd_getfd(fd_peer_pidfd, first_fd, 0);
+	if (fd < 0) {
+		fprintf(stderr, "%s: pidfd_getfd during the handshake failed: %m\n", __func__);
+		goto out;
+	}
+	close(fd);
+
+	mask = COREDUMP_WAIT;
+	mask |= t->userspace ? COREDUMP_USERSPACE : COREDUMP_KERNEL;
+	if (t->close)
+		mask |= COREDUMP_CLOSE_FILES;
+
+	if (!send_coredump_ack(fd_coredump, &req, mask, 0)) {
+		fprintf(stderr, "%s: send_coredump_ack failed\n", __func__);
+		goto out;
+	}
+
+	if (!read_marker(fd_coredump, COREDUMP_MARK_REQACK)) {
+		fprintf(stderr, "%s: read_marker COREDUMP_MARK_REQACK failed\n", __func__);
+		goto out;
+	}
+
+	bytes = read_nointr(fd_coredump, &c, 1);
+	if (bytes != (t->userspace ? 0 : 1)) {
+		fprintf(stderr, "%s: read after the ack returned %zd: %m\n", __func__, bytes);
+		goto out;
+	}
+
+	if (lock_held(fd_lock, t->kind) != !t->released) {
+		fprintf(stderr, "%s: lock %s while the coredump is generated\n",
+			__func__, t->released ? "still held" : "released");
+		goto out;
+	}
+
+	nr_fds = count_fds(info.pid, &first_fd);
+	if (nr_fds < 0 || !nr_fds != t->close) {
+		fprintf(stderr, "%s: %d descriptors while the coredump is generated\n",
+			__func__, nr_fds);
+		goto out;
+	}
+
+	fd = sys_pidfd_getfd(fd_peer_pidfd, first_fd, 0);
+	if (t->close) {
+		if (fd >= 0 || errno != EBADF) {
+			fprintf(stderr, "%s: pidfd_getfd after the close returned %d: %m\n",
+				__func__, fd);
+			goto out;
+		}
+	} else {
+		if (fd < 0) {
+			fprintf(stderr, "%s: pidfd_getfd during the coredump failed: %m\n",
+				__func__);
+			goto out;
+		}
+		close(fd);
+	}
+
+	/* COREDUMP_WAIT keeps the task around until we hang up. */
+	if (!get_pidfd_info(fd_peer_pidfd, &info)) {
+		fprintf(stderr, "%s: get_pidfd_info failed\n", __func__);
+		goto out;
+	}
+
+	if (info.mask & PIDFD_INFO_EXIT) {
+		fprintf(stderr, "%s: task exited before the coredump finished\n", __func__);
+		goto out;
+	}
+
+	for (;;) {
+		char buffer[4096];
+
+		bytes = read_nointr(fd_coredump, buffer, sizeof(buffer));
+		if (bytes < 0) {
+			fprintf(stderr, "%s: read from coredump socket failed: %m\n", __func__);
+			goto out;
+		}
+
+		if (bytes == 0)
+			break;
+	}
+
+	exit_code = EXIT_SUCCESS;
+out:
+	if (fd_lock >= 0)
+		close(fd_lock);
+	if (fd_peer_pidfd >= 0)
+		close(fd_peer_pidfd);
+	if (fd_coredump >= 0)
+		close(fd_coredump);
+	if (fd_server >= 0)
+		close(fd_server);
+	return exit_code;
+}
+
+static void run_close_test(struct __test_metadata *const _metadata,
+			   FIXTURE_DATA(coredump) *self,
+			   const struct close_test *t)
+{
+	int fd, status, ipc_sockets[2], release_pipe[2], result_pipe[2];
+	pid_t pid, pid_coredump_server;
+	char c;
+
+	ASSERT_TRUE(set_core_pattern("@@/tmp/coredump.socket"));
+
+	fd = open(LOCK_FILE, O_RDWR | O_CREAT | O_TRUNC | O_CLOEXEC, 0644);
+	ASSERT_GE(fd, 0);
+	EXPECT_EQ(close(fd), 0);
+
+	ASSERT_EQ(socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets), 0);
+
+	pid_coredump_server = fork();
+	ASSERT_GE(pid_coredump_server, 0);
+	if (pid_coredump_server == 0) {
+		close(ipc_sockets[0]);
+		_exit(close_server(t, ipc_sockets[1]));
+	}
+	self->pid_coredump_server = pid_coredump_server;
+
+	EXPECT_EQ(close(ipc_sockets[1]), 0);
+	ASSERT_EQ(read_nointr(ipc_sockets[0], &c, 1), 1);
+	EXPECT_EQ(close(ipc_sockets[0]), 0);
+
+	/* Only the crashing child and what it spawns see these pipes. */
+	ASSERT_EQ(pipe2(release_pipe, O_CLOEXEC), 0);
+	ASSERT_EQ(pipe2(result_pipe, O_CLOEXEC), 0);
+
+	pid = fork();
+	ASSERT_GE(pid, 0);
+	if (pid == 0) {
+		close(release_pipe[1]);
+		close(result_pipe[0]);
+		crashing_child_locked(t, release_pipe[0], result_pipe[1]);
+	}
+	EXPECT_EQ(close(release_pipe[0]), 0);
+	EXPECT_EQ(close(result_pipe[1]), 0);
+
+	waitpid(pid, &status, 0);
+	ASSERT_TRUE(WIFSIGNALED(status));
+	ASSERT_TRUE(WCOREDUMP(status));
+
+	wait_and_check_coredump_server(pid_coredump_server, _metadata, self);
+
+	/* Let the process holding the shared lock go. */
+	EXPECT_EQ(close(release_pipe[1]), 0);
+
+	/* The crashing child is gone but what it shared with is untouched. */
+	if (t->share == LOCK_SHARE_FORK || t->share == LOCK_SHARE_FILES) {
+		ASSERT_EQ(read_nointr(result_pipe[0], &c, 1), 1);
+		ASSERT_EQ(c, 'O');
+	}
+	EXPECT_EQ(close(result_pipe[0]), 0);
+}
+
+TEST_F(coredump, close_files_posix)
+{
+	const struct close_test t = {
+		.kind = LOCK_KIND_POSIX,
+		.share = LOCK_SHARE_NONE,
+		.threads = true,
+		.close = true,
+		.released = true,
+	};
+
+	run_close_test(_metadata, self, &t);
+}
+
+TEST_F(coredump, close_files_ofd)
+{
+	const struct close_test t = {
+		.kind = LOCK_KIND_OFD,
+		.share = LOCK_SHARE_NONE,
+		.close = true,
+		.released = true,
+	};
+
+	run_close_test(_metadata, self, &t);
+}
+
+TEST_F(coredump, close_files_flock)
+{
+	const struct close_test t = {
+		.kind = LOCK_KIND_FLOCK,
+		.share = LOCK_SHARE_NONE,
+		.threads = true,
+		.close = true,
+		.released = true,
+	};
+
+	run_close_test(_metadata, self, &t);
+}
+
+TEST_F(coredump, close_files_flock_dup)
+{
+	const struct close_test t = {
+		.kind = LOCK_KIND_FLOCK,
+		.share = LOCK_SHARE_DUP,
+		.close = true,
+		.released = true,
+	};
+
+	run_close_test(_metadata, self, &t);
+}
+
+TEST_F(coredump, close_files_posix_fork)
+{
+	const struct close_test t = {
+		.kind = LOCK_KIND_POSIX,
+		.share = LOCK_SHARE_FORK,
+		.close = true,
+		.released = true,
+	};
+
+	run_close_test(_metadata, self, &t);
+}
+
+TEST_F(coredump, close_files_flock_fork)
+{
+	const struct close_test t = {
+		.kind = LOCK_KIND_FLOCK,
+		.share = LOCK_SHARE_FORK,
+		.close = true,
+		.released = false,
+	};
+
+	run_close_test(_metadata, self, &t);
+}
+
+TEST_F(coredump, close_files_ofd_fork)
+{
+	const struct close_test t = {
+		.kind = LOCK_KIND_OFD,
+		.share = LOCK_SHARE_FORK,
+		.close = true,
+		.released = false,
+	};
+
+	run_close_test(_metadata, self, &t);
+}
+
+TEST_F(coredump, close_files_posix_shared_table)
+{
+	const struct close_test t = {
+		.kind = LOCK_KIND_POSIX,
+		.share = LOCK_SHARE_FILES,
+		.close = true,
+		.released = false,
+	};
+
+	run_close_test(_metadata, self, &t);
+}
+
+TEST_F(coredump, close_files_userspace)
+{
+	const struct close_test t = {
+		.kind = LOCK_KIND_FLOCK,
+		.share = LOCK_SHARE_NONE,
+		.close = true,
+		.userspace = true,
+		.released = true,
+	};
+
+	run_close_test(_metadata, self, &t);
+}
+
+TEST_F(coredump, close_files_not_requested)
+{
+	const struct close_test t = {
+		.kind = LOCK_KIND_FLOCK,
+		.share = LOCK_SHARE_NONE,
+		.threads = true,
+		.close = false,
+		.released = false,
+	};
+
+	run_close_test(_metadata, self, &t);
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/coredump/coredump_socket_protocol_test.c b/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
index f5c9bad87546..beb167698cfd 100644
--- a/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
+++ b/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
@@ -2377,6 +2377,12 @@ TEST_F(coredump, socket_request_memory_types_without_kernel)
 	check_conflicting_ack(_metadata, self, COREDUMP_USERSPACE | COREDUMP_MEMORY_TYPES);
 }
 
+/* A rejected task closes everything on its way out anyway. */
+TEST_F(coredump, socket_request_close_files_reject)
+{
+	check_conflicting_ack(_metadata, self, COREDUMP_REJECT | COREDUMP_CLOSE_FILES);
+}
+
 /*
  * A server built with the first structs reads the request it knows,
  * discards the rest and acks with the ack it knows. It raises nothing
diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
index 4e36e3e4fb78..65132a0deced 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.c
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
@@ -1612,7 +1612,8 @@ bool send_coredump_ack(int fd, const struct coredump_req *req,
 #define TEST_REQ_MASK_ALL					\
 	(COREDUMP_KERNEL | COREDUMP_USERSPACE |			\
 	 COREDUMP_REJECT | COREDUMP_WAIT |			\
-	 COREDUMP_RECORDS | COREDUMP_SPARSE | COREDUMP_MEMORY_TYPES)
+	 COREDUMP_RECORDS | COREDUMP_SPARSE | COREDUMP_MEMORY_TYPES | \
+	 COREDUMP_CLOSE_FILES)
 
 bool check_coredump_req(const struct coredump_req *req)
 {

-- 
2.53.0