LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Michael Tirado <mtirado418@gmail.com>
To: Andy Lutomirski <luto@amacapital.net>
Cc: Jann Horn <jannh@google.com>,
	joel@joelfernandes.org, LKML <linux-kernel@vger.kernel.org>,
	jreck@google.com, john.stultz@linaro.org, tkjos@google.com,
	gregkh@linuxfoundation.org, hch@infradead.org,
	viro@zeniv.linux.org.uk,
	Andrew Morton <akpm@linux-foundation.org>,
	dancol@google.com, bfields@fieldses.org, jlayton@kernel.org,
	khalid.aziz@oracle.com, Lei.Yang@windriver.com,
	linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	linux-mm@kvack.org, marcandre.lureau@redhat.com,
	mike.kravetz@oracle.com, minchan@kernel.org, shuah@kernel.org,
	valdis.kletnieks@vt.edu, hughd@google.com,
	linux-api@vger.kernel.org
Subject: Re: [PATCH v3 resend 1/2] mm: Add an F_SEAL_FUTURE_WRITE seal to memfd
Date: Fri, 9 Nov 2018 20:02:14 +0000
Message-ID: <CAMkWEXOLJ=ymbVjQfA2MD8XA7Y9Lu3ByJYUY-JvpnYKJ5gkY1w@mail.gmail.com> (raw)
In-Reply-To: <A7EC46BC-441A-4A06-9E2F-A26DA88B5320@amacapital.net>

On Fri, Nov 9, 2018 at 9:41 PM Andy Lutomirski <luto@amacapital.net> wrote:
>
>
>
> > On Nov 9, 2018, at 1:06 PM, Jann Horn <jannh@google.com> wrote:
> >
> > +linux-api for API addition
> > +hughd as FYI since this is somewhat related to mm/shmem
> >
> > On Fri, Nov 9, 2018 at 9:46 PM Joel Fernandes (Google)
> > <joel@joelfernandes.org> wrote:
> >> Android uses ashmem for sharing memory regions. We are looking forward
> >> to migrating all usecases of ashmem to memfd so that we can possibly
> >> remove the ashmem driver in the future from staging while also
> >> benefiting from using memfd and contributing to it. Note staging drivers
> >> are also not ABI and generally can be removed at anytime.
> >>
> >> One of the main usecases Android has is the ability to create a region
> >> and mmap it as writeable, then add protection against making any
> >> "future" writes while keeping the existing already mmap'ed
> >> writeable-region active.  This allows us to implement a usecase where
> >> receivers of the shared memory buffer can get a read-only view, while
> >> the sender continues to write to the buffer.

Oh I remember trying this years ago with a new seal, F_SEAL_WRITE_PEER,
or something like that.

> >
> > So you're fiddling around with the file, but not the inode? How are
> > you preventing code like the following from re-opening the file as
> > writable?
> >
> > $ cat memfd.c
> > #define _GNU_SOURCE
> > #include <unistd.h>
> > #include <sys/syscall.h>
> > #include <printf.h>
> > #include <fcntl.h>
> > #include <err.h>
> > #include <stdio.h>
> >
> > int main(void) {
> >  int fd = syscall(__NR_memfd_create, "testfd", 0);
> >  if (fd == -1) err(1, "memfd");
> >  char path[100];
> >  sprintf(path, "/proc/self/fd/%d", fd);
> >  int fd2 = open(path, O_RDWR);
> >  if (fd2 == -1) err(1, "reopen");
> >  printf("reopen successful: %d\n", fd2);
> > }
> > $ gcc -o memfd memfd.c
> > $ ./memfd
> > reopen successful: 4
> > $
> >

The race condition between memfd_create and applying seals in fcntl?
I think it would be possible to block new write mappings from peer processes if
there is a new memfd_create api that accepts seals. Allowing caller to
set a seal
like the one I proposed years ago, though in a race-free manner. Then
also consider
how to properly handle blocking inherited +W mapping through
clone/fork. Maybe I'm
forgetting some other pitfalls?



