linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
To: "rppt@kernel.org" <rppt@kernel.org>
Cc: "david@redhat.com" <david@redhat.com>,
	"cl@linux.com" <cl@linux.com>, "hpa@zytor.com" <hpa@zytor.com>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"catalin.marinas@arm.com" <catalin.marinas@arm.com>,
	"linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	"will@kernel.org" <will@kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"idan.yaniv@ibm.com" <idan.yaniv@ibm.com>,
	"kirill@shutemov.name" <kirill@shutemov.name>,
	"viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>,
	"rppt@linux.ibm.com" <rppt@linux.ibm.com>,
	"Williams, Dan J" <dan.j.williams@intel.com>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"bp@alien8.de" <bp@alien8.de>,
	"willy@infradead.org" <willy@infradead.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"luto@kernel.org" <luto@kernel.org>,
	"shuah@kernel.org" <shuah@kernel.org>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
	"x86@kernel.org" <x86@kernel.org>,
	"linux-riscv@lists.infradead.org"
	<linux-riscv@lists.infradead.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"Reshetova, Elena" <elena.reshetova@intel.com>,
	"palmer@dabbelt.com" <palmer@dabbelt.com>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"mtk.manpages@gmail.com" <mtk.manpages@gmail.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"tycho@tycho.ws" <tycho@tycho.ws>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-api@vger.kernel.org" <linux-api@vger.kernel.org>,
	"jejb@linux.ibm.com" <jejb@linux.ibm.com>,
	"paul.walmsley@sifive.com" <paul.walmsley@sifive.com>
Subject: Re: [PATCH v6 3/6] mm: introduce memfd_secret system call to create "secret" memory areas
Date: Wed, 30 Sep 2020 20:11:28 +0000	[thread overview]
Message-ID: <b5d8e90c5366a42e7ad0a337fba5f2b1bcfe52c2.camel@intel.com> (raw)
In-Reply-To: <20200930103507.GK2142832@kernel.org>

On Wed, 2020-09-30 at 13:35 +0300, Mike Rapoport wrote:
> On Tue, Sep 29, 2020 at 08:06:03PM +0000, Edgecombe, Rick P wrote:
> > On Tue, 2020-09-29 at 16:06 +0300, Mike Rapoport wrote:
> > > On Tue, Sep 29, 2020 at 04:58:44AM +0000, Edgecombe, Rick P
> > > wrote:
> > > > On Thu, 2020-09-24 at 16:29 +0300, Mike Rapoport wrote:
> > > > > Introduce "memfd_secret" system call with the ability to
> > > > > create
> > > > > memory
> > > > > areas visible only in the context of the owning process and
> > > > > not
> > > > > mapped not
> > > > > only to other processes but in the kernel page tables as
> > > > > well.
> > > > > 
> > > > > The user will create a file descriptor using the
> > > > > memfd_secret()
> > > > > system call
> > > > > where flags supplied as a parameter to this system call will
> > > > > define
> > > > > the
> > > > > desired protection mode for the memory associated with that
> > > > > file
> > > > > descriptor.
> > > > > 
> > > > >   Currently there are two protection modes:
> > > > > 
> > > > > * exclusive - the memory area is unmapped from the kernel
> > > > > direct
> > > > > map
> > > > > and it
> > > > >                is present only in the page tables of the
> > > > > owning
> > > > > mm.
> > > > 
> > > > Seems like there were some concerns raised around direct map
> > > > efficiency, but in case you are going to rework this...how does
> > > > this
> > > > memory work for the existing kernel functionality that does
> > > > things
> > > > like
> > > > this?
> > > > 
> > > > get_user_pages(, &page);
> > > > ptr = kmap(page);
> > > > foo = *ptr;
> > > > 
> > > > Not sure if I'm missing something, but I think apps could cause
> > > > the
> > > > kernel to access a not-present page and oops.
> > > 
> > > The idea is that this memory should not be accessible by the
> > > kernel,
> > > so
> > > the sequence you describe should indeed fail.
> > > 
> > > Probably oops would be to noisy and in this case the report needs
> > > to
> > > be
> > > less verbose.
> > 
> > I was more concerned that it could cause kernel instabilities.
> 
> I think kernel recovers nicely from such sort of page fault, at least
> on
> x86.

We are talking about the kernel taking a direct map NP fault and
oopsing? Hmm, I thought it should often recover, but stability should
be considered reduced. How could the kernel know whether to release
locks or clean up other state? Pretty sure I've seen deadlocks in this
case.

> > I see, so it should not be accessed even at the userspace address?
> > I
> > wonder if it should be prevented somehow then. At least
> > get_user_pages() should be prevented I think. Blocking
> > copy_*_user()
> > access might not be simple.
> > 
> > I'm also not so sure that a user would never have any possible
> > reason
> > to copy data from this memory into the kernel, even if it's just
> > convenience. In which case a user setup could break if a specific
> > kernel implementation switched to get_user_pages()/kmap() from
> > using
> > copy_*_user(). So seems maybe a bit thorny without fully blocking
> > access from the kernel, or deprecating that pattern.
> > 
> > You should probably call out these "no passing data to/from the
> > kernel"
> > expectations, unless I missed them somewhere.
> 
> You are right, I should have been more explicit in the description of
> the expected behavoir. 
> 
> Our thinking was that copy_*user() would work in the context of the
> process that "owns" the secretmem and gup() would not allow access in
> general, unless requested with certail (yet another) FOLL_ flag.

