linux-kernel - Re: [PATCH v4 0/3] MAP_DIRECT and block-map sealed files

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 15 Aug 2017 19:01:16 +1000
From:   Dave Chinner <david@...morbit.com>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     darrick.wong@...cle.com, Jan Kara <jack@...e.cz>,
        Arnd Bergmann <arnd@...db.de>, linux-nvdimm@...ts.01.org,
        linux-api@...r.kernel.org, linux-kernel@...r.kernel.org,
        Christoph Hellwig <hch@....de>, linux-xfs@...r.kernel.org,
        linux-mm@...ck.org, Jeff Moyer <jmoyer@...hat.com>,
        Alexander Viro <viro@...iv.linux.org.uk>, luto@...nel.org,
        linux-fsdevel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Ross Zwisler <ross.zwisler@...ux.intel.com>
Subject: Re: [PATCH v4 0/3] MAP_DIRECT and block-map sealed files

On Mon, Aug 14, 2017 at 11:12:05PM -0700, Dan Williams wrote:
> Changes since v3 [1]:
> * Move from an fallocate(2) interface to a new mmap(2) flag and rename
>   'immutable' to 'sealed'.
> 
> * Do not record the sealed state in permanent metadata it is now purely
>   a temporary state for as long as a MAP_DIRECT vma is referencing the
>   inode (Christoph)
> 
> * Drop the CAP_IMMUTABLE requirement, but do require a PROT_WRITE
>   mapping.
> 
> [1]: https://lwn.net/Articles/730570/
> 
> ---
> 
> This is the next revision of a patch series that aims to enable
> applications that otherwise need to resort to DAX mapping a raw device
> file to instead move to a filesystem.
> 
> In the course of reviewing a previous posting, Christoph said:
> 
>     That being said I think we absolutely should support RDMA memory
>     registrations for DAX mappings.  I'm just not sure how S_IOMAP_IMMUTABLE
>     helps with that.  We'll want a MAP_SYNC | MAP_POPULATE to make sure all
>     the blocks are populated and all ptes are set up.  Second we need to
>     make sure get_user_page works, which for now means we'll need a struct
>     page mapping for the region (which will be really annoying for PCIe
>     mappings, like the upcoming NVMe persistent memory region), and we need
>     to guarantee that the extent mapping won't change while the
>     get_user_pages holds the pages inside it.  I think that is true due to
>     side effects even with the current DAX code, but we'll need to make it
>     explicit.  And maybe that's where we need to converge - "sealing" the
>     extent map makes sense as such a temporary measure that is not persisted
>     on disk, which automatically gets released when the holding process
>     exits, because we sort of already do this implicitly.  It might also
>     make sense to have explicitly breakable seals similar to what I do for
>     the pNFS blocks kernel server, as any userspace RDMA file server would
>     also need those semantics.
> 
> So, this is an attempt to converge on the idea that we need an explicit
> and process-lifetime-temporary mechanism for a process to be able to
> make assumptions about the mapping to physical page to dax-file-offset
> relationship. The "explicitly breakable seals" aspect is not addressed
> in these patches, but I wonder if it might be a voluntary mechanism that
> can implemented via userfaultfd.
> 
> These pass a basic smoke test and are meant to just gauge 'right track'
> / 'wrong track'. The main question it seems is whether the pinning done
> in this patchset is too early (applies before get_user_pages()) and too
> coarse (applies to the whole file). Perhaps this is where I discarded
> too easily Jan's suggestion to look at Peter Z's mm_mpin() syscall [2]? On
> the other hand, the coarseness and simple lifetime rules of MAP_DIRECT
> make it an easy mechanism to implement and explain.
> 
> Another reason I kept the scope of S_IOMAP_SEALED coarsely defined was
> to support Dave's desired use case of sealing for operating on reflinked
> files [3].

Which really needs a fcntl() interface to set/clear iomap seals.

Which, now that I look at it, already has a bunch of "file sealing"
commands defined which arrived in 3.17. It appears to be a special
purpose access control interface for memfd_create() to manage shared
access to anonymous tmpfs files and will EINVAL on any fd that
points to a real file.

Oh, even more problematic:

	Seals are a property of an inode. [....] Furthermore, seals
	can never be removed, only added.

That seems somewhat difficult to reconcile with how I need
F_SEAL_IOMAP to operate.

/me calls it a day and goes looking for the hard liquor.....

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com