Re: [RFC] coredump: Do not interrupt dump for TIF_NOTIFY_SIGNAL

From: Jens Axboe <axboe@kernel.dk>
To: Olivier Langlois <olivier@trillion01.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Pavel Begunkov <asml.silence@gmail.com>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	io-uring@vger.kernel.org,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Oleg Nesterov <oleg@redhat.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [RFC] coredump: Do not interrupt dump for TIF_NOTIFY_SIGNAL
Date: Tue, 31 May 2022 21:15:57 -0600	[thread overview]
Message-ID: <a29a1649-5e50-4221-9f44-66a35fbdff80@kernel.dk> (raw)
In-Reply-To: <8218f1a245d054c940e25142fd00a5f17238d078.camel@trillion01.com>

On 5/31/22 2:06 PM, Olivier Langlois wrote:
> On Mon, 2022-03-14 at 18:58 -0500, Eric W. Biederman wrote:
>>
>> Thank you very much for this.  There were some bugs elsewhere I had
>> to
>> deal with so I am slower looking at this part of the code than I was
>> expecting.
>>
>> I have now reproduced this with the commit reverted on current
>> kernels
>> and the repro.c from the syzcaller report.  I am starting to look
>> into
>> how this interacts with my planned code changes in this area.
>>
>> In combination with my other planned changes I think all that needs
>> to
>> happen in do_coredump is to clear TIF_NOTIFY_SIGNAL along with
>> TIF_SIGPENDING to prevent io_uring interaction problems.  But we will
>> see.
>>
>> The deadlock you demonstrate here shows that it is definitely not
>> enough
>> to clear TIF_NOTIFY_SIGNAL (without other changes) so that
>> signal_pending returns false, which I was hoping was be the case.
>>
>> Eric
> 
> I have been away for some time but if this is not resoved yet, I just
> want to remind that clearing TIF_NOTIFY_SIGNAL along with
> TIF_SIGPENDING won't do it because io_uring may set them asynchronously
> to report some io completion while do_coredump() is executing.
> 
> IMHO, just calling io_uring_task_cancel() from do_coredump() before
> actually writing the dump, while maybe not the perfect solution, is the
> simplest one.
> 
> Otherwise, maybe masking interrupts locally could work but I wouldn't
> dare to explore this option personally...

Eric, are you fine with doing the cancelation based patch for now? IMHO
it's not the ideal approach, but it will resolve the issue. And it'd
honestly be great to get some closure on this so we don't have truncated
core dumps if they are interrupted by task_work.

The best solution would be to make the core dumps resilient to
task_work, but a workable solution would be nice at this point...

-- 
Jens Axboe