Hmm, yes. I think one easier thing about this design over the series
Kirill sent out is that the actual page will never transition to and
from unmapped while it's mapped in userspace. If it could transition,
you'd have to worry about a race window between
get_user_pages(FOLL_foo) and the kmap() where the page might get
unmapped.

Without the ability to transition pages though, using this for KVM
guests memory remains a not completely worked through problem since it
has the get_user_pages()/kmap() pattern quite a bit. Did you have an
idea for that? (I thought I saw that use case mentioned somewhere).


  reply	other threads:[~2020-09-30 20:11 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-24 13:28 [PATCH v6 0/6] mm: introduce memfd_secret system call to create "secret" memory areas Mike Rapoport
2020-09-24 13:28 ` [PATCH v6 1/6] mm: add definition of PMD_PAGE_ORDER Mike Rapoport
2020-09-24 13:29 ` [PATCH v6 2/6] mmap: make mlock_future_check() global Mike Rapoport
2020-09-24 13:29 ` [PATCH v6 3/6] mm: introduce memfd_secret system call to create "secret" memory areas Mike Rapoport
2020-09-29  4:58   ` Edgecombe, Rick P
2020-09-29 13:06     ` Mike Rapoport
2020-09-29 20:06       ` Edgecombe, Rick P
2020-09-30 10:35         ` Mike Rapoport
2020-09-30 20:11           ` Edgecombe, Rick P [this message]
2020-10-11  9:42             ` Mike Rapoport
2020-09-24 13:29 ` [PATCH v6 4/6] arch, mm: wire up memfd_secret system call were relevant Mike Rapoport
2020-09-24 13:29 ` [PATCH v6 5/6] mm: secretmem: use PMD-size pages to amortize direct map fragmentation Mike Rapoport
2020-09-25  7:41   ` Peter Zijlstra
2020-09-25  9:00     ` David Hildenbrand
2020-09-25  9:50       ` Peter Zijlstra
2020-09-25 10:31         ` Mark Rutland
2020-09-25 14:57           ` Tycho Andersen
2020-09-29 14:04           ` Mike Rapoport
2020-09-29 13:07         ` Mike Rapoport
2020-09-29 13:06       ` Mike Rapoport
2020-09-29 13:05     ` Mike Rapoport
2020-09-29 14:12       ` Peter Zijlstra
2020-09-29 14:31         ` Dave Hansen
2020-09-29 14:58         ` Mike Rapoport
2020-09-29 15:15           ` Peter Zijlstra
2020-09-30 10:27             ` Mike Rapoport
2020-09-30 14:39               ` James Bottomley
2020-09-30 14:45                 ` David Hildenbrand
2020-09-30 15:17                   ` James Bottomley
2020-09-30 15:25                     ` David Hildenbrand
2020-09-30 15:09               ` Matthew Wilcox
2020-10-01  8:14                 ` Mike Rapoport
2020-09-29 15:03         ` James Bottomley
2020-09-30 10:20         ` Mike Rapoport
2020-09-30 10:43           ` Peter Zijlstra
2020-09-24 13:29 ` [PATCH v6 6/6] secretmem: test: add basic selftest for memfd_secret(2) Mike Rapoport
2020-09-24 13:35 ` [PATCH] man2: new page describing memfd_secret() system call Mike Rapoport
2020-09-24 14:55   ` Alejandro Colomar
2020-10-03  9:32     ` Alejandro Colomar
2020-10-05  7:32       ` Mike Rapoport
2020-11-16 21:01         ` [PATCH v2] memfd_secret.2: New " Alejandro Colomar
2020-11-17  6:26           ` Mike Rapoport
2020-09-25  2:34 ` [PATCH v6 0/6] mm: introduce memfd_secret system call to create "secret" memory areas Andrew Morton
2020-09-25  6:42   ` Mike Rapoport
2020-11-01 11:09 ` Hagen Paul Pfeifer
2020-11-02 15:40   ` Mike Rapoport
2020-11-03 13:52     ` Hagen Paul Pfeifer
2020-11-03 16:30       ` Mike Rapoport
2020-11-04 11:39         ` Hagen Paul Pfeifer
2020-11-04 17:02           ` Mike Rapoport
2020-11-09 10:41             ` Hagen Paul Pfeifer
2020-11-02  9:11 ` David Hildenbrand
2020-11-02  9:31   ` David Hildenbrand
2020-11-02 17:43   ` Mike Rapoport
2020-11-02 17:51     ` David Hildenbrand
2020-11-03  9:52       ` Mike Rapoport
2020-11-03 10:11         ` David Hildenbrand

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=b5d8e90c5366a42e7ad0a337fba5f2b1bcfe52c2.camel@intel.com \
    --to=rick.p.edgecombe@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --cc=cl@linux.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=david@redhat.com \
    --cc=elena.reshetova@intel.com \
    --cc=hpa@zytor.com \
    --cc=idan.yaniv@ibm.com \
    --cc=jejb@linux.ibm.com \
    --cc=kirill@shutemov.name \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.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=linux-nvdimm@lists.01.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=luto@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=mtk.manpages@gmail.com \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=peterz@infradead.org \
    --cc=rppt@kernel.org \
    --cc=rppt@linux.ibm.com \
    --cc=shuah@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tycho@tycho.ws \
    --cc=viro@zeniv.linux.org.uk \
    --cc=will@kernel.org \
    --cc=willy@infradead.org \
    --cc=x86@kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).