Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <20171031184052.25253-5-marcandre.lureau@redhat.com>
References: <20171031184052.25253-1-marcandre.lureau@redhat.com> <20171031184052.25253-5-marcandre.lureau@redhat.com>
From:   David Herrmann <dh.herrmann@gmail.com>
Date:   Fri, 3 Nov 2017 18:03:54 +0100
Message-ID: <CANq1E4SC8Hi4h9hxUM70+qOL4K95cXuMF9DKGw4dGhfmktrqsA@mail.gmail.com>
Subject: Re: [PATCH 4/6] hugetlbfs: implement memfd sealing
To:     =?UTF-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= <marcandre.lureau@redhat.com>
Cc:     linux-mm <linux-mm@kvack.org>,
        linux-kernel <linux-kernel@vger.kernel.org>, aarcange@redhat.com,
        Hugh Dickins <hughd@google.com>, nyc@holomorphy.com,
        mike.kravetz@oracle.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi

On Tue, Oct 31, 2017 at 7:40 PM, Marc-Andr=C3=A9 Lureau
<marcandre.lureau@redhat.com> wrote:
> Implements memfd sealing, similar to shmem:
> - WRITE: deny fallocate(PUNCH_HOLE). mmap() write is denied in
>   memfd_add_seals(). write() doesn't exist for hugetlbfs.
> - SHRINK: added similar check as shmem_setattr()
> - GROW: added similar check as shmem_setattr() & shmem_fallocate()
>
> Except write() operation that doesn't exist with hugetlbfs, that
> should make sealing as close as it can be to shmem support.

SEAL, SHRINK, and GROW look fine to me.

Regarding WRITE you need to make sure there are no page references
left around. For instance, on shmem any process might trigger the
kernel to GUP mapped shmem pages for asynchronous IO, then unmap the
file and request F_SEAL_WRITE. In this case the seal must be rejected
*iff* the pages are still pinned. shmem does this by requiring the
page-refcounts to be 0. Preferably there would be some better
infrastructure that tells us whether someone operates on those pages,
but this does not exist right now. See shmem_wait_for_pins() for
details.

I have little knowledge on how hugetlbs integrate with the page-cache
and radix-tree, hence I'd prefer if someone can explicitly ACK that
shmem_wait_for_pins() is suitable for hugetlbfs.

Otherwise, this series looks good to me (minus the #ifdef mess..).

Thanks
David

From 1582909313731292688@xxx Wed Nov 01 23:45:57 +0000 2017
X-GM-THRID: 1582799756524927158
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread