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

Add switch_files_struct() to install another table on a task. It
consumes the reference to the new table and puts the old one. Convert
every place that switches a descriptor table except unshare_files().

No functional changes.

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

diff --git a/fs/file.c b/fs/file.c
index 59673547de90..345011dad472 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -515,16 +515,18 @@ void put_files_struct(struct files_struct *files)
 	}
 }
 
-void exit_files(struct task_struct *tsk)
+/* Install @files on @tsk, consuming the reference, and put the old table. */
+void switch_files_struct(struct task_struct *tsk, struct files_struct *files)
 {
-	struct files_struct * files = tsk->files;
+	scoped_guard(task_lock, tsk)
+		swap(tsk->files, files);
+	put_files_struct(files);
+}
 
-	if (files) {
-		task_lock(tsk);
-		tsk->files = NULL;
-		task_unlock(tsk);
-		put_files_struct(files);
-	}
+void exit_files(struct task_struct *tsk)
+{
+	if (tsk->files)
+		switch_files_struct(tsk, NULL);
 }
 
 struct files_struct init_files = {
@@ -855,10 +857,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);
+		switch_files_struct(me, cur_fds);
 	}
 
 	return 0;
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index c45306a9f007..9614c6ecd477 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);
+void 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..7c6fe82dbb9f 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)
+			switch_files_struct(current, no_free_ptr(new_fd));
 
 		if (new_cred) {
 			/* Install the new user namespace */

-- 
2.53.0

Move unshare_fd() 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 345011dad472..636a87e527b3 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 9614c6ecd477..4ee1598848bb 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -102,6 +102,7 @@ struct task_struct;
 void put_files_struct(struct files_struct *fs);
 void 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 7c6fe82dbb9f..fddba55bbc14 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

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 d3081c8f7c10..977778f44cfc 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)
+		switch_files_struct(me, files);
 
 	/*
 	 * We have to apply CLOEXEC before we change whether the process is
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index 4ee1598848bb..666808a1caf5 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -101,7 +101,6 @@ struct task_struct;
 
 void put_files_struct(struct files_struct *fs);
 void 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 fddba55bbc14..300cb46bc51e 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

Currently close() already does a synchronous release of the last
reference since the task is about to return to userspace and the
deferral through task work buys nothing. Add a filp_close_sync() helper.
We'll use that in the next patches.

No functional changes.

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

diff --git a/fs/internal.h b/fs/internal.h
index c658c8a5ebd5..8812de210d3f 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -198,6 +198,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_close_sync(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..998e42ac319a 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1537,6 +1537,19 @@ int filp_close(struct file *filp, fl_owner_t id)
 }
 EXPORT_SYMBOL(filp_close);
 
+/* Like filp_close() but the last reference is put right here. */
+int filp_close_sync(struct file *filp, fl_owner_t id)
+{
+	int retval;
+
+	/* Kernel threads must never put their final reference here. */
+	VFS_WARN_ON_ONCE(current->flags & PF_KTHREAD);
+	retval = filp_flush(filp, id);
+	fput_close_sync(filp);
+
+	return retval;
+}
+
 /*
  * Careful here! We test whether the file pointer is NULL before
  * releasing the fd. This ensures that one clone task can't release
@@ -1551,13 +1564,11 @@ SYSCALL_DEFINE1(close, unsigned int, fd)
 	if (!file)
 		return -EBADF;
 
-	retval = filp_flush(file, current->files);
-
 	/*
 	 * We're returning to user space. Don't bother
 	 * with any delayed fput() cases.
 	 */
-	fput_close_sync(file);
+	retval = filp_close_sync(file, current->files);
 
 	if (likely(retval == 0))
 		return 0;

-- 
2.53.0

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 leaves a list for any later task_work_cancel() to search
under ->pi_lock and costs a lot of atomics too.

