All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Olivier Langlois <olivier@trillion01.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Oleg Nesterov <oleg@redhat.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	io-uring <io-uring@vger.kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Jens Axboe <axboe@kernel.dk>,
	"Pavel Begunkov>" <asml.silence@gmail.com>
Subject: Re: [PATCH] coredump: Limit what can interrupt coredumps
Date: Fri, 18 Jun 2021 16:05:36 -0400	[thread overview]
Message-ID: <b8327afcd3ba1d9a2d2def40343efb2e79c489b7.camel@trillion01.com> (raw)
In-Reply-To: <87v96dd1gz.fsf@disp2133>

On Wed, 2021-06-16 at 15:00 -0500, Eric W. Biederman wrote:
> Olivier Langlois <olivier@trillion01.com> writes:
> 
> > I redid my test but this time instead of dumping directly into a
> > file,
> > I did let the coredump be piped to the systemd coredump module and
> > the
> > coredump generation isn't working as expected when piping.
> > 
> > So your code review conclusions are correct.
> 
> Thank you for confirming that.
> 
> Do you know how your test program is using io_uring?
> 
> I have been trying to put the pieces together on what io_uring is
> doing
> that stops the coredump.  The fact that it takes a little while
> before
> it kills the coredump is a little puzzling.  The code looks like all
> of
> the io_uring operations should have been canceled before the coredump
> starts.
> 
> 
With a very simple setup, I guess that this could easily be
reproducible. Make a TCP connection with a server that is streaming
non-stop data and enter a loop where you keep initiating async
OP_IOURING_READ operations on your TCP fd.

Once you have that, manually sending a SIG_SEGV is a sure fire way to
stumble into the problem. This is how I am testing the patches.

IRL, it is possible to call io_uring_enter() to submit operations and
return from the syscall without waiting on all events to have
completed. Once the process is back in userspace, if it stumble into a
bug that triggers a coredump, any remaining pending I/O operations can
set TIF_SIGNAL_NOTIFY while the coredump is generated.

I have read the part of your previous email where you share the result
of your ongoing investigation. I didn't comment as the definitive
references in io_uring matters are Jens and Pavel but I am going to
share my opinion on the matter.

I think that you did put the finger on the code cleaning up the
io_uring instance in regards to pending operations. It seems to be
io_uring_release() which is probably called from exit_files() which
happens to be after the call to exit_mm().

At first, I did entertain the idea of considering if it could be
possible to duplicate some of the operations performed by
io_uring_release() related to the infamous TIF_SIGNAL_NOTIFY setting
into io_uring_files_cancel() which is called before exit_mm().

but the idea is useless as it is not the other threads of the group
that are causing the TIF_SIGNAL_NOTIFY problem. It is the thread
calling do_coredump() which is done by the signal handing code even
before that thread enters do_exit() and start to be cleaned up. That
thread when it enters do_coredump() is still fully loaded and
operational in terms of io_uring functionality.

