mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jeff Moyer <jmoyer@redhat.com>
To: Benjamin LaHaise <bcrl@kvack.org>
Cc: Seth Jenkins <sethjenkins@google.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org, linux-aio@kvack.org,
	linux-kernel@vger.kernel.org, Jann Horn <jannh@google.com>,
	Pavel Emelyanov <xemul@parallels.com>,
	stable@vger.kernel.org, Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] aio: fix mremap after fork null-deref
Date: Fri, 13 Jan 2023 11:06:31 -0500	[thread overview]
Message-ID: <x49edryl8qg.fsf@segfault.boston.devel.redhat.com> (raw)
In-Reply-To: <20230112220939.GO19133@kvack.org> (Benjamin LaHaise's message of "Thu, 12 Jan 2023 17:09:39 -0500")

Hi, Ben,

Thanks for taking the time to chime in.

Benjamin LaHaise <bcrl@kvack.org> writes:

> On Thu, Jan 12, 2023 at 04:32:42PM -0500, Jeff Moyer wrote:
>> With the patch I proposed (flagging the ring buffer with VM_DONTCOPY),
>> the child process would still be unable to submit and reap I/Os via the
>> aio system calls.  What changes is that the child process would now be
>> unable to reap completions via the shared ring buffer.  In fact, because
>> the ring is no longer mapped in the child process, any attempt to access
>> that memory would result in a segmentation fault.  However, I would be
>> very surprised if the interface was being used in this way.
>> 
>> > If we're okay with this change though, I think it makes sense.
>> 
>> My preference is to make the interface consistent.  I think setting
>> VM_DONTCOPY on the mapping is the right way forward.  I'd welcome other
>> opinions on whether the potential risk is worth it.
>
> VM_DONTCOPY makes sense, but a SEGV is a pretty bad failure mode.  Any
> process reaping events in the child after fork() isn't going to be
> consistent in behaviour, and is able to see partial completion of an I/O
> and other inconsistencies, so they're going to be subtly broken at best.
>
> Unfortunately, we have no way of knowing if this behaviour is exercised
> anywhere without changing it and waiting for someone to holler.

OK, so it sounds like you would rather err on the side of caution.
That's fine with me.

Seth, I'll go back and review your patch.

Thanks!
Jeff


  reply	other threads:[~2023-01-13 16:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-04 21:25 Seth Jenkins
2023-01-10 22:39 ` Andrew Morton
2023-01-11 19:40 ` Jeff Moyer
2023-01-11 20:16   ` Seth Jenkins
2023-01-12 21:32     ` Jeff Moyer
2023-01-12 22:09       ` Benjamin LaHaise
2023-01-13 16:06         ` Jeff Moyer [this message]
2023-01-13 16:18 ` Jeff Moyer

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=x49edryl8qg.fsf@segfault.boston.devel.redhat.com \
    --to=jmoyer@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=bcrl@kvack.org \
    --cc=jannh@google.com \
    --cc=linux-aio@kvack.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sethjenkins@google.com \
    --cc=stable@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=xemul@parallels.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®