Re: [PATCH 1/8] signal: Make SIGKILL during coredumps an explicit special case

"Eric W. Biederman" [off-list ref] writes:

Olivier Langlois [off-list ref] writes:

quoted

On Mon, 2022-01-10 at 15:11 -0600, Eric W. Biederman wrote:

quoted

I have been able to confirm that changing wait_event_interruptible to
wait_event_killable was the culprit.  Something about the way
systemd-coredump handles coredumps is not compatible with
wait_event_killable.

This is my experience too that systemd-coredump is doing something
unexpected. When I tested the patch:
https://lore.kernel.org/lkml/cover.1629655338.git.olivier@trillion01.com/ (local)

to make sure that the patch worked, sending coredumps to systemd-
coredump was making systemd-coredump, well, core dump... Not very
useful...

Oh.  Wow....

quoted

Sending the dumps through a pipe to anything else than systemd-coredump
was working fine.

Interesting.

I need to read through the pipe code and see how all of that works.  For
writing directly to disk only ignoring killable interruptions are the
usual semantics.  Ordinary pipe code has different semantics, and I
suspect that is what is tripping things up.

As for systemd-coredump it does whatever it does and I suspect some
versions of systemd-coredump are simply not robust if a coredump stops
unexpectedly.

The good news is the pipe code is simple enough, it will be possible to
completely read through that code.

My bug, obvious in hindsight is that "try_to_wait_up(TASK_INTERRUPTIBLE)"
does not work on a task that is in sleeping in TASK_KILLABLE.
That looks fixable in wait_for_dump_helpers it just won't be as easy
as changing wait_event_interruptible to wait_event_killable.

To prevent short pipe write from causing short writes during a coredump
I believe all we need to do handle -ERSTARTSYS with TIF_NOTIFY_SIGNAL.
Something like what I have below.

Until wait_for_dump_helpers is sorted out the coredump won't wait for
the dump helper the way it should, but otherwise things should work.

diff --git a/fs/coredump.c b/fs/coredump.c
index 7dece20b162b..0db1baf91420 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c

@@ -796,6 +796,10 @@ static int __dump_emit(struct coredump_params *cprm, const void *addr, int nr)
 	if (dump_interrupted())
 		return 0;
 	n = __kernel_write(file, addr, nr, &pos);
+	while ((n == -ERESTARTSYS) && test_thread_flag(TIF_NOTIFY_SIGNAL)) {
+		tracehook_notify_signal();
+		n = __kernel_write(file, addr, nr, &pos);
+	}
 	if (n != nr)
 		return 0;
 	file->f_pos = pos;

Eric