> > That aside: I wonder whether a better API would be something that
> > allows you to create a new readonly file descriptor, instead of
> > fiddling with the writability of an existing fd.
>
> Every now and then I try to write a patch to prevent using proc to reopen a file with greater permission than the original open.
>
> I like your idea to have a clean way to reopen a a memfd with reduced permissions. But I would make it a syscall instead and maybe make it only work for memfd at first.  And the proc issue would need to be fixed, too.

IMO the best solution would handle the issue at memfd creation time by
removing the race condition.

  reply index

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-08  4:15 Joel Fernandes (Google)
2018-11-08  4:15 ` [PATCH v3 resend 2/2] selftests/memfd: Add tests for F_SEAL_FUTURE_WRITE seal Joel Fernandes (Google)
2018-11-09  8:49 ` [PATCH v3 resend 1/2] mm: Add an F_SEAL_FUTURE_WRITE seal to memfd Joel Fernandes
2018-11-09 20:36 ` Andrew Morton
2018-11-10  3:54   ` Joel Fernandes
2018-11-09 21:06 ` Jann Horn
2018-11-09 21:19   ` Jann Horn
2018-11-10  3:20     ` Joel Fernandes
2018-11-10  6:05       ` Andy Lutomirski
2018-11-10 18:24         ` Joel Fernandes
2018-11-10 18:45           ` Daniel Colascione
2018-11-10 19:11             ` Daniel Colascione
2018-11-10 19:55               ` Andy Lutomirski
2018-11-10 22:09               ` Joel Fernandes
2018-11-10 22:18                 ` Andy Lutomirski
2018-11-11  2:38                   ` Joel Fernandes
2018-11-11  3:40                     ` Andy Lutomirski
2018-11-11  4:01                       ` Joel Fernandes
2018-11-11  8:09                       ` Joel Fernandes
2018-11-11  8:30                         ` Daniel Colascione
2018-11-11 15:14                           ` Andy Lutomirski
2018-11-11 17:36                             ` Joel Fernandes
     [not found]       ` <CAKOZuethC7+YrRyyGciUCfhSSa9cCcAFJ8g_qEw9uh3TBbyOcg@mail.gmail.com>
2018-11-10 17:10         ` Joel Fernandes
2018-11-09 21:40   ` Andy Lutomirski
2018-11-09 20:02     ` Michael Tirado [this message]
2018-11-10  1:49       ` Joel Fernandes
2018-11-09 22:20   ` Daniel Colascione
2018-11-09 22:37     ` Andy Lutomirski
2018-11-09 22:42       ` Daniel Colascione
2018-11-09 23:14         ` Andy Lutomirski
2018-11-10  1:36           ` Joel Fernandes
2018-11-09 23:46   ` Joel Fernandes

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='CAMkWEXOLJ=ymbVjQfA2MD8XA7Y9Lu3ByJYUY-JvpnYKJ5gkY1w@mail.gmail.com' \
    --to=mtirado418@gmail.com \
    --cc=Lei.Yang@windriver.com \
    --cc=akpm@linux-foundation.org \
    --cc=bfields@fieldses.org \
    --cc=dancol@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@infradead.org \
    --cc=hughd@google.com \
    --cc=jannh@google.com \
    --cc=jlayton@kernel.org \
    --cc=joel@joelfernandes.org \
    --cc=john.stultz@linaro.org \
    --cc=jreck@google.com \
    --cc=khalid.aziz@oracle.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@amacapital.net \
    --cc=marcandre.lureau@redhat.com \
    --cc=mike.kravetz@oracle.com \
    --cc=minchan@kernel.org \
    --cc=shuah@kernel.org \
    --cc=tkjos@google.com \
    --cc=valdis.kletnieks@vt.edu \
    --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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git