All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Theodore Ts'o <tytso@mit.edu>
Cc: linux-fscrypt@vger.kernel.org, linux-ext4@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net,
	linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org,
	linux-integrity@vger.kernel.org, Jaegeuk Kim <jaegeuk@kernel.org>,
	Victor Hsieh <victorhsieh@google.com>,
	Dave Chinner <david@fromorbit.com>,
	Christoph Hellwig <hch@lst.de>,
	"Darrick J . Wong" <darrick.wong@oracle.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH v4 01/16] fs-verity: add a documentation file
Date: Tue, 18 Jun 2019 09:31:18 -0700	[thread overview]
Message-ID: <20190618163116.GA184520@gmail.com> (raw)
In-Reply-To: <20190615123920.GB6142@mit.edu>

Hi Ted,

On Sat, Jun 15, 2019 at 08:39:20AM -0400, Theodore Ts'o wrote:
> On Thu, Jun 06, 2019 at 08:51:50AM -0700, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > Add a documentation file for fs-verity, covering....
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> 
> Looks good; you can add:
> 
> Reviewed-by: Theodore Ts'o <tytso@mit.edu>
> 
> 
> One minor design point below:
> 
> > +ext4 stores the verity metadata (Merkle tree and fsverity_descriptor)
> > +past the end of the file, starting at the first page fully beyond
>                                                    ^^^^
> > +i_size.  This approach works because (a) verity files are readonly,
> > +and (b) pages fully beyond i_size aren't visible to userspace but can
> > +be read/written internally by ext4 with only some relatively small
> > +changes to ext4.  This approach avoids having to depend on the
> > +EA_INODE feature and on rearchitecturing ext4's xattr support to
> > +support paging multi-gigabyte xattrs into memory, and to support
> > +encrypting xattrs.  Note that the verity metadata *must* be encrypted
> > +when the file is, since it contains hashes of the plaintext data.
> 
> If we ever want to support mounting, say, a file system with 4k blocks
> and fsverity enabled on a architecture with a 16k or 64k page size,
> then "page" in that first sentence will need to become "block".  At
> the moment we only support fsverity when page size == block size, so
> it's not an issue.
> 
> However, it's worth reflecting on what this means.  In order to
> satisfy this requirement (from the mmap man page):
> 
>        A file is mapped in multiples of the page size.  For a file
>        that is not a multiple of the page size, the remaining memory
>        is zeroed when mapped...
> 
> we're going to have to special case how the last page gets mmaped.
> The simplest way to do this will be to map in an anonymous page which
> just has the blocks that are part of the data block copied in, and the
> rest of the page can be zero'ed.
> 
> One thing we might consider doing just to make life much easier for
> ourselves (should we ever want to support page size != block size ---
> which I could imagine some folks like Chandan might find desirable) is
> to specify that the fsverity metadata begins at an offset which begins
> at i_size rounded up to the next 64k binary, which should handle all
> current and future architectures' page sizes.
> 

Thanks for the review.  Good point; I think we should just go with the "always
round up to the next 64K boundary" method.  Special-casing how the last page
gets mmap()ed seems it would be really painful.

Since there can be a hole between the end of the file and the start of the
verity metadata, this doesn't even necessarily use any additional disk space.

For consistency and since there is little downside I think I'll do the same for
f2fs too, though f2fs doesn't currently support PAGE_SIZE != 4096 at all anyway.

- Eric

WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org>
To: Theodore Ts'o <tytso@mit.edu>
Cc: "Darrick J . Wong" <darrick.wong@oracle.com>,
	linux-api@vger.kernel.org, Dave Chinner <david@fromorbit.com>,
	linux-f2fs-devel@lists.sourceforge.net,
	linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Jaegeuk Kim <jaegeuk@kernel.org>,
	linux-integrity@vger.kernel.org, linux-ext4@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Christoph Hellwig <hch@lst.de>,
	Victor Hsieh <victorhsieh@google.com>
