Oleg Nesterov <oleg@xxxxxxxxxx> writes: > Eric, et al, sorry for delay, I didn't read emails several days. > > On 06/10, Eric W. Biederman wrote: >> >> v2: Don't remove the now unnecessary code in prepare_signal. > > No, that code is still needed. Otherwise any fatal signal will be > turned into SIGKILL. > >> --- a/fs/coredump.c >> +++ b/fs/coredump.c >> @@ -519,7 +519,7 @@ static bool dump_interrupted(void) >> * but then we need to teach dump_write() to restart and clear >> * TIF_SIGPENDING. >> */ >> - return signal_pending(current); >> + return fatal_signal_pending(current) || freezing(current); >> } > > > Well yes, this is what the comment says. > > But note that there is another reason why dump_interrupted() returns true > if signal_pending(), it assumes thagt __dump_emit()->__kernel_write() may > fail anyway if signal_pending() is true. Say, pipe_write(), or iirc nfs, > perhaps something else... The pipe_write case is a good case to consider. In general filesystems are only allowed to stop if fatal_signal_pending. It is an ancient unix/posix thing. > That is why zap_threads() clears TIF_SIGPENDING. Perhaps it should clear > TIF_NOTIFY_SIGNAL as well and we should change io-uring to not abuse the > dumping threads? I would very much like some clarity on TIF_NOTIFY_SIGNAL. At the very least it would be nice if it could get renamed TIF_NOTIFY_TASK_WORK. I don't know what it has to do with signals. > Or perhaps we should change __dump_emit() to clear signal_pending() and > restart __kernel_write() if it fails or returns a short write. Given that the code needs to handle pipes that seems a reasonable thing to do. Note. All of the blocking of new signals in prepare_signal is still in place. So only a SIGKILL can come in set TIF_SIGPENDING. So I would say that the current fix is correct (and safe to backport). But still requires some magic in prepare_signal until we do more. I don't understand the logic with well enough of adding work to non-io_uring threads with task_work_add to understand why that happens in the first place. There are a lot of silly corners here. Yes please let's keep on digging. Eric