I guess that this io_uring cancel all pending operations hook would
have to be called from do_coredump or from get_signal() but if it is
the way to go, I feel that this is a change major enough that wouldn't
dare going there without the blessing of the maintainers in cause....



  reply	other threads:[~2021-06-18 20:05 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <192c9697e379bf084636a8213108be6c3b948d0b.camel@trillion01.com>
     [not found] ` <9692dbb420eef43a9775f425cb8f6f33c9ba2db9.camel@trillion01.com>
     [not found]   ` <87h7i694ij.fsf_-_@disp2133>
2021-06-09 20:33     ` [RFC] coredump: Do not interrupt dump for TIF_NOTIFY_SIGNAL Linus Torvalds
2021-06-09 20:48       ` Eric W. Biederman
2021-06-09 20:52         ` Linus Torvalds
2021-06-09 21:02       ` Olivier Langlois
2021-06-09 21:05         ` Eric W. Biederman
2021-06-09 21:26           ` Olivier Langlois
2021-06-09 21:56             ` Olivier Langlois
2021-06-10 14:26             ` Eric W. Biederman
2021-06-10 15:17               ` Olivier Langlois
2021-06-10 18:58               ` [CFT}[PATCH] coredump: Limit what can interrupt coredumps Eric W. Biederman
2021-06-10 19:10                 ` Linus Torvalds
2021-06-10 19:18                   ` Eric W. Biederman
2021-06-10 19:50                     ` Linus Torvalds
2021-06-10 20:11                       ` [PATCH] " Eric W. Biederman
2021-06-10 21:04                         ` Linus Torvalds
2021-06-12 14:36                         ` Olivier Langlois
2021-06-12 16:26                           ` Jens Axboe
2021-06-14 14:10                         ` Oleg Nesterov
2021-06-14 16:37                           ` Eric W. Biederman
2021-06-14 16:59                             ` Oleg Nesterov
2021-06-15 22:08                           ` Eric W. Biederman
2021-06-16 19:23                             ` Olivier Langlois
2021-06-16 20:00                               ` Eric W. Biederman
2021-06-18 20:05                                 ` Olivier Langlois [this message]
2021-08-05 13:06                             ` Olivier Langlois
2021-08-10 21:48                               ` Tony Battersby
2021-08-11 20:47                                 ` Olivier Langlois
2021-08-12  1:55                                 ` Jens Axboe
2021-08-12 13:53                                   ` Tony Battersby
2021-08-15 20:42                                   ` Olivier Langlois
2021-08-16 13:02                                     ` Pavel Begunkov
2021-08-16 13:06                                       ` Pavel Begunkov
2021-08-17 18:15                                     ` Jens Axboe
2021-08-17 18:24                                       ` Jens Axboe
2021-08-17 19:29                                         ` Tony Battersby
2021-08-17 19:59                                           ` Jens Axboe
2021-08-17 21:28                                             ` Jens Axboe
2021-08-17 21:39                                               ` Tony Battersby
2021-08-17 22:05                                                 ` Jens Axboe
2021-08-18 14:37                                                   ` Tony Battersby
2021-08-18 14:46                                                     ` Jens Axboe
2021-08-18  2:57                                               ` Jens Axboe
2021-08-18  2:58                                                 ` Jens Axboe
2021-08-21 10:08                                                 ` Olivier Langlois
2021-08-21 16:47                                                   ` Olivier Langlois
2021-08-21 16:51                                                     ` Jens Axboe
2021-08-21 17:21                                                       ` Olivier Langlois
2021-08-21  9:52                                         ` Olivier Langlois
2021-08-21  9:48                                       ` Olivier Langlois
2021-10-22 14:13     ` [RFC] coredump: Do not interrupt dump for TIF_NOTIFY_SIGNAL Pavel Begunkov
2021-12-24  1:34       ` Olivier Langlois
2021-12-24 10:37         ` Pavel Begunkov
2021-12-24 19:52           ` Eric W. Biederman
2021-12-28 11:24             ` Pavel Begunkov
2022-03-14 23:58               ` Eric W. Biederman
     [not found]                 ` <8218f1a245d054c940e25142fd00a5f17238d078.camel@trillion01.com>
2022-06-01  3:15                   ` Jens Axboe
2022-07-20 16:49                     ` [PATCH 0/2] coredump: Allow io_uring using apps to dump to pipes Eric W. Biederman
2022-07-20 16:50                       ` [PATCH 1/2] signal: Move stopping for the coredump from do_exit into get_signal Eric W. Biederman
2022-07-20 16:51                       ` [PATCH 2/2] coredump: Allow coredumps to pipes to work with io_uring Eric W. Biederman
2022-08-22 21:16                         ` Olivier Langlois
2022-08-23  3:35                           ` Olivier Langlois
2022-08-23 18:22                             ` Eric W. Biederman
2022-08-23 18:27                               ` Jens Axboe
2022-08-24 15:11                                 ` Eric W. Biederman
2022-08-24 15:51                                   ` Jens Axboe
2022-01-05 19:39           ` [RFC] coredump: Do not interrupt dump for TIF_NOTIFY_SIGNAL Olivier Langlois

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b8327afcd3ba1d9a2d2def40343efb2e79c489b7.camel@trillion01.com \
    --to=olivier@trillion01.com \
    --cc=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=ebiederm@xmission.com \
    --cc=io-uring@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.