Subject: Re: [PATCH v4 01/16] fs-verity: add a documentation file
Date: Tue, 18 Jun 2019 09:31:18 -0700	[thread overview]
Message-ID: <20190618163116.GA184520@gmail.com> (raw)
In-Reply-To: <20190615123920.GB6142@mit.edu>

Hi Ted,

On Sat, Jun 15, 2019 at 08:39:20AM -0400, Theodore Ts'o wrote:
> On Thu, Jun 06, 2019 at 08:51:50AM -0700, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > Add a documentation file for fs-verity, covering....
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> 
> Looks good; you can add:
> 
> Reviewed-by: Theodore Ts'o <tytso@mit.edu>
> 
> 
> One minor design point below:
> 
> > +ext4 stores the verity metadata (Merkle tree and fsverity_descriptor)
> > +past the end of the file, starting at the first page fully beyond
>                                                    ^^^^
> > +i_size.  This approach works because (a) verity files are readonly,
> > +and (b) pages fully beyond i_size aren't visible to userspace but can
> > +be read/written internally by ext4 with only some relatively small
> > +changes to ext4.  This approach avoids having to depend on the
> > +EA_INODE feature and on rearchitecturing ext4's xattr support to
> > +support paging multi-gigabyte xattrs into memory, and to support
> > +encrypting xattrs.  Note that the verity metadata *must* be encrypted
> > +when the file is, since it contains hashes of the plaintext data.
> 
> If we ever want to support mounting, say, a file system with 4k blocks
> and fsverity enabled on a architecture with a 16k or 64k page size,
> then "page" in that first sentence will need to become "block".  At
> the moment we only support fsverity when page size == block size, so
> it's not an issue.
> 
> However, it's worth reflecting on what this means.  In order to
> satisfy this requirement (from the mmap man page):
> 
>        A file is mapped in multiples of the page size.  For a file
>        that is not a multiple of the page size, the remaining memory
>        is zeroed when mapped...
> 
> we're going to have to special case how the last page gets mmaped.
> The simplest way to do this will be to map in an anonymous page which
> just has the blocks that are part of the data block copied in, and the
> rest of the page can be zero'ed.
> 
> One thing we might consider doing just to make life much easier for
> ourselves (should we ever want to support page size != block size ---
> which I could imagine some folks like Chandan might find desirable) is
> to specify that the fsverity metadata begins at an offset which begins
> at i_size rounded up to the next 64k binary, which should handle all
> current and future architectures' page sizes.
> 

Thanks for the review.  Good point; I think we should just go with the "always
round up to the next 64K boundary" method.  Special-casing how the last page
gets mmap()ed seems it would be really painful.

Since there can be a hole between the end of the file and the start of the
verity metadata, this doesn't even necessarily use any additional disk space.

For consistency and since there is little downside I think I'll do the same for
f2fs too, though f2fs doesn't currently support PAGE_SIZE != 4096 at all anyway.

- Eric

WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org>
To: Theodore Ts'o <tytso@mit.edu>
Cc: "Darrick J . Wong" <darrick.wong@oracle.com>,
	linux-api@vger.kernel.org, Dave Chinner <david@fromorbit.com>,
	linux-f2fs-devel@lists.sourceforge.net,
	linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Jaegeuk Kim <jaegeuk@kernel.org>,
	linux-integrity@vger.kernel.org, linux-ext4@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Christoph Hellwig <hch@lst.de>,
	Victor Hsieh <victorhsieh@google.com>
Subject: Re: [f2fs-dev] [PATCH v4 01/16] fs-verity: add a documentation file
Date: Tue, 18 Jun 2019 09:31:18 -0700	[thread overview]
Message-ID: <20190618163116.GA184520@gmail.com> (raw)
In-Reply-To: <20190615123920.GB6142@mit.edu>

Hi Ted,

