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
next prev parent 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®