Let close_files() close right away. Flush and put each file inline the
way close(2) does. The final __fput() runs during the table walk now
instead of from task_work_run() in exit_task_work(). One difference is
the order: task work ran the final __fput()s in reverse and now they run
in table order.

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
were 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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index 636a87e527b3..b2b466dce7fb 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
@@ -509,7 +509,7 @@ 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_close_sync(file, files);
 					cond_resched();
 				}
 			}

-- 
2.53.0

__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().

Flush and put each file inline while ->file_lock is dropped.
close_range(2) now behaves like close(2).

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

diff --git a/fs/file.c b/fs/file.c
index b2b466dce7fb..178c8cb9da09 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -807,7 +807,7 @@ 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_close_sync(file, files);
 			cond_resched();
 			spin_lock(&files->file_lock);
 			fdt = files_fdtable(files);

-- 
2.53.0

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 977778f44cfc..1d9163155d16 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1179,7 +1179,7 @@ int begin_new_exec(struct linux_binprm * bprm)
 	 * This must happen after the point of no return, and after unsharing
 	 * the FD table.
 	 */
-	do_close_on_exec(me->files);
+	close_cloexec_files(me->files);
 
 	/*
 	 * Must be called _before_ exec_mmap() as bprm->mm is
diff --git a/fs/file.c b/fs/file.c
index 178c8cb9da09..b0490566719c 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -901,7 +901,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 666808a1caf5..2965acd120bc 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -106,7 +106,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

Punting file closing to task work during exec slows down exec
significantly when its done with a bunch of file descriptors. We can
do this in-band instead. Flush already runs synchronous. Jann moved
close-on-exec in e780259b54e6 ("exec: do_close_on_exec() before taking
exec_update_lock") outside of exec_update_lock.

The only lock that's still held now is cred_guard_mutex. It's deprecated
and has five takers

(1) exec
(2) ptrace_attach()
(3) seccomp() with SECCOMP_FILTER_FLAG_TSYNC
(4) writes to /proc/<pid>/attr/*
(5) lsm_set_self_attr()

Four of them take the task's own cred_guard_mutex. When
close_cloexec_files() runs, de_thread() ensured that the calling task is
the only one alive in its thread-group. That leaves ptrace() waiting on
cred_guard_mutex of the tracee going through exec. exec already sleeps
under cred_guard_mutex in de_thread() when it reads binary and
interpreter. So while we add wait-time to an attaching ptracer no new
lock dependency is added. vfork() als waits but that's a dup_fd() copy
of the fdtable and rarely holds the last reference. If that's an issue
we can always change that later.

Link: https://lore.kernel.org/CAGudoHEsGP1P+sAWaw_tbh1NesJhSeww8869uzmaqtgk8F43=Q@mail.gmail.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/exec.c | 6 +++---
 fs/file.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 1d9163155d16..075a744421e1 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1173,9 +1173,9 @@ int begin_new_exec(struct linux_binprm * bprm)
 	 * trying to access the should-be-closed file descriptors of a process
 	 * undergoing exec(2).
 	 *
-	 * This can block on filesystem ->flush() handlers, including waiting
-	 * for FUSE daemons, so do it before exec_mmap takes the
-	 * exec_update_lock.
+	 * This can block on filesystem ->flush() and ->release() handlers,
+	 * including waiting for FUSE daemons, so do it before exec_mmap
+	 * takes the exec_update_lock.
 	 * This must happen after the point of no return, and after unsharing
 	 * the FD table.
 	 */
diff --git a/fs/file.c b/fs/file.c
index b0490566719c..76e328edf630 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -928,7 +928,7 @@ 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_close_sync(file, files);
 			cond_resched();
 			spin_lock(&files->file_lock);
 		}

-- 
2.53.0

The core_state->dumper field isn't used anymore. Only its ->next pointer
is. The current task is always the dumping thread and the ->task pointer
is never read. Replace it with a plain pointer to the list of parked
threads.