On Sat, Jun 15, 2019 at 08:39:20AM -0400, Theodore Ts'o wrote:
> On Thu, Jun 06, 2019 at 08:51:50AM -0700, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > Add a documentation file for fs-verity, covering....
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> 
> Looks good; you can add:
> 
> Reviewed-by: Theodore Ts'o <tytso@mit.edu>
> 
> 
> One minor design point below:
> 
> > +ext4 stores the verity metadata (Merkle tree and fsverity_descriptor)
> > +past the end of the file, starting at the first page fully beyond
>                                                    ^^^^
> > +i_size.  This approach works because (a) verity files are readonly,
> > +and (b) pages fully beyond i_size aren't visible to userspace but can
> > +be read/written internally by ext4 with only some relatively small
> > +changes to ext4.  This approach avoids having to depend on the
> > +EA_INODE feature and on rearchitecturing ext4's xattr support to
> > +support paging multi-gigabyte xattrs into memory, and to support
> > +encrypting xattrs.  Note that the verity metadata *must* be encrypted
> > +when the file is, since it contains hashes of the plaintext data.
> 
> If we ever want to support mounting, say, a file system with 4k blocks
> and fsverity enabled on a architecture with a 16k or 64k page size,
> then "page" in that first sentence will need to become "block".  At
> the moment we only support fsverity when page size == block size, so
> it's not an issue.
> 
> However, it's worth reflecting on what this means.  In order to
> satisfy this requirement (from the mmap man page):
> 
>        A file is mapped in multiples of the page size.  For a file
>        that is not a multiple of the page size, the remaining memory
>        is zeroed when mapped...
> 
> we're going to have to special case how the last page gets mmaped.
> The simplest way to do this will be to map in an anonymous page which
> just has the blocks that are part of the data block copied in, and the
> rest of the page can be zero'ed.
> 
> One thing we might consider doing just to make life much easier for
> ourselves (should we ever want to support page size != block size ---
> which I could imagine some folks like Chandan might find desirable) is
> to specify that the fsverity metadata begins at an offset which begins
> at i_size rounded up to the next 64k binary, which should handle all
> current and future architectures' page sizes.
> 

Thanks for the review.  Good point; I think we should just go with the "always
round up to the next 64K boundary" method.  Special-casing how the last page
gets mmap()ed seems it would be really painful.

Since there can be a hole between the end of the file and the start of the
verity metadata, this doesn't even necessarily use any additional disk space.

For consistency and since there is little downside I think I'll do the same for
f2fs too, though f2fs doesn't currently support PAGE_SIZE != 4096 at all anyway.

- Eric


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2019-06-18 16:31 UTC|newest]

