linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Herrmann <dh.herrmann@gmail.com>
To: Andy Lutomirski <luto@amacapital.net>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Michael Kerrisk <mtk.manpages@gmail.com>,
	Ryan Lortie <desrt@desrt.ca>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	Linux FS Devel <linux-fsdevel@vger.kernel.org>,
	Linux API <linux-api@vger.kernel.org>,
	Greg Kroah-Hartman <greg@kroah.com>,
	John Stultz <john.stultz@linaro.org>,
	Lennart Poettering <lennart@poettering.net>,
	Daniel Mack <zonque@gmail.com>, Kay Sievers <kay@vrfy.org>,
	Hugh Dickins <hughd@google.com>,
	Tony Battersby <tonyb@cybernetics.com>
Subject: Re: [RFC v3 7/7] shm: isolate pinned pages when sealing files
Date: Fri, 13 Jun 2014 17:27:14 +0200	[thread overview]
Message-ID: <CANq1E4TQXKD8jaBcOJsL3h3ZPRXq176fz8Z9yevFbS3P0q1FQg@mail.gmail.com> (raw)
In-Reply-To: <CALCETrWaUsq_D2Z1PwbbwQQWKrnsWTLOdUR6bqPuedi8ZHgvEQ@mail.gmail.com>

Hi

On Fri, Jun 13, 2014 at 5:06 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Fri, Jun 13, 2014 at 3:36 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
>> When setting SEAL_WRITE, we must make sure nobody has a writable reference
>> to the pages (via GUP or similar). We currently check references and wait
>> some time for them to be dropped. This, however, might fail for several
>> reasons, including:
>>  - the page is pinned for longer than we wait
>>  - while we wait, someone takes an already pinned page for read-access
>>
>> Therefore, this patch introduces page-isolation. When sealing a file with
>> SEAL_WRITE, we copy all pages that have an elevated ref-count. The newpage
>> is put in place atomically, the old page is detached and left alone. It
>> will get reclaimed once the last external user dropped it.
>>
>> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>
> Won't this have unexpected effects?
>
> Thread 1:  start read into mapping backed by fd
>
> Thread 2:  SEAL_WRITE
>
> Thread 1: read finishes.  now the page doesn't match the sealed page

Just to be clear: you're talking about read() calls that write into
the memfd? (like my FUSE example does) Your language might be
ambiguous to others as "read into" actually implies a write.

No, this does not have unexpected effects. But yes, your conclusion is
right. To be clear, this behavior would be part of the API. Any
asynchronous write might be cut off by SEAL_WRITE _iff_ you unmap your
buffer before the write finishes. But you actually have to extend your
example:

Thread 1: p = mmap(memfd, SIZE);
Thread 1: h = async_read(some_fd, p, SIZE);
Thread 1: munmap(p, SIZE);
Thread 2: SEAL_WRITE
Thread 1: async_wait(h);

If you don't do the unmap(), then SEAL_WRITE will fail due to an
elevated i_mmap_writable. I think this is fine. In fact, I remember
reading that async-IO is not required to resolve user-space addresses
at the time of the syscall, but might delay it to the time of the
actual write. But you're right, it would be misleading that the AIO
operation returns success. This would be part of the memfd-API,
though. And if you mess with your address space while running an
async-IO operation on it, you're screwed anyway.

Btw., your sealing use-case is really odd. No-one guarantees that the
SEAL_WRITE happens _after_ you schedule your async-read. In case you
have some synchronization there, you just have to move it after
waiting for your async-io to finish.

Does that clear things up?
Thanks
David

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2014-06-13 15:27 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-13 10:36 [PATCH v3 0/7] File Sealing & memfd_create() David Herrmann
2014-06-13 10:36 ` [PATCH v3 1/7] mm: allow drivers to prevent new writable mappings David Herrmann
2014-07-09  8:55   ` Hugh Dickins
2014-07-19 16:12     ` David Herrmann
2014-06-13 10:36 ` [PATCH v3 2/7] shm: add sealing API David Herrmann
2014-07-16 10:06   ` Hugh Dickins
2014-07-19 16:17     ` David Herrmann
2014-06-13 10:36 ` [PATCH v3 3/7] shm: add memfd_create() syscall David Herrmann
2014-06-13 12:27   ` Michael Kerrisk (man-pages)
2014-06-13 12:41     ` David Herrmann
2014-06-13 14:20       ` Michael Kerrisk (man-pages)
2014-06-13 16:20         ` John Stultz
2014-06-16  4:12           ` Michael Kerrisk (man-pages)
2014-07-08 18:39         ` David Herrmann
2014-06-15 10:50   ` Jann Horn
2014-07-16 10:07   ` Hugh Dickins
2014-07-19 16:29     ` David Herrmann
2014-06-13 10:36 ` [PATCH v3 4/7] selftests: add memfd_create() + sealing tests David Herrmann
2014-07-16 10:07   ` Hugh Dickins
2014-07-19 16:31     ` David Herrmann
2014-06-13 10:36 ` [PATCH v3 5/7] selftests: add memfd/sealing page-pinning tests David Herrmann
2014-07-16 10:08   ` Hugh Dickins
2014-07-19 16:32     ` David Herrmann
2014-06-13 10:36 ` [RFC v3 6/7] shm: wait for pins to be released when sealing David Herrmann
2014-07-16 10:09   ` Hugh Dickins
2014-07-19 16:36     ` David Herrmann
2014-06-13 10:36 ` [RFC v3 7/7] shm: isolate pinned pages when sealing files David Herrmann
2014-06-13 15:06   ` Andy Lutomirski
2014-06-13 15:27     ` David Herrmann [this message]
2014-06-13 17:23       ` Andy Lutomirski
2014-07-09  8:57   ` Hugh Dickins
2014-07-19 16:40     ` David Herrmann
2014-06-13 15:10 ` [PATCH v3 0/7] File Sealing & memfd_create() Andy Lutomirski
2014-06-13 15:15   ` David Herrmann
2014-06-13 15:17     ` Andy Lutomirski
2014-06-13 15:33       ` David Herrmann
2014-06-17  9:54         ` Florian Weimer
2014-06-17 10:01           ` David Herrmann
2014-06-17 10:04             ` Florian Weimer
2014-06-17 10:10               ` David Herrmann
2014-06-17 12:13                 ` Florian Weimer
2014-06-17 13:26                   ` David Herrmann
2014-06-17 16:20             ` Andy Lutomirski
2014-06-17 16:36               ` David Herrmann
2014-06-17 16:41                 ` Andy Lutomirski
2014-06-17 16:51                   ` David Herrmann
2014-06-17 17:01                     ` Andy Lutomirski
2014-06-17 20:31                       ` Hugh Dickins
2014-06-17 21:25                         ` Andy Lutomirski
2014-07-08 16:54 ` David Herrmann
2014-07-09  8:53   ` Hugh Dickins

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=CANq1E4TQXKD8jaBcOJsL3h3ZPRXq176fz8Z9yevFbS3P0q1FQg@mail.gmail.com \
    --to=dh.herrmann@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=desrt@desrt.ca \
    --cc=greg@kroah.com \
    --cc=hughd@google.com \
    --cc=john.stultz@linaro.org \
    --cc=kay@vrfy.org \
    --cc=lennart@poettering.net \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@amacapital.net \
    --cc=mtk.manpages@gmail.com \
    --cc=tonyb@cybernetics.com \
    --cc=torvalds@linux-foundation.org \
    --cc=zonque@gmail.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