Historically, core_state->dumper was used. Its ->task pointer was read.
by fill_note_info() started at &core_state->dumper to ensure that the
dumping thread came first in the ELF thread notes. That changed in
commit 4b0e21d64253 ("[elf][regset] simplify thread list handling in
fill_note_info()"). The first iteration was taken out of the loop. So
it's been unused ever since.

No functional changes.

Suggested-by: NeilBrown <neilb@ownmail.net>
Link: https://lore.kernel.org/178900159210.207413.8292125177519817528@noble.neil.brown.name
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/binfmt_elf.c              | 2 +-
 fs/binfmt_elf_fdpic.c        | 2 +-
 fs/coredump.c                | 7 +++----
 include/linux/sched/signal.h | 2 +-
 kernel/exit.c                | 4 ++--
 5 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 6b7ffac5d665..bf7f8f47548d 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1875,7 +1875,7 @@ static int fill_note_info(struct elfhdr *elf, int phdrs,
 		return 0;
 
 	info->thread->task = dump_task;
-	for (ct = dump_task->signal->core_state->dumper.next; ct; ct = ct->next) {
+	for (ct = dump_task->signal->core_state->tasks; ct; ct = ct->next) {
 		t = kzalloc_flex(*t, notes, info->thread_notes);
 		if (unlikely(!t))
 			return 0;
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 005f0a084483..d3872169f55e 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1504,7 +1504,7 @@ static bool elf_fdpic_core_dump(struct coredump_params *cprm)
 	if (!psinfo)
 		goto end_coredump;
 
-	for (ct = current->signal->core_state->dumper.next;
+	for (ct = current->signal->core_state->tasks;
 					ct; ct = ct->next) {
 		tmp = elf_dump_thread_status(cprm->siginfo->si_signo,
 					     ct->task, &thread_status_size);
diff --git a/fs/coredump.c b/fs/coredump.c
index 9addd2d59b7b..16d3fb54500b 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -524,8 +524,7 @@ static int coredump_wait(int exit_code, struct core_state *core_state)
 	int core_waiters = -EBUSY;
 
 	init_completion(&core_state->startup);
-	core_state->dumper.task = tsk;
-	core_state->dumper.next = NULL;
+	core_state->tasks = NULL;
 
 	core_waiters = zap_threads(tsk, core_state, exit_code);
 	if (core_waiters > 0) {
@@ -538,7 +537,7 @@ static int coredump_wait(int exit_code, struct core_state *core_state)
 		 * all the thread context (extended register state, like
 		 * fpu etc) gets copied to the memory.
 		 */
-		ptr = core_state->dumper.next;
+		ptr = core_state->tasks;
 		while (ptr != NULL) {
 			wait_task_inactive(ptr->task, TASK_ANY);
 			ptr = ptr->next;
@@ -556,7 +555,7 @@ static void coredump_finish(enum coredump_state state)
 	spin_lock_irq(&current->sighand->siglock);
 	if ((state & COREDUMP_STATE_STARTED) && !__fatal_signal_pending(current))
 		current->signal->group_exit_code |= 0x80;
-	next = current->signal->core_state->dumper.next;
+	next = current->signal->core_state->tasks;
 	current->signal->core_state = NULL;
 	spin_unlock_irq(&current->sighand->siglock);
 
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index d45a5476b97d..14b55d00d605 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -80,7 +80,7 @@ struct core_thread {
 
 struct core_state {
 	atomic_t nr_threads;
-	struct core_thread dumper;
+	struct core_thread *tasks;
 	struct completion startup;
 };
 
diff --git a/kernel/exit.c b/kernel/exit.c
index 4e028f157597..3df1fffc6674 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -435,12 +435,12 @@ static void coredump_task_exit(struct task_struct *tsk,
 
 	self.task = tsk;
 	if (self.task->flags & PF_SIGNALED)
-		self.next = xchg(&core_state->dumper.next, &self);
+		self.next = xchg(&core_state->tasks, &self);
 	else
 		self.task = NULL;
 	/*
 	 * Implies mb(), the result of xchg() must be visible
-	 * to core_state->dumper.
+	 * to the dumper.
 	 */
 	if (atomic_dec_and_test(&core_state->nr_threads))
 		complete(&core_state->startup);

-- 
2.53.0

All wait_var_event() sleep in a fixed task state. For coredumps we need
a variant that takes the state from the caller the way
wait_event_state() does. This allows us to continue sleeping with
TASK_FREEZABLE. That's certainly also a useful addition for other places.

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

diff --git a/include/linux/wait_bit.h b/include/linux/wait_bit.h
index 553d7b23e3ad..af077ed4caf6 100644
--- a/include/linux/wait_bit.h
+++ b/include/linux/wait_bit.h
@@ -432,6 +432,32 @@ do {									\
 	__ret;								\
 })
 
+/**
+ * wait_var_event_state - wait for a variable to be updated and notified
+ * @var: the address of variable being waited on
+ * @condition: the condition to wait for
+ * @state: the task state to sleep in, %TASK_UNINTERRUPTIBLE etc.
+ *
+ * Wait for a @condition to be true, only re-checking when a wake up is
+ * received for the given @var (an arbitrary kernel address which need
+ * not be directly related to the given condition, but usually is).
+ *
+ * Returns 0 if the condition became true, or %-ERESTARTSYS if a signal
+ * arrived which @state allows to interrupt.
+ *
+ * The condition should normally use smp_load_acquire() or a similarly
+ * ordered access to ensure that any changes to memory made before the
+ * condition became true will be visible after the wait completes.
+ */
+#define wait_var_event_state(var, condition, state)					\
+({											\
+	int __ret = 0;									\
+	might_sleep();									\
+	if (!(condition))								\
+		__ret = ___wait_var_event(var, condition, (state), 0, 0, schedule());	\
+	__ret;										\
+})
+
 /**
  * wait_var_event_any_lock - wait for a variable to be updated under a lock
  * @var: the address of the variable being waited on

-- 
2.53.0

coredump_wait() sets core_state->nr_threads to the number of tasks
killed and waits for the last thread to enter coredump_task_exit() to
signal completion. Let's just wait on the count directly. The exiting
tasks can use atomic_dec_and_wake_up() and the dumping task sleeps in
wait_var_event_state().

The dumping task must remain freezable since commit f5d39b020809
("freezer,sched: Rewrite core freezer logic"). So keep the wait
TASK_UNINTERRUPTIBLE|TASK_FREEZABLE.

Drop the completion and rename nr_threads to threads_remaining.

No functional changes.

Suggested-by: NeilBrown <neilb@ownmail.net>
Link: https://lore.kernel.org/178899497961.207413.10554121774377911612@noble.neil.brown.name
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/coredump.c                | 9 +++++----
 include/linux/sched/signal.h | 4 ++--
 kernel/exit.c                | 6 +++---
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index 16d3fb54500b..2e2005c14d93 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -39,6 +39,7 @@
 #include <linux/oom.h>
 #include <linux/compat.h>
 #include <linux/fs.h>
+#include <linux/wait_bit.h>
 #include <linux/path.h>
 #include <linux/timekeeping.h>
 #include <linux/sysctl.h>
@@ -512,7 +513,7 @@ static int zap_threads(struct task_struct *tsk,
 		nr = zap_process(signal, exit_code);
 		clear_tsk_thread_flag(tsk, TIF_SIGPENDING);
 		tsk->flags |= PF_DUMPCORE;
-		atomic_set(&core_state->nr_threads, nr);
+		atomic_set(&core_state->threads_remaining, nr);
 	}
 	spin_unlock_irq(&tsk->sighand->siglock);
 	return nr;
@@ -523,15 +524,15 @@ 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);
 	core_state->tasks = NULL;
 
 	core_waiters = zap_threads(tsk, core_state, exit_code);
 	if (core_waiters > 0) {
 		struct core_thread *ptr;
 
-		wait_for_completion_state(&core_state->startup,
-					  TASK_UNINTERRUPTIBLE|TASK_FREEZABLE);
+		wait_var_event_state(&core_state->threads_remaining,
+				     !atomic_read_acquire(&core_state->threads_remaining),
+				     TASK_UNINTERRUPTIBLE|TASK_FREEZABLE);
 		/*
 		 * Wait for all the threads to become inactive, so that
 		 * all the thread context (extended register state, like
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 14b55d00d605..e039e29cd8c5 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -79,9 +79,9 @@ struct core_thread {
 };
 
 struct core_state {
-	atomic_t nr_threads;
+	/* Threads the dumper still waits for. */
+	atomic_t threads_remaining;
 	struct core_thread *tasks;
-	struct completion startup;
 };
 
 /*
diff --git a/kernel/exit.c b/kernel/exit.c
index 3df1fffc6674..55dbea3b242e 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/capability.h>
 #include <linux/completion.h>
+#include <linux/wait_bit.h>
 #include <linux/personality.h>
 #include <linux/tty.h>
 #include <linux/iocontext.h>
@@ -442,8 +443,7 @@ static void coredump_task_exit(struct task_struct *tsk,
 	 * Implies mb(), the result of xchg() must be visible
 	 * to the dumper.
 	 */
-	if (atomic_dec_and_test(&core_state->nr_threads))
-		complete(&core_state->startup);
+	atomic_dec_and_wake_up(&core_state->threads_remaining);
 
 	for (;;) {
 		set_current_state(TASK_IDLE|TASK_FREEZABLE);
@@ -917,7 +917,7 @@ static void synchronize_group_exit(struct task_struct *tsk, long code)
 	 * Serialize with any possible pending coredump.
 	 * We must hold siglock around checking core_state
 	 * and setting PF_POSTCOREDUMP.  The core-inducing thread
-	 * will increment ->nr_threads for each thread in the
+	 * will increment ->threads_remaining for each thread in the
 	 * group without PF_POSTCOREDUMP set.
 	 */
 	tsk->flags |= PF_POSTCOREDUMP;

-- 
2.53.0

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 | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index 2e2005c14d93..daeca723bdae 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -519,6 +519,22 @@ 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_var_event_state(&core_state->threads_remaining,
+			     !atomic_read_acquire(&core_state->threads_remaining),
+			     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.
+	 */
+	for (ptr = core_state->tasks; ptr; ptr = ptr->next)
+		wait_task_inactive(ptr->task, TASK_ANY);
+}
+
 static int coredump_wait(int exit_code, struct core_state *core_state)
 {
 	struct task_struct *tsk = current;
@@ -527,23 +543,8 @@ static int coredump_wait(int exit_code, struct core_state *core_state)
 	core_state->tasks = NULL;
 
 	core_waiters = zap_threads(tsk, core_state, exit_code);
-	if (core_waiters > 0) {
-		struct core_thread *ptr;
-
-		wait_var_event_state(&core_state->threads_remaining,
-				     !atomic_read_acquire(&core_state->threads_remaining),
-				     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->tasks;
-		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

Add a way to get an empty descriptor table with one reference. Let
dup_fd() share the allocation. 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 76e328edf630..8fa58500b6d1 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 2965acd120bc..155ad783f2ce 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);
 void switch_files_struct(struct task_struct *tsk, struct files_struct *files);
 int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp);
 struct fd_range {

-- 
2.53.0

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.

We switch to an empty fdtable instead of simply clearing because
io_uring_files_cancel() runs task work in do_exit() before
exit_signals() sets PF_EXITING and may reissue requests that dereference
current->files.

The files are closed like close(2) would, PF_EXITING isn't set yet so
SO_LINGER sockets linger.

Reported-by: Xin Zhao <jackzxcui1989@163.com>
Link: https://lore.kernel.org/20260618030700.2511668-1-jackzxcui1989@163.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/coredump.c                 | 47 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/sched/signal.h  |  2 ++
 include/uapi/linux/coredump.h |  8 ++++++++
 kernel/exit.c                 | 11 ++++++++--
 4 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index daeca723bdae..4ffd801aca01 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -549,6 +549,40 @@ 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;
+
+	files = alloc_files_struct();
+	if (!files)
+		return false;
+
+	for (ct = core_state->tasks; ct; ct = ct->next) {
+		/* Tasks without a table such as vhost workers can be skipped. */
+		if (!ct->task->files)
+			continue;
+		atomic_inc(&core_state->threads_remaining);
+		/* 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 overridden ones. */
+	scoped_with_creds(current_real_cred())
+		switch_files_struct(current, files);
+
+	coredump_wait_inactive(core_state);
+	return true;
+}
+
 static void coredump_finish(enum coredump_state state)
 {
 	struct core_thread *curr, *next;
@@ -838,7 +872,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,
@@ -906,6 +941,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) {
@@ -1221,6 +1262,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 e039e29cd8c5..70067ccfe2ba 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -76,6 +76,8 @@ 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 {
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 55dbea3b242e..04f2c8c78879 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -26,6 +26,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>
@@ -432,9 +433,8 @@ 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 core_thread self;
+	struct core_thread self = { .task = tsk };
 
-	self.task = tsk;
 	if (self.task->flags & PF_SIGNALED)
 		self.next = xchg(&core_state->tasks, &self);
 	else
@@ -449,6 +449,13 @@ 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(). */
+		if (smp_load_acquire(&self.files)) {
+			__set_current_state(TASK_RUNNING);
+			switch_files_struct(tsk, no_free_ptr(self.files));
+			atomic_dec_and_wake_up(&core_state->threads_remaining);
+			continue;
+		}
 		schedule();
 	}
 	__set_current_state(TASK_RUNNING);

-- 
2.53.0

COREDUMP_CLOSE_FILES allows userspace to request to drop the descriptor
tables of the thread-group before the coredump is written. io_uring
requests the threads had in flight survive and say a poll-triggered
accept can install a descriptor into whatever table the thread has at
that point.

Since the whole thread-group is going down in a coredump do what exec
does after de_thread() and cancel everything. After that no io_uring
task work can be queued to the thread anymore.

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

diff --git a/fs/coredump.c b/fs/coredump.c
index 4ffd801aca01..87befc5df961 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -40,6 +40,7 @@
 #include <linux/compat.h>
 #include <linux/fs.h>
 #include <linux/wait_bit.h>
+#include <linux/io_uring.h>
 #include <linux/path.h>
 #include <linux/timekeeping.h>
 #include <linux/sysctl.h>
@@ -576,8 +577,10 @@ static bool coredump_close_files(struct core_state *core_state)
 	}
 
 	/* Use the dumper's real creds not the overridden ones. */
-	scoped_with_creds(current_real_cred())
+	scoped_with_creds(current_real_cred()) {
+		io_uring_task_cancel();
 		switch_files_struct(current, files);
+	}
 
 	coredump_wait_inactive(core_state);
 	return true;
diff --git a/kernel/exit.c b/kernel/exit.c
index 04f2c8c78879..4c94a475b958 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -452,6 +452,7 @@ static void coredump_task_exit(struct task_struct *tsk,
 		/* Pairs with the release in coredump_close_files(). */
 		if (smp_load_acquire(&self.files)) {
 			__set_current_state(TASK_RUNNING);
+			io_uring_task_cancel();
 			switch_files_struct(tsk, no_free_ptr(self.files));
 			atomic_dec_and_wake_up(&core_state->threads_remaining);
 			continue;

-- 
2.53.0

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

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