Thread overview: 126+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-06 15:51 [PATCH v4 00/16] fs-verity: read-only file-based authenticity protection Eric Biggers
2019-06-06 15:51 ` [f2fs-dev] " Eric Biggers
2019-06-06 15:51 ` Eric Biggers
2019-06-06 15:51 ` [PATCH v4 01/16] fs-verity: add a documentation file Eric Biggers
2019-06-06 15:51   ` [f2fs-dev] " Eric Biggers
2019-06-06 15:51   ` Eric Biggers
2019-06-15 12:39   ` Theodore Ts'o
2019-06-15 12:39     ` [f2fs-dev] " Theodore Ts'o
2019-06-15 12:39     ` Theodore Ts'o
2019-06-18 16:31     ` Eric Biggers [this message]
2019-06-18 16:31       ` [f2fs-dev] " Eric Biggers
2019-06-18 16:31       ` Eric Biggers
2019-06-06 15:51 ` [f2fs-dev] [PATCH v4 02/16] fs-verity: add MAINTAINERS file entry Eric Biggers
2019-06-06 15:51   ` Eric Biggers
2019-06-06 15:51   ` Eric Biggers
2019-06-15 12:39   ` Theodore Ts'o
2019-06-15 12:39     ` [f2fs-dev] " Theodore Ts'o
2019-06-15 12:39     ` Theodore Ts'o
2019-06-06 15:51 ` [f2fs-dev] [PATCH v4 03/16] fs-verity: add UAPI header Eric Biggers
2019-06-06 15:51   ` Eric Biggers
2019-06-06 15:51   ` Eric Biggers
2019-06-06 15:51 ` [f2fs-dev] [PATCH v4 04/16] fs: uapi: define verity bit for FS_IOC_GETFLAGS Eric Biggers
2019-06-06 15:51   ` Eric Biggers
2019-06-06 15:51   ` Eric Biggers
2019-06-15 12:40   ` Theodore Ts'o
2019-06-15 12:40     ` [f2fs-dev] " Theodore Ts'o
2019-06-15 12:40     ` Theodore Ts'o
2019-06-06 15:51 ` [PATCH v4 05/16] fs-verity: add Kconfig and the helper functions for hashing Eric Biggers
2019-06-06 15:51   ` [f2fs-dev] " Eric Biggers
2019-06-06 15:51   ` Eric Biggers
2019-06-15 12:57   ` Theodore Ts'o
2019-06-15 12:57     ` [f2fs-dev] " Theodore Ts'o
2019-06-15 12:57     ` Theodore Ts'o
2019-06-18 16:32     ` Eric Biggers
2019-06-18 16:32       ` [f2fs-dev] " Eric Biggers
2019-06-18 16:32       ` Eric Biggers
2019-06-06 15:51 ` [PATCH v4 06/16] fs-verity: add inode and superblock fields Eric Biggers
2019-06-06 15:51   ` [f2fs-dev] " Eric Biggers
2019-06-06 15:51   ` Eric Biggers
2019-06-15 12:57   ` Theodore Ts'o
2019-06-15 12:57     ` [f2fs-dev] " Theodore Ts'o
2019-06-15 12:57     ` Theodore Ts'o
2019-06-06 15:51 ` [PATCH v4 07/16] fs-verity: add the hook for file ->open() Eric Biggers
2019-06-06 15:51   ` [f2fs-dev] " Eric Biggers
2019-06-06 15:51   ` Eric Biggers
2019-06-15 14:42   ` Theodore Ts'o
2019-06-15 14:42     ` [f2fs-dev] " Theodore Ts'o
2019-06-15 14:42     ` Theodore Ts'o
2019-06-18 16:35     ` Eric Biggers
2019-06-18 16:35       ` [f2fs-dev] " Eric Biggers
2019-06-18 16:35       ` Eric Biggers
2019-06-06 15:51 ` [PATCH v4 08/16] fs-verity: add the hook for file ->setattr() Eric Biggers
2019-06-06 15:51   ` [f2fs-dev] " Eric Biggers
2019-06-06 15:51   ` Eric Biggers
2019-06-15 14:43   ` Theodore Ts'o
2019-06-15 14:43     ` [f2fs-dev] " Theodore Ts'o
2019-06-15 14:43     ` Theodore Ts'o
2019-06-06 15:51 ` [PATCH v4 09/16] fs-verity: add data verification hooks for ->readpages() Eric Biggers
2019-06-06 15:51   ` [f2fs-dev] " Eric Biggers
2019-06-06 15:51   ` Eric Biggers
2019-06-15 14:48   ` Theodore Ts'o
2019-06-15 14:48     ` [f2fs-dev] " Theodore Ts'o
2019-06-15 14:48     ` Theodore Ts'o
2019-06-06 15:51 ` [PATCH v4 10/16] fs-verity: implement FS_IOC_ENABLE_VERITY ioctl Eric Biggers
2019-06-06 15:51   ` [f2fs-dev] " Eric Biggers
2019-06-06 15:51   ` Eric Biggers
2019-06-15 15:08   ` Theodore Ts'o
2019-06-15 15:08     ` [f2fs-dev] " Theodore Ts'o
2019-06-15 15:08     ` Theodore Ts'o
2019-06-18 16:50     ` Eric Biggers
2019-06-18 16:50       ` [f2fs-dev] " Eric Biggers
2019-06-18 16:50       ` Eric Biggers
2019-06-06 15:52 ` [PATCH v4 11/16] fs-verity: implement FS_IOC_MEASURE_VERITY ioctl Eric Biggers
2019-06-06 15:52   ` [f2fs-dev] " Eric Biggers
2019-06-06 15:52   ` Eric Biggers
2019-06-15 15:10   ` Theodore Ts'o
2019-06-15 15:10     ` [f2fs-dev] " Theodore Ts'o
2019-06-15 15:10     ` Theodore Ts'o
2019-06-06 15:52 ` [PATCH v4 12/16] fs-verity: add SHA-512 support Eric Biggers
2019-06-06 15:52   ` [f2fs-dev] " Eric Biggers
2019-06-06 15:52   ` Eric Biggers
2019-06-15 15:11   ` Theodore Ts'o
2019-06-15 15:11     ` [f2fs-dev] " Theodore Ts'o
2019-06-15 15:11     ` Theodore Ts'o
2019-06-06 15:52 ` [PATCH v4 13/16] fs-verity: support builtin file signatures Eric Biggers
2019-06-06 15:52   ` [f2fs-dev] " Eric Biggers
2019-06-06 15:52   ` Eric Biggers
2019-06-15 15:21   ` Theodore Ts'o
2019-06-15 15:21     ` [f2fs-dev] " Theodore Ts'o
2019-06-15 15:21     ` Theodore Ts'o
2019-06-18 16:58     ` Eric Biggers
2019-06-18 16:58       ` [f2fs-dev] " Eric Biggers
2019-06-18 16:58       ` Eric Biggers
2019-06-06 15:52 ` [PATCH v4 14/16] ext4: add basic fs-verity support Eric Biggers
2019-06-06 15:52   ` [f2fs-dev] " Eric Biggers
2019-06-06 15:52   ` Eric Biggers
2019-06-15 15:31   ` Theodore Ts'o
2019-06-15 15:31     ` [f2fs-dev] " Theodore Ts'o
2019-06-15 15:31     ` Theodore Ts'o
2019-06-18 17:51     ` Eric Biggers
2019-06-18 17:51       ` [f2fs-dev] " Eric Biggers
2019-06-18 17:51       ` Eric Biggers
2019-06-18 22:46       ` Theodore Ts'o
2019-06-18 22:46         ` [f2fs-dev] " Theodore Ts'o
2019-06-18 22:46         ` Theodore Ts'o
2019-06-18 23:41         ` Eric Biggers
2019-06-18 23:41           ` [f2fs-dev] " Eric Biggers
2019-06-18 23:41           ` Eric Biggers
2019-06-19  3:05           ` Theodore Ts'o
2019-06-19  3:05             ` [f2fs-dev] " Theodore Ts'o
2019-06-19  3:05             ` Theodore Ts'o
2019-06-19 19:13             ` Eric Biggers
2019-06-19 19:13               ` [f2fs-dev] " Eric Biggers
2019-06-19 19:13               ` Eric Biggers
2019-06-06 15:52 ` [PATCH v4 15/16] ext4: add fs-verity read support Eric Biggers
2019-06-06 15:52   ` [f2fs-dev] " Eric Biggers
2019-06-06 15:52   ` Eric Biggers
2019-06-06 15:52 ` [PATCH v4 16/16] f2fs: add fs-verity support Eric Biggers
2019-06-06 15:52   ` [f2fs-dev] " Eric Biggers
2019-06-06 15:52   ` Eric Biggers
2019-06-06 17:21 ` [PATCH v4 00/16] fs-verity: read-only file-based authenticity protection Linus Torvalds
2019-06-06 17:21   ` [f2fs-dev] " Linus Torvalds
2019-06-06 17:21   ` Linus Torvalds
2019-06-06 19:43   ` Eric Biggers
2019-06-06 19:43     ` [f2fs-dev] " Eric Biggers
2019-06-06 19:43     ` Eric Biggers

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=20190618163116.GA184520@gmail.com \
    --to=ebiggers@kernel.org \
    --cc=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=hch@lst.de \
    --cc=jaegeuk@kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=victorhsieh@google.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.