[PATCH 0/3] coredump: io_uring: Cancel io_uring to avoid core truncation

[PATCH 0/3] coredump: io_uring: Cancel io_uring to avoid core truncation

[Olivier Langlois]

Before writing the core dump, io_uring requests have to be cancelled.

Also, io_uring cancellation code had to be modified as it could set the
TIF_NOTIFY_SIGNAL bit.

Few notes about this patchset:

1. My coredump.c proposal puts the io_uring_task_cancel call further
down the do_coredump function over what Jens did propose.

Considering that this function call can be relatively expensive, I
believe that postponing it as much as possible is the way to go.

I did place it before coredump_wait which clears signal bits so that
seems to be an appropriate location but the logic could possibly be
pushed even more with possibly no harm.

2. The current patch proposal only address specifically the issue caused
by io_uring. It could reoccur as soon as something else flips the
TIF_NOTIFY_SIGNAL bit.

Therefore, another solution would simply be to modify __dump_emit to make it
resilient to TIF_NOTIFY_SIGNAL as Eric W. Biederman originally
suggested.

or maybe do both...

So making __dump_emit more robust to the TIF_NOTIFY_SIGNAL situation
might be something interesting to investigate if it would be a good idea
to do on top or in replacement to this patchset.

Lastly, Jens did already submit a patch to solve the same problem:
https://lkml.org/lkml/2021/8/17/1156

If his patch ends being a superior solution to the problem,
the first 2 patches of this set are still relevant.

Olivier Langlois (3):
  tracehook: Add a return value to tracehook_notify_signal
  io_uring: Clear TIF_NOTIFY_SIGNAL when cancelling requests
  coredump: cancel io_uring requests before dumping core

 fs/coredump.c             | 3 +++
 fs/io_uring.c             | 2 +-
 include/linux/tracehook.h | 8 ++++++--
 3 files changed, 10 insertions(+), 3 deletions(-)

-- 
2.32.0

[PATCH 1/3] tracehook: Add a return value to tracehook_notify_signal

[Olivier Langlois]

The return value indicates if task_work_run has been called.
This knowledge can be of value to the caller. In particular, it allows
io_uring to easily replace calls to io_run_task_work with
tracehook_notify_signal when clearing TIF_NOTIFY_SIGNAL is needed.

Signed-off-by: Olivier Langlois <olivier@trillion01.com>
---
 include/linux/tracehook.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 3e80c4bc66f7..1f778ed9c6e2 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -204,12 +204,16 @@ static inline void tracehook_notify_resume(struct pt_regs *regs)
  * is currently used by TWA_SIGNAL based task_work, which requires breaking
  * wait loops to ensure that task_work is noticed and run.
  */
-static inline void tracehook_notify_signal(void)
+static inline bool tracehook_notify_signal(void)
 {
+	bool ret;
+
 	clear_thread_flag(TIF_NOTIFY_SIGNAL);
 	smp_mb__after_atomic();
-	if (current->task_works)
+	ret = current->task_works;
+	if (ret)
 		task_work_run();
+	return ret;
 }
 
 /*
-- 
2.32.0

[PATCH 2/3] io_uring: Clear TIF_NOTIFY_SIGNAL when cancelling requests

[Olivier Langlois]

It is a reasonable expectation from a function  to leave the task struct
in a clean state. During io_uring_try_cancel_requests TIF_NOTIFY_SIGNAL
can be set.  Make sure that it is cleared by replacing calls to
io_run_task_work with tracehook_notify_signal

Signed-off-by: Olivier Langlois <olivier@trillion01.com>
---
 fs/io_uring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index a2e20a6fbfed..a9c83a5fc9f1 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -9032,7 +9032,7 @@ static void io_uring_try_cancel_requests(struct io_ring_ctx *ctx,
 		ret |= io_poll_remove_all(ctx, task, cancel_all);
 		ret |= io_kill_timeouts(ctx, task, cancel_all);
 		if (task)
-			ret |= io_run_task_work();
+			ret |= tracehook_notify_signal();
 		if (!ret)
 			break;
 		cond_resched();
-- 
2.32.0

[PATCH 3/3] coredump: cancel io_uring requests before dumping core

[Olivier Langlois]

The previous solution of ignoring the TIF_NOTIFY_SIGNAL bit while
dumping core is only working when the core dump is sent in a file.

When a pipe is used, pipe_write returns -ERESTARTSYS if signal_pending
which includes TIF_NOTIFY_SIGNAL is true.

A more robust solution is to make sure that io_uring will not set
TIF_NOTIFY_SIGNAL while the core dump is generated by cancelling all
the io_uring requests made by the current task before starting.

Fixes: 06af8679449d ("coredump: Limit what can interrupt coredumps")
Signed-off-by: Olivier Langlois <olivier@trillion01.com>
---
 fs/coredump.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/coredump.c b/fs/coredump.c
index 07afb5ddb1c4..9aceb4b3b40d 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -41,6 +41,7 @@
 #include <linux/fs.h>
 #include <linux/path.h>
 #include <linux/timekeeping.h>
+#include <linux/io_uring.h>
 
 #include <linux/uaccess.h>
 #include <asm/mmu_context.h>
@@ -625,6 +626,8 @@ void do_coredump(const kernel_siginfo_t *siginfo)
 		need_suid_safe = true;
 	}
 
+	io_uring_task_cancel();
+
 	retval = coredump_wait(siginfo->si_signo, &core_state);
 	if (retval < 0)
 		goto fail_creds;
-- 
2.32.0