All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: David Herrmann <dh.herrmann@gmail.com>,
	Tony Battersby <tonyb@cybernetics.com>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-mm <linux-mm@kvack.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	Andy Lutomirski <luto@amacapital.net>
Subject: Re: [PATCH 2/6] shm: add sealing API
Date: Fri, 11 Apr 2014 14:36:39 -0700	[thread overview]
Message-ID: <53486067.6090506@mit.edu> (raw)
In-Reply-To: <CANq1E4T=38VLezGH2XUZ9kc=Vtp6Ca++-ATwmEfaXZS6UrTPig@mail.gmail.com>

On 04/11/2014 02:31 PM, David Herrmann wrote:
> Hi
> 
> On Fri, Apr 11, 2014 at 3:43 PM, Tony Battersby <tonyb@cybernetics.com> wrote:
>> Exactly.  For O_DIRECT, that would be the call to get_user_pages_fast()
>> from dio_refill_pages() in fs/direct-io.c, which is ultimately called
>> from blkdev_direct_IO().
> 
> If you drop mmap_sem after pinning a page without taking a write-ref,
> you break i_mmap_writable / VM_DENYWRITE. In memfd I rely on
> i_mmap_writable to work, same thing is done by exec() (and the old,
> now disabled, MAP_DENYWRITE).
> 
> I don't know whether I should care. I mean, everyone pinning pages and
> writing to it without holding the mmap_sem has to take a write-ref for
> each page or it breaks i_mmap_writable. So this seems to be a bug in
> direct-IO, not in anyone relying on it, right?

A quick grep of the kernel tree finds exactly zero code paths
incrementing i_mmap_writable outside of mmap and fork.

Or do you mean a different kind of write ref?  What am I missing here?

--Andy

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: Andy Lutomirski <luto@amacapital.net>
To: David Herrmann <dh.herrmann@gmail.com>,
	Tony Battersby <tonyb@cybernetics.com>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-mm <linux-mm@kvack.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	Andy Lutomirski <luto@amacapital.net>
Subject: Re: [PATCH 2/6] shm: add sealing API
Date: Fri, 11 Apr 2014 14:36:39 -0700	[thread overview]
Message-ID: <53486067.6090506@mit.edu> (raw)
In-Reply-To: <CANq1E4T=38VLezGH2XUZ9kc=Vtp6Ca++-ATwmEfaXZS6UrTPig@mail.gmail.com>

On 04/11/2014 02:31 PM, David Herrmann wrote:
> Hi
> 
> On Fri, Apr 11, 2014 at 3:43 PM, Tony Battersby <tonyb@cybernetics.com> wrote:
>> Exactly.  For O_DIRECT, that would be the call to get_user_pages_fast()
>> from dio_refill_pages() in fs/direct-io.c, which is ultimately called
>> from blkdev_direct_IO().
> 
> If you drop mmap_sem after pinning a page without taking a write-ref,
> you break i_mmap_writable / VM_DENYWRITE. In memfd I rely on
> i_mmap_writable to work, same thing is done by exec() (and the old,
> now disabled, MAP_DENYWRITE).
> 
> I don't know whether I should care. I mean, everyone pinning pages and
> writing to it without holding the mmap_sem has to take a write-ref for
> each page or it breaks i_mmap_writable. So this seems to be a bug in
> direct-IO, not in anyone relying on it, right?

A quick grep of the kernel tree finds exactly zero code paths
incrementing i_mmap_writable outside of mmap and fork.

Or do you mean a different kind of write ref?  What am I missing here?

--Andy

  reply	other threads:[~2014-04-11 21:36 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-10 21:33 [PATCH 2/6] shm: add sealing API Tony Battersby
2014-04-10 21:33 ` Tony Battersby
2014-04-11  0:22 ` David Herrmann
2014-04-11  0:22   ` David Herrmann
2014-04-11  0:22   ` David Herrmann
2014-04-11  1:27   ` Andy Lutomirski
2014-04-11  1:27     ` Andy Lutomirski
2014-04-11 13:43     ` Tony Battersby
2014-04-11 13:43       ` Tony Battersby
2014-04-11 21:31       ` David Herrmann
2014-04-11 21:31         ` David Herrmann
2014-04-11 21:31         ` David Herrmann
2014-04-11 21:36         ` Andy Lutomirski [this message]
2014-04-11 21:36           ` Andy Lutomirski
2014-04-11 21:42           ` David Herrmann
2014-04-11 21:42             ` David Herrmann
2014-04-11 21:42             ` David Herrmann
2014-04-11 22:07             ` Andy Lutomirski
2014-04-11 22:07               ` Andy Lutomirski
2014-04-14  0:03               ` David Herrmann
2014-04-14  0:03                 ` David Herrmann
2014-04-14  0:03                 ` David Herrmann
  -- strict thread matches above, loose matches on Subject: below --
2014-03-19 19:06 [PATCH 0/6] File Sealing & memfd_create() David Herrmann
2014-03-19 19:06 ` [PATCH 2/6] shm: add sealing API David Herrmann
2014-03-19 19:06   ` David Herrmann
2014-03-19 19:06   ` David Herrmann
2014-03-19 19:06   ` David Herrmann

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=53486067.6090506@mit.edu \
    --to=luto@amacapital.net \
    --cc=dh.herrmann@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=tonyb@cybernetics.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 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.