Re: [f2fs-dev] [PATCH V2 10/13] fscrypt_encrypt_page: Loop across all blocks mapped by a page range
On Thursday, May 2, 2019 3:59:01 AM IST Eric Biggers wrote:
> Hi Chandan,
>
> On Wed, May 01, 2019 at 08:19:35PM +0530, Chandan Rajendra wrote:
> > On Wednesday, May 1, 2019 4:38:41 AM IST Eric Biggers wrote:
> > > On Tue, Apr 30, 2019 at 10:11:35AM -0700, Eric Biggers wrote:
> > > > On Sun, Apr 28, 2019 at 10:01:18AM +0530, Chandan Rajendra wrote:
> > > > > For subpage-sized blocks, this commit now encrypts all blocks mapped
> > > > > by
> > > > > a page range.
> > > > >
> > > > > Signed-off-by: Chandan Rajendra
> > > > > ---
> > > > > fs/crypto/crypto.c | 37 +
> > > > > 1 file changed, 25 insertions(+), 12 deletions(-)
> > > > >
> > > > > diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> > > > > index 4f0d832cae71..2d65b431563f 100644
> > > > > --- a/fs/crypto/crypto.c
> > > > > +++ b/fs/crypto/crypto.c
> > > > > @@ -242,18 +242,26 @@ struct page *fscrypt_encrypt_page(const struct
> > > > > inode *inode,
> > > >
> > > > Need to update the function comment to clearly explain what this
> > > > function
> > > > actually does now.
> > > >
> > > > > {
> > > > > struct fscrypt_ctx *ctx;
> > > > > struct page *ciphertext_page = page;
> > > > > + int i, page_nr_blks;
> > > > > int err;
> > > > >
> > > > > BUG_ON(len % FS_CRYPTO_BLOCK_SIZE != 0);
> > > > >
> > > >
> > > > Make a 'blocksize' variable so you don't have to keep calling
> > > > i_blocksize().
> > > >
> > > > Also, you need to check whether 'len' and 'offs' are
> > > > filesystem-block-aligned,
> > > > since the code now assumes it.
> > > >
> > > > const unsigned int blocksize = i_blocksize(inode);
> > > >
> > > > if (!IS_ALIGNED(len | offs, blocksize))
> > > > return -EINVAL;
> > > >
> > > > However, did you check whether that's always true for ubifs? It looks
> > > > like it
> > > > may expect to encrypt a prefix of a block, that is only padded to the
> > > > next
> > > > 16-byte boundary.
> > > >
> > > > > + page_nr_blks = len >> inode->i_blkbits;
> > > > > +
> > > > > if (inode->i_sb->s_cop->flags & FS_CFLG_OWN_PAGES) {
> > > > > /* with inplace-encryption we just encrypt the page */
> > > > > - err = fscrypt_do_page_crypto(inode, FS_ENCRYPT,
> > > > > lblk_num, page,
> > > > > - ciphertext_page, len, offs,
> > > > > - gfp_flags);
> > > > > - if (err)
> > > > > - return ERR_PTR(err);
> > > > > -
> > > > > + for (i = 0; i < page_nr_blks; i++) {
> > > > > + err = fscrypt_do_page_crypto(inode, FS_ENCRYPT,
> > > > > + lblk_num, page,
> > > > > + ciphertext_page,
> > > > > + i_blocksize(inode),
> > > > > offs,
> > > > > + gfp_flags);
> > > > > + if (err)
> > > > > + return ERR_PTR(err);
> > >
> > > Apparently ubifs does encrypt data shorter than the filesystem block
> > > size, so
> > > this part is wrong.
> > >
> > > I suggest we split this into two functions,
> > > fscrypt_encrypt_block_inplace() and
> > > fscrypt_encrypt_blocks(), so that it's conceptually simpler what each
> > > function
> > > does. Currently this works completely differently depending on whether
> > > the
> > > filesystem set FS_CFLG_OWN_PAGES in its fscrypt_operations, which is
> > > weird.
> > >
> > > I also noticed that using fscrypt_ctx for writes seems to be unnecessary.
> > > AFAICS, page_private(bounce_page) could point directly to the pagecache
> > > page.
> > > That would simplify things a lot, especially since then fscrypt_ctx could
> > > be
> > > removed entirely after you convert reads to use read_callbacks_ctx.
> > >
> > > IMO, these would be worthwhile cleanups for fscrypt by themselves, without
> > > waiting for the read_callbacks stuff to be finalized. Finalizing the
> > > read_callbacks stuff will probably require reaching a consensus about how
> > > they
> > > should work with future filesystem features like fsverity and compression.
> > >
> > > So to move things forward, I'm considering sending out a series with the
> > > above
> > > cleanups for fscrypt, plus the equivalent of your patches:
> > >
> > > "fscrypt_encrypt_page: Loop across all blocks mapped by a page range"
> > > "fscrypt_zeroout_range: Encrypt all zeroed out blocks of a page"
> > > "Add decryption support for sub-pagesized blocks" (fs/crypto/ part only)
> > >
> > > Then hopefully we can get all that applied for 5.3 so that fs/crypto/
> > > itself is
> > > ready for blocksize != PAGE_SIZE; and get your changes to
> > > ext4_bio_write_page(),
> > > __ext4_block_zero_page_range(), and ext4_block_write_begin() applied too,
> > > so
[f2fs-dev] [PATCH v2] f2fs: Add option to limit required GC for checkpoint=disable
This extends the checkpoint option to allow checkpoint=disable:%u
This allows you to specify what percent of the drive you are willing
to lose access to while mounting with checkpoint=disable. If the amount
lost would be higher, the mount will return -EAGAIN.
Currently, we need to run garbage collection until the amount of holes
is smaller than the OVP space. With the new option, f2fs can mark
space as unusable up front instead of requiring that
the space be freed up with garbage collection.
Change-Id: Ib8c3cb46085bf4aac3cdc06d4b64d6a20db87028
Signed-off-by: Daniel Rosenberg
---
> If we allow a high unusable blocks, I doubt in inc_valid_{node,block}_count(),
> we may overflow in below calculation:
>
> if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED)))
> valid_block_count += sbi->unusable_block_count;
>
> if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED)))
> avail_user_block_count -= sbi->unusable_block_count;
The starting value of unusable_block_count is limited.
Considering the case where the disk is full, we have
total blocks = used + OVP
OVP = reserved + holes
So, the holes must be less than OVP, and unusable = 0
In general, total = used + available + OVP
total-used-OVP=avail
(free being blocks from free segments,
available being blocks available to the user)
reserved + holes + free = OVP + available
holes - OVP = available - reserved - free
unusable = max(data_holes, node_holes)-OVP
unusable <= holes-OVP
unusable <= available-reserved - free segs
unusable <= available
similarly,
total = used + available + OVP
reserved + holes + free = OVP + available
available = reserved + holes + free - OVP
total = used + reserved + holes + free - OVP
unusable<=holes-OVP
total >= used + reserved + unusable + free
total >= used + unusable
So the unusable block count is lower than the available block count when we
first mount with checkpoint=disable, and we only grow unusable when we grow
available blocks. We also have an upper bound on used+unusable which is less
the total block count. So unless I've missed something elsewhere, we should
not be able to underflow or overflow these values.
v2:
Changed default to 0 to be in line with current behavior.
Adjusted description of checkpoint=disable to be more clear, since with that
default, mounting with checkpoint=disable may return EAGAIN unless you use
checkpoint=disable:100
Changed description of /sys/fs/f2fs//unusable to be clearer
Documentation/ABI/testing/sysfs-fs-f2fs | 8 +
Documentation/filesystems/f2fs.txt | 12 ++-
fs/f2fs/f2fs.h | 6 +++-
fs/f2fs/segment.c | 17 --
fs/f2fs/super.c | 44 +
fs/f2fs/sysfs.c | 16 +
6 files changed, 77 insertions(+), 26 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs
b/Documentation/ABI/testing/sysfs-fs-f2fs
index 91822ce258317..dca326e0ee3e1 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -243,3 +243,11 @@ Description:
- Del: echo '[h/c]!extension' >
/sys/fs/f2fs//extension_list
- [h] means add/del hot file extension
- [c] means add/del cold file extension
+
+What: /sys/fs/f2fs//unusable
+Date April 2019
+Contact: "Daniel Rosenberg"
+Description:
+ If checkpoint=disable, it displays the number of blocks that
are unusable.
+If checkpoint=enable it displays the enumber of blocks that
would be unusable
+if checkpoint=disable were to be set.
diff --git a/Documentation/filesystems/f2fs.txt
b/Documentation/filesystems/f2fs.txt
index f7b5e4ff0de3e..d49f5ac8bf9b6 100644
--- a/Documentation/filesystems/f2fs.txt
+++ b/Documentation/filesystems/f2fs.txt
@@ -214,11 +214,21 @@ fsync_mode=%s Control the policy of fsync.
Currently supports "posix",
non-atomic files likewise "nobarrier" mount option.
test_dummy_encryption Enable dummy encryption, which provides a fake fscrypt
context. The fake fscrypt context is used by xfstests.
-checkpoint=%s Set to "disable" to turn off checkpointing. Set to
"enable"
+checkpoint=%s[:%u] Set to "disable" to turn off checkpointing. Set to
"enable"
to reenable checkpointing. Is enabled by default. While
disabled, any unmounting or unexpected shutdowns will
cause
the filesystem contents to appear as they did when the
filesystem was mounted with that option.
+ While mounting with checkpoint=disabled, the filesystem
must
+ run garbage collection to ensure that all available
space can
+ be used. If this takes too much time, the mount may
return
+
[f2fs-dev] [PATCH 09/13] fscrypt: support decrypting multiple filesystem blocks per page
From: Eric Biggers
Rename fscrypt_decrypt_page() to fscrypt_decrypt_pagecache_blocks() and
redefine its behavior to decrypt all filesystem blocks in the given
region of the given page, rather than assuming that the region consists
of just one filesystem block. Also remove the 'inode' and 'lblk_num'
parameters, since they can be retrieved from the page as it's already
assumed to be a pagecache page.
This is in preparation for allowing encryption on ext4 filesystems with
blocksize != PAGE_SIZE.
This is based on work by Chandan Rajendra.
Signed-off-by: Eric Biggers
---
fs/crypto/bio.c | 5 ++---
fs/crypto/crypto.c | 46 -
fs/ext4/inode.c | 7 +++
include/linux/fscrypt.h | 12 +--
4 files changed, 43 insertions(+), 27 deletions(-)
diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index e67e9d4d342b3..b4f47b98ee6d9 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -34,9 +34,8 @@ static void __fscrypt_decrypt_bio(struct bio *bio, bool done)
bio_for_each_segment_all(bv, bio, i, iter_all) {
struct page *page = bv->bv_page;
- int ret = fscrypt_decrypt_page(page->mapping->host, page,
- PAGE_SIZE, 0, page->index);
-
+ int ret = fscrypt_decrypt_pagecache_blocks(page, bv->bv_len,
+ bv->bv_offset);
if (ret)
SetPageError(page);
else if (done)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 2e6fb5e4f7a7f..dcf630d7e4460 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -282,29 +282,47 @@ int fscrypt_encrypt_block_inplace(const struct inode
*inode, struct page *page,
EXPORT_SYMBOL(fscrypt_encrypt_block_inplace);
/**
- * fscrypt_decrypt_page() - Decrypts a page in-place
- * @inode: The corresponding inode for the page to decrypt.
- * @page: The page to decrypt. Must be locked.
- * @len: Number of bytes in @page to be decrypted.
- * @offs: Start of data in @page.
- * @lblk_num: Logical block number.
+ * fscrypt_decrypt_pagecache_blocks() - Decrypt filesystem blocks in a
pagecache page
+ * @page: The locked pagecache page containing the block(s) to decrypt
+ * @len: Total size of the block(s) to decrypt. Must be a nonzero
+ * multiple of the filesystem's block size.
+ * @offs: Byte offset within @page of the first block to decrypt. Must be
+ * a multiple of the filesystem's block size.
*
- * Decrypts page in-place using the ctx encryption context.
+ * The specified block(s) are decrypted in-place within the pagecache page,
+ * which must still be locked and not uptodate. Normally, blocksize ==
+ * PAGE_SIZE and the whole page is decrypted at once.
*
- * Called from the read completion callback.
+ * This is for use by the filesystem's ->readpages() method.
*
- * Return: Zero on success, non-zero otherwise.
+ * Return: 0 on success; -errno on failure
*/
-int fscrypt_decrypt_page(const struct inode *inode, struct page *page,
- unsigned int len, unsigned int offs, u64 lblk_num)
+int fscrypt_decrypt_pagecache_blocks(struct page *page, unsigned int len,
+unsigned int offs)
{
+ const struct inode *inode = page->mapping->host;
+ const unsigned int blockbits = inode->i_blkbits;
+ const unsigned int blocksize = 1 << blockbits;
+ u64 lblk_num = ((u64)page->index << (PAGE_SHIFT - blockbits)) +
+ (offs >> blockbits);
+ unsigned int i;
+ int err;
+
if (WARN_ON_ONCE(!PageLocked(page)))
return -EINVAL;
- return fscrypt_crypt_block(inode, FS_DECRYPT, lblk_num, page, page,
- len, offs, GFP_NOFS);
+ if (WARN_ON_ONCE(len <= 0 || !IS_ALIGNED(len | offs, blocksize)))
+ return -EINVAL;
+
+ for (i = offs; i < offs + len; i += blocksize, lblk_num++) {
+ err = fscrypt_crypt_block(inode, FS_DECRYPT, lblk_num, page,
+ page, blocksize, i, GFP_NOFS);
+ if (err)
+ return err;
+ }
+ return 0;
}
-EXPORT_SYMBOL(fscrypt_decrypt_page);
+EXPORT_SYMBOL(fscrypt_decrypt_pagecache_blocks);
/**
* fscrypt_decrypt_block_inplace() - Decrypt a filesystem block in-place
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index b32a57bc5d5d6..1ef5d791834fc 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1228,8 +1228,7 @@ static int ext4_block_write_begin(struct page *page,
loff_t pos, unsigned len,
if (unlikely(err))
page_zero_new_buffers(page, from, to);
else if (decrypt)
- err = fscrypt_decrypt_page(page->mapping->host, page,
- PAGE_SIZE, 0, page->index);
+ err =
[f2fs-dev] [PATCH 06/13] fscrypt: support encrypting multiple filesystem blocks per page
From: Eric Biggers
Rename fscrypt_encrypt_page() to fscrypt_encrypt_pagecache_blocks() and
redefine its behavior to encrypt all filesystem blocks from the given
region of the given page, rather than assuming that the region consists
of just one filesystem block. Also remove the 'inode' and 'lblk_num'
parameters, since they can be retrieved from the page as it's already
assumed to be a pagecache page.
This is in preparation for allowing encryption on ext4 filesystems with
blocksize != PAGE_SIZE.
This is based on work by Chandan Rajendra.
Signed-off-by: Eric Biggers
---
fs/crypto/crypto.c | 67 -
fs/ext4/page-io.c | 4 +--
fs/f2fs/data.c | 5 +--
include/linux/fscrypt.h | 17 ++-
4 files changed, 53 insertions(+), 40 deletions(-)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index e978541e2ec19..7bdb985126d97 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -121,8 +121,8 @@ struct page *fscrypt_alloc_bounce_page(gfp_t gfp_flags)
/**
* fscrypt_free_bounce_page() - free a ciphertext bounce page
*
- * Free a bounce page that was allocated by fscrypt_encrypt_page(), or by
- * fscrypt_alloc_bounce_page() directly.
+ * Free a bounce page that was allocated by fscrypt_encrypt_pagecache_blocks(),
+ * or by fscrypt_alloc_bounce_page() directly.
*/
void fscrypt_free_bounce_page(struct page *bounce_page)
{
@@ -197,52 +197,63 @@ int fscrypt_crypt_block(const struct inode *inode,
fscrypt_direction_t rw,
}
/**
- * fscypt_encrypt_page() - Encrypts a page
- * @inode: The inode for which the encryption should take place
- * @page: The page to encrypt. Must be locked.
- * @len: Length of data to encrypt in @page and encrypted
- * data in returned page.
- * @offs: Offset of data within @page and returned
- * page holding encrypted data.
- * @lblk_num: Logical block number. This must be unique for multiple
- * calls with same inode, except when overwriting
- * previously written data.
- * @gfp_flags: The gfp flag for memory allocation
+ * fscrypt_encrypt_pagecache_blocks() - Encrypt filesystem blocks from a
pagecache page
+ * @page: The locked pagecache page containing the block(s) to encrypt
+ * @len: Total size of the block(s) to encrypt. Must be a nonzero
+ * multiple of the filesystem's block size.
+ * @offs: Byte offset within @page of the first block to encrypt. Must be
+ * a multiple of the filesystem's block size.
+ * @gfp_flags: Memory allocation flags
+ *
+ * A new bounce page is allocated, and the specified block(s) are encrypted
into
+ * it. In the bounce page, the ciphertext block(s) will be located at the same
+ * offsets at which the plaintext block(s) were located in the source page; any
+ * other parts of the bounce page will be left uninitialized. However,
normally
+ * blocksize == PAGE_SIZE and the whole page is encrypted at once.
*
- * Encrypts @page. A bounce page is allocated, the data is encrypted into the
- * bounce page, and the bounce page is returned. The caller is responsible for
- * calling fscrypt_free_bounce_page().
+ * This is for use by the filesystem's ->writepages() method.
*
- * Return: A page containing the encrypted data on success; else an ERR_PTR()
+ * Return: the new encrypted bounce page on success; an ERR_PTR() on failure
*/
-struct page *fscrypt_encrypt_page(const struct inode *inode,
- struct page *page,
- unsigned int len,
- unsigned int offs,
- u64 lblk_num, gfp_t gfp_flags)
+struct page *fscrypt_encrypt_pagecache_blocks(struct page *page,
+ unsigned int len,
+ unsigned int offs,
+ gfp_t gfp_flags)
{
+ const struct inode *inode = page->mapping->host;
+ const unsigned int blockbits = inode->i_blkbits;
+ const unsigned int blocksize = 1 << blockbits;
struct page *ciphertext_page;
+ u64 lblk_num = ((u64)page->index << (PAGE_SHIFT - blockbits)) +
+ (offs >> blockbits);
+ unsigned int i;
int err;
if (WARN_ON_ONCE(!PageLocked(page)))
return ERR_PTR(-EINVAL);
+ if (WARN_ON_ONCE(len <= 0 || !IS_ALIGNED(len | offs, blocksize)))
+ return ERR_PTR(-EINVAL);
+
ciphertext_page = fscrypt_alloc_bounce_page(gfp_flags);
if (!ciphertext_page)
return ERR_PTR(-ENOMEM);
- err = fscrypt_crypt_block(inode, FS_ENCRYPT, lblk_num, page,
- ciphertext_page, len, offs, gfp_flags);
- if (err) {
- fscrypt_free_bounce_page(ciphertext_page);
- return ERR_PTR(err);
+ for (i = offs; i < offs + len; i +=
[f2fs-dev] [PATCH 02/13] fscrypt: remove the "write" part of struct fscrypt_ctx
From: Eric Biggers
Now that fscrypt_ctx is not used for writes, remove the 'w' fields.
Signed-off-by: Eric Biggers
---
fs/crypto/bio.c | 11 +--
fs/crypto/crypto.c | 14 +++---
include/linux/fscrypt.h | 7 ++-
3 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index 9107e8fe55897..548ec6bb569cf 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -54,9 +54,8 @@ EXPORT_SYMBOL(fscrypt_decrypt_bio);
static void completion_pages(struct work_struct *work)
{
- struct fscrypt_ctx *ctx =
- container_of(work, struct fscrypt_ctx, r.work);
- struct bio *bio = ctx->r.bio;
+ struct fscrypt_ctx *ctx = container_of(work, struct fscrypt_ctx, work);
+ struct bio *bio = ctx->bio;
__fscrypt_decrypt_bio(bio, true);
fscrypt_release_ctx(ctx);
@@ -65,9 +64,9 @@ static void completion_pages(struct work_struct *work)
void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx, struct bio *bio)
{
- INIT_WORK(>r.work, completion_pages);
- ctx->r.bio = bio;
- fscrypt_enqueue_decrypt_work(>r.work);
+ INIT_WORK(>work, completion_pages);
+ ctx->bio = bio;
+ fscrypt_enqueue_decrypt_work(>work);
}
EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 4b076f8daab75..ebfa13cfecb7d 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -58,11 +58,11 @@ void fscrypt_enqueue_decrypt_work(struct work_struct *work)
EXPORT_SYMBOL(fscrypt_enqueue_decrypt_work);
/**
- * fscrypt_release_ctx() - Releases an encryption context
- * @ctx: The encryption context to release.
+ * fscrypt_release_ctx() - Release a decryption context
+ * @ctx: The decryption context to release.
*
- * If the encryption context was allocated from the pre-allocated pool, returns
- * it to that pool. Else, frees it.
+ * If the decryption context was allocated from the pre-allocated pool, return
+ * it to that pool. Else, free it.
*/
void fscrypt_release_ctx(struct fscrypt_ctx *ctx)
{
@@ -79,12 +79,12 @@ void fscrypt_release_ctx(struct fscrypt_ctx *ctx)
EXPORT_SYMBOL(fscrypt_release_ctx);
/**
- * fscrypt_get_ctx() - Gets an encryption context
+ * fscrypt_get_ctx() - Get a decryption context
* @gfp_flags: The gfp flag for memory allocation
*
- * Allocates and initializes an encryption context.
+ * Allocate and initialize a decryption context.
*
- * Return: A new encryption context on success; an ERR_PTR() otherwise.
+ * Return: A new decryption context on success; an ERR_PTR() otherwise.
*/
struct fscrypt_ctx *fscrypt_get_ctx(gfp_t gfp_flags)
{
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 77837e72e4add..11e7187fa2e52 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -63,16 +63,13 @@ struct fscrypt_operations {
unsigned int max_namelen;
};
+/* Decryption work */
struct fscrypt_ctx {
union {
- struct {
- struct page *bounce_page; /* Ciphertext page */
- struct page *control_page; /* Original page */
- } w;
struct {
struct bio *bio;
struct work_struct work;
- } r;
+ };
struct list_head free_list; /* Free list */
};
u8 flags; /* Flags */
--
2.21.0.593.g511ec345e18-goog
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 10/13] ext4: clear BH_Uptodate flag on decryption error
From: Chandan Rajendra
If decryption fails, ext4_block_write_begin() can return with the page's
buffer_head marked with the BH_Uptodate flag. This commit clears the
BH_Uptodate flag in such cases.
Signed-off-by: Chandan Rajendra
Signed-off-by: Eric Biggers
---
fs/ext4/inode.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 1ef5d791834fc..9382e1bcefe49 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1225,10 +1225,14 @@ static int ext4_block_write_begin(struct page *page,
loff_t pos, unsigned len,
if (!buffer_uptodate(*wait_bh))
err = -EIO;
}
- if (unlikely(err))
+ if (unlikely(err)) {
page_zero_new_buffers(page, from, to);
- else if (decrypt)
+ } else if (decrypt) {
err = fscrypt_decrypt_pagecache_blocks(page, PAGE_SIZE, 0);
+ if (err)
+ clear_buffer_uptodate(*wait_bh);
+ }
+
return err;
}
#endif
--
2.21.0.593.g511ec345e18-goog
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 13/13] ext4: encrypt only up to last block in ext4_bio_write_page()
From: Eric Biggers
As an optimization, don't encrypt blocks fully beyond i_size, since
those definitely won't need to be written out. Also add a comment.
This is in preparation for allowing encryption on ext4 filesystems with
blocksize != PAGE_SIZE.
This is based on work by Chandan Rajendra.
Signed-off-by: Eric Biggers
---
fs/ext4/page-io.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 457ddf051608f..ab843ad89df2f 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -468,11 +468,19 @@ int ext4_bio_write_page(struct ext4_io_submit *io,
bh = head = page_buffers(page);
+ /*
+* If any blocks are being written to an encrypted file, encrypt them
+* into a bounce page. For simplicity, just encrypt until the last
+* block which might be needed. This may cause some unneeded blocks
+* (e.g. holes) to be unnecessarily encrypted, but this is rare and
+* can't happen in the common case of blocksize == PAGE_SIZE.
+*/
if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode) && nr_to_submit) {
gfp_t gfp_flags = GFP_NOFS;
+ unsigned int enc_bytes = round_up(len, i_blocksize(inode));
retry_encrypt:
- bounce_page = fscrypt_encrypt_pagecache_blocks(page, PAGE_SIZE,
+ bounce_page = fscrypt_encrypt_pagecache_blocks(page, enc_bytes,
0, gfp_flags);
if (IS_ERR(bounce_page)) {
ret = PTR_ERR(bounce_page);
--
2.21.0.593.g511ec345e18-goog
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 03/13] fscrypt: rename fscrypt_do_page_crypto() to fscrypt_crypt_block()
From: Eric Biggers
fscrypt_do_page_crypto() only does a single encryption or decryption
operation, with a single logical block number (single IV). So it
actually operates on a filesystem block, not a "page" per se. To
reflect this, rename it to fscrypt_crypt_block().
Signed-off-by: Eric Biggers
---
fs/crypto/bio.c | 6 +++---
fs/crypto/crypto.c | 24
fs/crypto/fscrypt_private.h | 11 +--
3 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index 548ec6bb569cf..bcab8822217b0 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -84,9 +84,9 @@ int fscrypt_zeroout_range(const struct inode *inode, pgoff_t
lblk,
return -ENOMEM;
while (len--) {
- err = fscrypt_do_page_crypto(inode, FS_ENCRYPT, lblk,
-ZERO_PAGE(0), ciphertext_page,
-PAGE_SIZE, 0, GFP_NOFS);
+ err = fscrypt_crypt_block(inode, FS_ENCRYPT, lblk,
+ ZERO_PAGE(0), ciphertext_page,
+ PAGE_SIZE, 0, GFP_NOFS);
if (err)
goto errout;
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index ebfa13cfecb7d..e6802d7aca3c7 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -147,10 +147,11 @@ void fscrypt_generate_iv(union fscrypt_iv *iv, u64
lblk_num,
crypto_cipher_encrypt_one(ci->ci_essiv_tfm, iv->raw, iv->raw);
}
-int fscrypt_do_page_crypto(const struct inode *inode, fscrypt_direction_t rw,
- u64 lblk_num, struct page *src_page,
- struct page *dest_page, unsigned int len,
- unsigned int offs, gfp_t gfp_flags)
+/* Encrypt or decrypt a single filesystem block of file contents */
+int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
+ u64 lblk_num, struct page *src_page,
+ struct page *dest_page, unsigned int len,
+ unsigned int offs, gfp_t gfp_flags)
{
union fscrypt_iv iv;
struct skcipher_request *req = NULL;
@@ -227,9 +228,9 @@ struct page *fscrypt_encrypt_page(const struct inode *inode,
if (inode->i_sb->s_cop->flags & FS_CFLG_OWN_PAGES) {
/* with inplace-encryption we just encrypt the page */
- err = fscrypt_do_page_crypto(inode, FS_ENCRYPT, lblk_num, page,
-ciphertext_page, len, offs,
-gfp_flags);
+ err = fscrypt_crypt_block(inode, FS_ENCRYPT, lblk_num, page,
+ ciphertext_page, len, offs,
+ gfp_flags);
if (err)
return ERR_PTR(err);
@@ -243,9 +244,8 @@ struct page *fscrypt_encrypt_page(const struct inode *inode,
if (!ciphertext_page)
return ERR_PTR(-ENOMEM);
- err = fscrypt_do_page_crypto(inode, FS_ENCRYPT, lblk_num,
-page, ciphertext_page, len, offs,
-gfp_flags);
+ err = fscrypt_crypt_block(inode, FS_ENCRYPT, lblk_num, page,
+ ciphertext_page, len, offs, gfp_flags);
if (err) {
fscrypt_free_bounce_page(ciphertext_page);
return ERR_PTR(err);
@@ -277,8 +277,8 @@ int fscrypt_decrypt_page(const struct inode *inode, struct
page *page,
if (!(inode->i_sb->s_cop->flags & FS_CFLG_OWN_PAGES))
BUG_ON(!PageLocked(page));
- return fscrypt_do_page_crypto(inode, FS_DECRYPT, lblk_num, page, page,
- len, offs, GFP_NOFS);
+ return fscrypt_crypt_block(inode, FS_DECRYPT, lblk_num, page, page,
+ len, offs, GFP_NOFS);
}
EXPORT_SYMBOL(fscrypt_decrypt_page);
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index a5a5486979ff7..8565536feb2b8 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -117,12 +117,11 @@ static inline bool fscrypt_valid_enc_modes(u32
contents_mode,
/* crypto.c */
extern struct kmem_cache *fscrypt_info_cachep;
extern int fscrypt_initialize(unsigned int cop_flags);
-extern int fscrypt_do_page_crypto(const struct inode *inode,
- fscrypt_direction_t rw, u64 lblk_num,
- struct page *src_page,
- struct page *dest_page,
- unsigned int len, unsigned int offs,
- gfp_t gfp_flags);
+extern int fscrypt_crypt_block(const struct inode *inode,
+ fscrypt_direction_t rw, u64 lblk_num,
+
[f2fs-dev] [PATCH 12/13] ext4: decrypt only the needed block in __ext4_block_zero_page_range()
From: Chandan Rajendra
In __ext4_block_zero_page_range(), only decrypt the block that actually
needs to be decrypted, rather than assuming blocksize == PAGE_SIZE and
decrypting the whole page.
This is in preparation for allowing encryption on ext4 filesystems with
blocksize != PAGE_SIZE.
Signed-off-by: Chandan Rajendra
(EB: rebase onto previous changes, improve the commit message, and use
bh_offset())
Signed-off-by: Eric Biggers
---
fs/ext4/inode.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index d24c50e4598f0..58597db621e1e 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4073,9 +4073,8 @@ static int __ext4_block_zero_page_range(handle_t *handle,
if (S_ISREG(inode->i_mode) && IS_ENCRYPTED(inode)) {
/* We expect the key to be set. */
BUG_ON(!fscrypt_has_encryption_key(inode));
- BUG_ON(blocksize != PAGE_SIZE);
WARN_ON_ONCE(fscrypt_decrypt_pagecache_blocks(
- page, PAGE_SIZE, 0));
+ page, blocksize, bh_offset(bh)));
}
}
if (ext4_should_journal_data(inode)) {
--
2.21.0.593.g511ec345e18-goog
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 04/13] fscrypt: clean up some BUG_ON()s in block encryption/decryption
From: Eric Biggers
Replace some BUG_ON()s with WARN_ON_ONCE() and returning an error code,
and move the check for len divisible by FS_CRYPTO_BLOCK_SIZE into
fscrypt_crypt_block() so that it's done for both encryption and
decryption, not just encryption.
Signed-off-by: Eric Biggers
---
fs/crypto/crypto.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index e6802d7aca3c7..9cda0147fca95 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -161,7 +161,10 @@ int fscrypt_crypt_block(const struct inode *inode,
fscrypt_direction_t rw,
struct crypto_skcipher *tfm = ci->ci_ctfm;
int res = 0;
- BUG_ON(len == 0);
+ if (WARN_ON_ONCE(len <= 0))
+ return -EINVAL;
+ if (WARN_ON_ONCE(len % FS_CRYPTO_BLOCK_SIZE != 0))
+ return -EINVAL;
fscrypt_generate_iv(, lblk_num, ci);
@@ -224,8 +227,6 @@ struct page *fscrypt_encrypt_page(const struct inode *inode,
struct page *ciphertext_page = page;
int err;
- BUG_ON(len % FS_CRYPTO_BLOCK_SIZE != 0);
-
if (inode->i_sb->s_cop->flags & FS_CFLG_OWN_PAGES) {
/* with inplace-encryption we just encrypt the page */
err = fscrypt_crypt_block(inode, FS_ENCRYPT, lblk_num, page,
@@ -237,7 +238,8 @@ struct page *fscrypt_encrypt_page(const struct inode *inode,
return ciphertext_page;
}
- BUG_ON(!PageLocked(page));
+ if (WARN_ON_ONCE(!PageLocked(page)))
+ return ERR_PTR(-EINVAL);
/* The encryption operation will require a bounce page. */
ciphertext_page = fscrypt_alloc_bounce_page(gfp_flags);
@@ -274,8 +276,9 @@ EXPORT_SYMBOL(fscrypt_encrypt_page);
int fscrypt_decrypt_page(const struct inode *inode, struct page *page,
unsigned int len, unsigned int offs, u64 lblk_num)
{
- if (!(inode->i_sb->s_cop->flags & FS_CFLG_OWN_PAGES))
- BUG_ON(!PageLocked(page));
+ if (WARN_ON_ONCE(!PageLocked(page) &&
+!(inode->i_sb->s_cop->flags & FS_CFLG_OWN_PAGES)))
+ return -EINVAL;
return fscrypt_crypt_block(inode, FS_DECRYPT, lblk_num, page, page,
len, offs, GFP_NOFS);
--
2.21.0.593.g511ec345e18-goog
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 08/13] fscrypt: introduce fscrypt_decrypt_block_inplace()
From: Eric Biggers
fscrypt_decrypt_page() behaves very differently depending on whether the
filesystem set FS_CFLG_OWN_PAGES in its fscrypt_operations. This makes
the function difficult to understand and document. It also makes it so
that all callers have to provide inode and lblk_num, when fscrypt could
determine these itself for pagecache pages.
Therefore, move the FS_CFLG_OWN_PAGES behavior into a new function
fscrypt_decrypt_block_inplace().
Signed-off-by: Eric Biggers
---
fs/crypto/crypto.c | 31 +++
fs/ubifs/crypto.c | 7 ---
include/linux/fscrypt.h | 11 +++
3 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 7bdb985126d97..2e6fb5e4f7a7f 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -284,8 +284,7 @@ EXPORT_SYMBOL(fscrypt_encrypt_block_inplace);
/**
* fscrypt_decrypt_page() - Decrypts a page in-place
* @inode: The corresponding inode for the page to decrypt.
- * @page: The page to decrypt. Must be locked in case
- * it is a writeback page (FS_CFLG_OWN_PAGES unset).
+ * @page: The page to decrypt. Must be locked.
* @len: Number of bytes in @page to be decrypted.
* @offs: Start of data in @page.
* @lblk_num: Logical block number.
@@ -299,8 +298,7 @@ EXPORT_SYMBOL(fscrypt_encrypt_block_inplace);
int fscrypt_decrypt_page(const struct inode *inode, struct page *page,
unsigned int len, unsigned int offs, u64 lblk_num)
{
- if (WARN_ON_ONCE(!PageLocked(page) &&
-!(inode->i_sb->s_cop->flags & FS_CFLG_OWN_PAGES)))
+ if (WARN_ON_ONCE(!PageLocked(page)))
return -EINVAL;
return fscrypt_crypt_block(inode, FS_DECRYPT, lblk_num, page, page,
@@ -308,6 +306,31 @@ int fscrypt_decrypt_page(const struct inode *inode, struct
page *page,
}
EXPORT_SYMBOL(fscrypt_decrypt_page);
+/**
+ * fscrypt_decrypt_block_inplace() - Decrypt a filesystem block in-place
+ * @inode: The inode to which this block belongs
+ * @page: The page containing the block to decrypt
+ * @len: Size of block to decrypt. Doesn't need to be a multiple of the
+ * fs block size, but must be a multiple of FS_CRYPTO_BLOCK_SIZE.
+ * @offs: Byte offset within @page at which the block to decrypt begins
+ * @lblk_num: Filesystem logical block number of the block, i.e. the 0-based
+ * number of the block within the file
+ *
+ * Decrypt a possibly-compressed filesystem block that is located in an
+ * arbitrary page, not necessarily in the original pagecache page. The @inode
+ * and @lblk_num must be specified, as they can't be determined from @page.
+ *
+ * Return: 0 on success; -errno on failure
+ */
+int fscrypt_decrypt_block_inplace(const struct inode *inode, struct page *page,
+ unsigned int len, unsigned int offs,
+ u64 lblk_num)
+{
+ return fscrypt_crypt_block(inode, FS_DECRYPT, lblk_num, page, page,
+ len, offs, GFP_NOFS);
+}
+EXPORT_SYMBOL(fscrypt_decrypt_block_inplace);
+
/*
* Validate dentries in encrypted directories to make sure we aren't
potentially
* caching stale dentries after a key has been added.
diff --git a/fs/ubifs/crypto.c b/fs/ubifs/crypto.c
index 032efdad2e668..22be7aeb96c4f 100644
--- a/fs/ubifs/crypto.c
+++ b/fs/ubifs/crypto.c
@@ -64,10 +64,11 @@ int ubifs_decrypt(const struct inode *inode, struct
ubifs_data_node *dn,
}
ubifs_assert(c, dlen <= UBIFS_BLOCK_SIZE);
- err = fscrypt_decrypt_page(inode, virt_to_page(>data), dlen,
- offset_in_page(>data), block);
+ err = fscrypt_decrypt_block_inplace(inode, virt_to_page(>data),
+ dlen, offset_in_page(>data),
+ block);
if (err) {
- ubifs_err(c, "fscrypt_decrypt_page failed: %i", err);
+ ubifs_err(c, "fscrypt_decrypt_block_inplace() failed: %d", err);
return err;
}
*out_len = clen;
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 39229fcdfac5c..f4890870ca984 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -114,6 +114,9 @@ extern int fscrypt_encrypt_block_inplace(const struct inode
*inode,
gfp_t gfp_flags);
extern int fscrypt_decrypt_page(const struct inode *, struct page *, unsigned
int,
unsigned int, u64);
+extern int fscrypt_decrypt_block_inplace(const struct inode *inode,
+struct page *page, unsigned int len,
+unsigned int offs, u64 lblk_num);
static inline bool fscrypt_is_bounce_page(struct page *page)
{
@@ -310,6 +313,14 @@ static inline int fscrypt_decrypt_page(const
[f2fs-dev] [PATCH 00/13] fscrypt, ext4: prepare for blocksize != PAGE_SIZE
Hello, This patch series prepares fs/crypto/, and partially ext4, for the 'blocksize != PAGE_SIZE' case. This basically contains the encryption changes from Chandan Rajendra's patch series "[V2,00/13] Consolidate FS read I/O callbacks code" (https://patchwork.kernel.org/project/linux-fscrypt/list/?series=111039) that don't require introducing the read_callbacks and don't depend on fsverity stuff. But they've been reworked to clean things up a lot. I propose that to move things forward for ext4 encryption with 'blocksize != PAGE_SIZE', we apply this series (or something similar) to the fscrypt tree for 5.3 on its own merits. Then the read_callbacks series on top of it will much smaller and easier to review. AFAIK, after this series the only thing stopping ext4 encryption from working with blocksize != PAGE_SIZE is the lack of encryption support in block_read_full_page(), which the read_callbacks will address. This series applies to the fscrypt tree, and it can also be retrieved from git at https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git branch "fscrypt-subpage-blocks-prep". Chandan Rajendra (3): ext4: clear BH_Uptodate flag on decryption error ext4: decrypt only the needed blocks in ext4_block_write_begin() ext4: decrypt only the needed block in __ext4_block_zero_page_range() Eric Biggers (10): fscrypt: simplify bounce page handling fscrypt: remove the "write" part of struct fscrypt_ctx fscrypt: rename fscrypt_do_page_crypto() to fscrypt_crypt_block() fscrypt: clean up some BUG_ON()s in block encryption/decryption fscrypt: introduce fscrypt_encrypt_block_inplace() fscrypt: support encrypting multiple filesystem blocks per page fscrypt: handle blocksize < PAGE_SIZE in fscrypt_zeroout_range() fscrypt: introduce fscrypt_decrypt_block_inplace() fscrypt: support decrypting multiple filesystem blocks per page ext4: encrypt only up to last block in ext4_bio_write_page() fs/crypto/bio.c | 73 +++-- fs/crypto/crypto.c | 299 fs/crypto/fscrypt_private.h | 14 +- fs/ext4/inode.c | 35 +++-- fs/ext4/page-io.c | 44 +++--- fs/f2fs/data.c | 17 +- fs/ubifs/crypto.c | 19 +-- include/linux/fscrypt.h | 96 8 files changed, 319 insertions(+), 278 deletions(-) -- 2.21.0.593.g511ec345e18-goog ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 05/13] fscrypt: introduce fscrypt_encrypt_block_inplace()
From: Eric Biggers
fscrypt_encrypt_page() behaves very differently depending on whether the
filesystem set FS_CFLG_OWN_PAGES in its fscrypt_operations. This makes
the function difficult to understand and document. It also makes it so
that all callers have to provide inode and lblk_num, when fscrypt could
determine these itself for pagecache pages.
Therefore, move the FS_CFLG_OWN_PAGES behavior into a new function
fscrypt_encrypt_block_inplace().
Signed-off-by: Eric Biggers
---
fs/crypto/crypto.c | 52 +
fs/ubifs/crypto.c | 12 +-
include/linux/fscrypt.h | 13 +++
3 files changed, 51 insertions(+), 26 deletions(-)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 9cda0147fca95..e978541e2ec19 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -199,8 +199,7 @@ int fscrypt_crypt_block(const struct inode *inode,
fscrypt_direction_t rw,
/**
* fscypt_encrypt_page() - Encrypts a page
* @inode: The inode for which the encryption should take place
- * @page: The page to encrypt. Must be locked for bounce-page
- * encryption.
+ * @page: The page to encrypt. Must be locked.
* @len: Length of data to encrypt in @page and encrypted
* data in returned page.
* @offs: Offset of data within @page and returned
@@ -210,12 +209,11 @@ int fscrypt_crypt_block(const struct inode *inode,
fscrypt_direction_t rw,
* previously written data.
* @gfp_flags: The gfp flag for memory allocation
*
- * Encrypts @page. If the filesystem set FS_CFLG_OWN_PAGES, then the data is
- * encrypted in-place and @page is returned. Else, a bounce page is allocated,
- * the data is encrypted into the bounce page, and the bounce page is returned.
- * The caller is responsible for calling fscrypt_free_bounce_page().
+ * Encrypts @page. A bounce page is allocated, the data is encrypted into the
+ * bounce page, and the bounce page is returned. The caller is responsible for
+ * calling fscrypt_free_bounce_page().
*
- * Return: A page containing the encrypted data on success, else an ERR_PTR()
+ * Return: A page containing the encrypted data on success; else an ERR_PTR()
*/
struct page *fscrypt_encrypt_page(const struct inode *inode,
struct page *page,
@@ -224,24 +222,12 @@ struct page *fscrypt_encrypt_page(const struct inode
*inode,
u64 lblk_num, gfp_t gfp_flags)
{
- struct page *ciphertext_page = page;
+ struct page *ciphertext_page;
int err;
- if (inode->i_sb->s_cop->flags & FS_CFLG_OWN_PAGES) {
- /* with inplace-encryption we just encrypt the page */
- err = fscrypt_crypt_block(inode, FS_ENCRYPT, lblk_num, page,
- ciphertext_page, len, offs,
- gfp_flags);
- if (err)
- return ERR_PTR(err);
-
- return ciphertext_page;
- }
-
if (WARN_ON_ONCE(!PageLocked(page)))
return ERR_PTR(-EINVAL);
- /* The encryption operation will require a bounce page. */
ciphertext_page = fscrypt_alloc_bounce_page(gfp_flags);
if (!ciphertext_page)
return ERR_PTR(-ENOMEM);
@@ -258,6 +244,32 @@ struct page *fscrypt_encrypt_page(const struct inode
*inode,
}
EXPORT_SYMBOL(fscrypt_encrypt_page);
+/**
+ * fscrypt_encrypt_block_inplace() - Encrypt a filesystem block in-place
+ * @inode: The inode to which this block belongs
+ * @page: The page containing the block to encrypt
+ * @len: Size of block to encrypt. Doesn't need to be a multiple of the
+ * fs block size, but must be a multiple of FS_CRYPTO_BLOCK_SIZE.
+ * @offs: Byte offset within @page at which the block to encrypt begins
+ * @lblk_num: Filesystem logical block number of the block, i.e. the 0-based
+ * number of the block within the file
+ * @gfp_flags: Memory allocation flags
+ *
+ * Encrypt a possibly-compressed filesystem block that is located in an
+ * arbitrary page, not necessarily in the original pagecache page. The @inode
+ * and @lblk_num must be specified, as they can't be determined from @page.
+ *
+ * Return: 0 on success; -errno on failure
+ */
+int fscrypt_encrypt_block_inplace(const struct inode *inode, struct page *page,
+ unsigned int len, unsigned int offs,
+ u64 lblk_num, gfp_t gfp_flags)
+{
+ return fscrypt_crypt_block(inode, FS_ENCRYPT, lblk_num, page, page,
+ len, offs, gfp_flags);
+}
+EXPORT_SYMBOL(fscrypt_encrypt_block_inplace);
+
/**
* fscrypt_decrypt_page() - Decrypts a page in-place
* @inode: The corresponding inode for the page to decrypt.
diff --git a/fs/ubifs/crypto.c b/fs/ubifs/crypto.c
index 4aaedf2d7f442..032efdad2e668 100644
---
[f2fs-dev] [PATCH 11/13] ext4: decrypt only the needed blocks in ext4_block_write_begin()
From: Chandan Rajendra
In ext4_block_write_begin(), only decrypt the blocks that actually need
to be decrypted (up to two blocks which intersect the boundaries of the
region that will be written to), rather than assuming blocksize ==
PAGE_SIZE and decrypting the whole page.
This is in preparation for allowing encryption on ext4 filesystems with
blocksize != PAGE_SIZE.
Signed-off-by: Chandan Rajendra
(EB: rebase onto previous changes and improve the commit message)
Signed-off-by: Eric Biggers
---
fs/ext4/inode.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 9382e1bcefe49..d24c50e4598f0 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1160,8 +1160,10 @@ static int ext4_block_write_begin(struct page *page,
loff_t pos, unsigned len,
int err = 0;
unsigned blocksize = inode->i_sb->s_blocksize;
unsigned bbits;
- struct buffer_head *bh, *head, *wait[2], **wait_bh = wait;
+ struct buffer_head *bh, *head, *wait[2];
+ int nr_wait = 0;
bool decrypt = false;
+ int i;
BUG_ON(!PageLocked(page));
BUG_ON(from > PAGE_SIZE);
@@ -1213,24 +1215,31 @@ static int ext4_block_write_begin(struct page *page,
loff_t pos, unsigned len,
!buffer_unwritten(bh) &&
(block_start < from || block_end > to)) {
ll_rw_block(REQ_OP_READ, 0, 1, );
- *wait_bh++ = bh;
+ wait[nr_wait++] = bh;
decrypt = IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode);
}
}
/*
* If we issued read requests, let them complete.
*/
- while (wait_bh > wait) {
- wait_on_buffer(*--wait_bh);
- if (!buffer_uptodate(*wait_bh))
+ for (i = 0; i < nr_wait; i++) {
+ wait_on_buffer(wait[i]);
+ if (!buffer_uptodate(wait[i]))
err = -EIO;
}
if (unlikely(err)) {
page_zero_new_buffers(page, from, to);
} else if (decrypt) {
- err = fscrypt_decrypt_pagecache_blocks(page, PAGE_SIZE, 0);
- if (err)
- clear_buffer_uptodate(*wait_bh);
+ for (i = 0; i < nr_wait; i++) {
+ int err2;
+
+ err2 = fscrypt_decrypt_pagecache_blocks(page, blocksize,
+
bh_offset(wait[i]));
+ if (err2) {
+ clear_buffer_uptodate(wait[i]);
+ err = err2;
+ }
+ }
}
return err;
--
2.21.0.593.g511ec345e18-goog
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 01/13] fscrypt: simplify bounce page handling
From: Eric Biggers
Currently, bounce page handling for writes to encrypted files is
unnecessarily complicated. A fscrypt_ctx is allocated along with each
bounce page, page_private(bounce_page) points to this fscrypt_ctx, and
fscrypt_ctx::w::control_page points to the original pagecache page.
However, because writes don't use the fscrypt_ctx for anything else,
there's no reason why page_private(bounce_page) can't just point to the
original pagecache page directly.
Therefore, this patch makes this change. In the process, it also cleans
up the API exposed to filesystems that allows testing whether a page is
a bounce page, getting the pagecache page from a bounce page, and
freeing a bounce page.
Signed-off-by: Eric Biggers
---
fs/crypto/bio.c | 38 ++---
fs/crypto/crypto.c | 104
fs/crypto/fscrypt_private.h | 3 +-
fs/ext4/page-io.c | 36 +
fs/f2fs/data.c | 12 ++---
include/linux/fscrypt.h | 38 -
6 files changed, 84 insertions(+), 147 deletions(-)
diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index 41dde4578f3b2..9107e8fe55897 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -71,46 +71,18 @@ void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx,
struct bio *bio)
}
EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
-void fscrypt_pullback_bio_page(struct page **page, bool restore)
-{
- struct fscrypt_ctx *ctx;
- struct page *bounce_page;
-
- /* The bounce data pages are unmapped. */
- if ((*page)->mapping)
- return;
-
- /* The bounce data page is unmapped. */
- bounce_page = *page;
- ctx = (struct fscrypt_ctx *)page_private(bounce_page);
-
- /* restore control page */
- *page = ctx->w.control_page;
-
- if (restore)
- fscrypt_restore_control_page(bounce_page);
-}
-EXPORT_SYMBOL(fscrypt_pullback_bio_page);
-
int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
sector_t pblk, unsigned int len)
{
- struct fscrypt_ctx *ctx;
- struct page *ciphertext_page = NULL;
+ struct page *ciphertext_page;
struct bio *bio;
int ret, err = 0;
BUG_ON(inode->i_sb->s_blocksize != PAGE_SIZE);
- ctx = fscrypt_get_ctx(GFP_NOFS);
- if (IS_ERR(ctx))
- return PTR_ERR(ctx);
-
- ciphertext_page = fscrypt_alloc_bounce_page(ctx, GFP_NOWAIT);
- if (IS_ERR(ciphertext_page)) {
- err = PTR_ERR(ciphertext_page);
- goto errout;
- }
+ ciphertext_page = fscrypt_alloc_bounce_page(GFP_NOWAIT);
+ if (!ciphertext_page)
+ return -ENOMEM;
while (len--) {
err = fscrypt_do_page_crypto(inode, FS_ENCRYPT, lblk,
@@ -148,7 +120,7 @@ int fscrypt_zeroout_range(const struct inode *inode,
pgoff_t lblk,
}
err = 0;
errout:
- fscrypt_release_ctx(ctx);
+ fscrypt_free_bounce_page(ciphertext_page);
return err;
}
EXPORT_SYMBOL(fscrypt_zeroout_range);
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 68e2ca4c4af63..4b076f8daab75 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -63,18 +63,11 @@ EXPORT_SYMBOL(fscrypt_enqueue_decrypt_work);
*
* If the encryption context was allocated from the pre-allocated pool, returns
* it to that pool. Else, frees it.
- *
- * If there's a bounce page in the context, this frees that.
*/
void fscrypt_release_ctx(struct fscrypt_ctx *ctx)
{
unsigned long flags;
- if (ctx->flags & FS_CTX_HAS_BOUNCE_BUFFER_FL && ctx->w.bounce_page) {
- mempool_free(ctx->w.bounce_page, fscrypt_bounce_page_pool);
- ctx->w.bounce_page = NULL;
- }
- ctx->w.control_page = NULL;
if (ctx->flags & FS_CTX_REQUIRES_FREE_ENCRYPT_FL) {
kmem_cache_free(fscrypt_ctx_cachep, ctx);
} else {
@@ -99,14 +92,8 @@ struct fscrypt_ctx *fscrypt_get_ctx(gfp_t gfp_flags)
unsigned long flags;
/*
-* We first try getting the ctx from a free list because in
-* the common case the ctx will have an allocated and
-* initialized crypto tfm, so it's probably a worthwhile
-* optimization. For the bounce page, we first try getting it
-* from the kernel allocator because that's just about as fast
-* as getting it from a list and because a cache of free pages
-* should generally be a "last resort" option for a filesystem
-* to be able to do its job.
+* First try getting a ctx from the free list so that we don't have to
+* call into the slab allocator.
*/
spin_lock_irqsave(_ctx_lock, flags);
ctx = list_first_entry_or_null(_free_ctxs,
@@ -122,11 +109,31 @@ struct fscrypt_ctx *fscrypt_get_ctx(gfp_t gfp_flags)
} else {
ctx->flags &= ~FS_CTX_REQUIRES_FREE_ENCRYPT_FL;
[f2fs-dev] [PATCH 07/13] fscrypt: handle blocksize < PAGE_SIZE in fscrypt_zeroout_range()
From: Eric Biggers
Adjust fscrypt_zeroout_range() to encrypt a block at a time rather than
a page at a time, so that it works when blocksize < PAGE_SIZE.
This isn't optimized for performance, but then again this function
already wasn't optimized for performance. As a future optimization, we
could submit much larger bios here.
This is in preparation for allowing encryption on ext4 filesystems with
blocksize != PAGE_SIZE.
This is based on work by Chandan Rajendra.
Signed-off-by: Eric Biggers
---
fs/crypto/bio.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index bcab8822217b0..e67e9d4d342b3 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -73,12 +73,12 @@ EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
sector_t pblk, unsigned int len)
{
+ const unsigned int blockbits = inode->i_blkbits;
+ const unsigned int blocksize = 1 << blockbits;
struct page *ciphertext_page;
struct bio *bio;
int ret, err = 0;
- BUG_ON(inode->i_sb->s_blocksize != PAGE_SIZE);
-
ciphertext_page = fscrypt_alloc_bounce_page(GFP_NOWAIT);
if (!ciphertext_page)
return -ENOMEM;
@@ -86,7 +86,7 @@ int fscrypt_zeroout_range(const struct inode *inode, pgoff_t
lblk,
while (len--) {
err = fscrypt_crypt_block(inode, FS_ENCRYPT, lblk,
ZERO_PAGE(0), ciphertext_page,
- PAGE_SIZE, 0, GFP_NOFS);
+ blocksize, 0, GFP_NOFS);
if (err)
goto errout;
@@ -96,14 +96,11 @@ int fscrypt_zeroout_range(const struct inode *inode,
pgoff_t lblk,
goto errout;
}
bio_set_dev(bio, inode->i_sb->s_bdev);
- bio->bi_iter.bi_sector =
- pblk << (inode->i_sb->s_blocksize_bits - 9);
+ bio->bi_iter.bi_sector = pblk << (blockbits - 9);
bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
- ret = bio_add_page(bio, ciphertext_page,
- inode->i_sb->s_blocksize, 0);
- if (ret != inode->i_sb->s_blocksize) {
+ ret = bio_add_page(bio, ciphertext_page, blocksize, 0);
+ if (WARN_ON(ret != blocksize)) {
/* should never happen! */
- WARN_ON(1);
bio_put(bio);
err = -EIO;
goto errout;
--
2.21.0.593.g511ec345e18-goog
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V2 10/13] fscrypt_encrypt_page: Loop across all blocks mapped by a page range
Hi Chandan,
On Wed, May 01, 2019 at 08:19:35PM +0530, Chandan Rajendra wrote:
> On Wednesday, May 1, 2019 4:38:41 AM IST Eric Biggers wrote:
> > On Tue, Apr 30, 2019 at 10:11:35AM -0700, Eric Biggers wrote:
> > > On Sun, Apr 28, 2019 at 10:01:18AM +0530, Chandan Rajendra wrote:
> > > > For subpage-sized blocks, this commit now encrypts all blocks mapped by
> > > > a page range.
> > > >
> > > > Signed-off-by: Chandan Rajendra
> > > > ---
> > > > fs/crypto/crypto.c | 37 +
> > > > 1 file changed, 25 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> > > > index 4f0d832cae71..2d65b431563f 100644
> > > > --- a/fs/crypto/crypto.c
> > > > +++ b/fs/crypto/crypto.c
> > > > @@ -242,18 +242,26 @@ struct page *fscrypt_encrypt_page(const struct
> > > > inode *inode,
> > >
> > > Need to update the function comment to clearly explain what this function
> > > actually does now.
> > >
> > > > {
> > > > struct fscrypt_ctx *ctx;
> > > > struct page *ciphertext_page = page;
> > > > + int i, page_nr_blks;
> > > > int err;
> > > >
> > > > BUG_ON(len % FS_CRYPTO_BLOCK_SIZE != 0);
> > > >
> > >
> > > Make a 'blocksize' variable so you don't have to keep calling
> > > i_blocksize().
> > >
> > > Also, you need to check whether 'len' and 'offs' are
> > > filesystem-block-aligned,
> > > since the code now assumes it.
> > >
> > > const unsigned int blocksize = i_blocksize(inode);
> > >
> > > if (!IS_ALIGNED(len | offs, blocksize))
> > > return -EINVAL;
> > >
> > > However, did you check whether that's always true for ubifs? It looks
> > > like it
> > > may expect to encrypt a prefix of a block, that is only padded to the next
> > > 16-byte boundary.
> > >
> > > > + page_nr_blks = len >> inode->i_blkbits;
> > > > +
> > > > if (inode->i_sb->s_cop->flags & FS_CFLG_OWN_PAGES) {
> > > > /* with inplace-encryption we just encrypt the page */
> > > > - err = fscrypt_do_page_crypto(inode, FS_ENCRYPT,
> > > > lblk_num, page,
> > > > -ciphertext_page, len, offs,
> > > > -gfp_flags);
> > > > - if (err)
> > > > - return ERR_PTR(err);
> > > > -
> > > > + for (i = 0; i < page_nr_blks; i++) {
> > > > + err = fscrypt_do_page_crypto(inode, FS_ENCRYPT,
> > > > + lblk_num, page,
> > > > + ciphertext_page,
> > > > + i_blocksize(inode),
> > > > offs,
> > > > + gfp_flags);
> > > > + if (err)
> > > > + return ERR_PTR(err);
> >
> > Apparently ubifs does encrypt data shorter than the filesystem block size,
> > so
> > this part is wrong.
> >
> > I suggest we split this into two functions, fscrypt_encrypt_block_inplace()
> > and
> > fscrypt_encrypt_blocks(), so that it's conceptually simpler what each
> > function
> > does. Currently this works completely differently depending on whether the
> > filesystem set FS_CFLG_OWN_PAGES in its fscrypt_operations, which is weird.
> >
> > I also noticed that using fscrypt_ctx for writes seems to be unnecessary.
> > AFAICS, page_private(bounce_page) could point directly to the pagecache
> > page.
> > That would simplify things a lot, especially since then fscrypt_ctx could be
> > removed entirely after you convert reads to use read_callbacks_ctx.
> >
> > IMO, these would be worthwhile cleanups for fscrypt by themselves, without
> > waiting for the read_callbacks stuff to be finalized. Finalizing the
> > read_callbacks stuff will probably require reaching a consensus about how
> > they
> > should work with future filesystem features like fsverity and compression.
> >
> > So to move things forward, I'm considering sending out a series with the
> > above
> > cleanups for fscrypt, plus the equivalent of your patches:
> >
> > "fscrypt_encrypt_page: Loop across all blocks mapped by a page range"
> > "fscrypt_zeroout_range: Encrypt all zeroed out blocks of a page"
> > "Add decryption support for sub-pagesized blocks" (fs/crypto/ part only)
> >
> > Then hopefully we can get all that applied for 5.3 so that fs/crypto/
> > itself is
> > ready for blocksize != PAGE_SIZE; and get your changes to
> > ext4_bio_write_page(),
> > __ext4_block_zero_page_range(), and ext4_block_write_begin() applied too, so
> > that ext4 is partially ready for encryption with blocksize != PAGE_SIZE.
> >
> > Then only the read_callbacks stuff will remain, to get encryption support
> > into
> > fs/mpage.c and fs/buffer.c. Do you think that's a good plan?
>
> Hi Eric,
>
> IMHO, I will continue posting the next
Re: [f2fs-dev] [PATCH V2 10/13] fscrypt_encrypt_page: Loop across all blocks mapped by a page range
On Wednesday, May 1, 2019 4:38:41 AM IST Eric Biggers wrote:
> On Tue, Apr 30, 2019 at 10:11:35AM -0700, Eric Biggers wrote:
> > On Sun, Apr 28, 2019 at 10:01:18AM +0530, Chandan Rajendra wrote:
> > > For subpage-sized blocks, this commit now encrypts all blocks mapped by
> > > a page range.
> > >
> > > Signed-off-by: Chandan Rajendra
> > > ---
> > > fs/crypto/crypto.c | 37 +
> > > 1 file changed, 25 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> > > index 4f0d832cae71..2d65b431563f 100644
> > > --- a/fs/crypto/crypto.c
> > > +++ b/fs/crypto/crypto.c
> > > @@ -242,18 +242,26 @@ struct page *fscrypt_encrypt_page(const struct
> > > inode *inode,
> >
> > Need to update the function comment to clearly explain what this function
> > actually does now.
> >
> > > {
> > > struct fscrypt_ctx *ctx;
> > > struct page *ciphertext_page = page;
> > > + int i, page_nr_blks;
> > > int err;
> > >
> > > BUG_ON(len % FS_CRYPTO_BLOCK_SIZE != 0);
> > >
> >
> > Make a 'blocksize' variable so you don't have to keep calling i_blocksize().
> >
> > Also, you need to check whether 'len' and 'offs' are
> > filesystem-block-aligned,
> > since the code now assumes it.
> >
> > const unsigned int blocksize = i_blocksize(inode);
> >
> > if (!IS_ALIGNED(len | offs, blocksize))
> > return -EINVAL;
> >
> > However, did you check whether that's always true for ubifs? It looks like
> > it
> > may expect to encrypt a prefix of a block, that is only padded to the next
> > 16-byte boundary.
> >
> > > + page_nr_blks = len >> inode->i_blkbits;
> > > +
> > > if (inode->i_sb->s_cop->flags & FS_CFLG_OWN_PAGES) {
> > > /* with inplace-encryption we just encrypt the page */
> > > - err = fscrypt_do_page_crypto(inode, FS_ENCRYPT, lblk_num, page,
> > > - ciphertext_page, len, offs,
> > > - gfp_flags);
> > > - if (err)
> > > - return ERR_PTR(err);
> > > -
> > > + for (i = 0; i < page_nr_blks; i++) {
> > > + err = fscrypt_do_page_crypto(inode, FS_ENCRYPT,
> > > + lblk_num, page,
> > > + ciphertext_page,
> > > + i_blocksize(inode), offs,
> > > + gfp_flags);
> > > + if (err)
> > > + return ERR_PTR(err);
>
> Apparently ubifs does encrypt data shorter than the filesystem block size, so
> this part is wrong.
>
> I suggest we split this into two functions, fscrypt_encrypt_block_inplace()
> and
> fscrypt_encrypt_blocks(), so that it's conceptually simpler what each function
> does. Currently this works completely differently depending on whether the
> filesystem set FS_CFLG_OWN_PAGES in its fscrypt_operations, which is weird.
>
> I also noticed that using fscrypt_ctx for writes seems to be unnecessary.
> AFAICS, page_private(bounce_page) could point directly to the pagecache page.
> That would simplify things a lot, especially since then fscrypt_ctx could be
> removed entirely after you convert reads to use read_callbacks_ctx.
>
> IMO, these would be worthwhile cleanups for fscrypt by themselves, without
> waiting for the read_callbacks stuff to be finalized. Finalizing the
> read_callbacks stuff will probably require reaching a consensus about how they
> should work with future filesystem features like fsverity and compression.
>
> So to move things forward, I'm considering sending out a series with the above
> cleanups for fscrypt, plus the equivalent of your patches:
>
> "fscrypt_encrypt_page: Loop across all blocks mapped by a page range"
> "fscrypt_zeroout_range: Encrypt all zeroed out blocks of a page"
> "Add decryption support for sub-pagesized blocks" (fs/crypto/ part only)
>
> Then hopefully we can get all that applied for 5.3 so that fs/crypto/ itself
> is
> ready for blocksize != PAGE_SIZE; and get your changes to
> ext4_bio_write_page(),
> __ext4_block_zero_page_range(), and ext4_block_write_begin() applied too, so
> that ext4 is partially ready for encryption with blocksize != PAGE_SIZE.
>
> Then only the read_callbacks stuff will remain, to get encryption support into
> fs/mpage.c and fs/buffer.c. Do you think that's a good plan?
Hi Eric,
IMHO, I will continue posting the next version of the current patchset and if
there are no serious reservations from FS maintainers the "read callbacks"
patchset can be merged. In such a scenario, the cleanups being
non-complicated, can be merged later.
--
chandan
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V2 11/13] ext4: Compute logical block and the page range to be encrypted
On Tuesday, April 30, 2019 10:31:51 PM IST Eric Biggers wrote:
> On Sun, Apr 28, 2019 at 10:01:19AM +0530, Chandan Rajendra wrote:
> > For subpage-sized blocks, the initial logical block number mapped by a
> > page can be different from page->index. Hence this commit adds code to
> > compute the first logical block mapped by the page and also the page
> > range to be encrypted.
> >
> > Signed-off-by: Chandan Rajendra
> > ---
> > fs/ext4/page-io.c | 9 +++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> > index 3e9298e6a705..75485ee9e800 100644
> > --- a/fs/ext4/page-io.c
> > +++ b/fs/ext4/page-io.c
> > @@ -418,6 +418,7 @@ int ext4_bio_write_page(struct ext4_io_submit *io,
> > {
> > struct page *data_page = NULL;
> > struct inode *inode = page->mapping->host;
> > + u64 page_blk;
> > unsigned block_start;
> > struct buffer_head *bh, *head;
> > int ret = 0;
> > @@ -478,10 +479,14 @@ int ext4_bio_write_page(struct ext4_io_submit *io,
> >
> > if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode) && nr_to_submit) {
> > gfp_t gfp_flags = GFP_NOFS;
> > + unsigned int page_bytes;
> > +
>
> page_blk should be declared here, just after page_bytes.
>
> > + page_bytes = round_up(len, i_blocksize(inode));
> > + page_blk = page->index << (PAGE_SHIFT - inode->i_blkbits);
>
> Although block numbers are 32-bit in ext4, if you're going to make 'page_blk'
> a
> u64 anyway, then for consistency page->index should be cast to u64 here.
>
> >
> > retry_encrypt:
> > - data_page = fscrypt_encrypt_page(inode, page, PAGE_SIZE, 0,
> > - page->index, gfp_flags);
> > + data_page = fscrypt_encrypt_page(inode, page, page_bytes, 0,
> > + page_blk, gfp_flags);
> > if (IS_ERR(data_page)) {
> > ret = PTR_ERR(data_page);
> > if (ret == -ENOMEM && wbc->sync_mode == WB_SYNC_ALL) {
>
>
I will implement the changes that have been suggested here.
--
chandan
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V2 02/13] Consolidate "read callbacks" into a new file
On Tuesday, April 30, 2019 7:07:28 AM IST Chao Yu wrote:
> On 2019/4/28 12:31, Chandan Rajendra wrote:
> > The "read callbacks" code is used by both Ext4 and F2FS. Hence to
> > remove duplicity, this commit moves the code into
> > include/linux/read_callbacks.h and fs/read_callbacks.c.
> >
> > The corresponding decrypt and verity "work" functions have been moved
> > inside fscrypt and fsverity sources. With these in place, the read
> > callbacks code now has to just invoke enqueue functions provided by
> > fscrypt and fsverity.
> >
> > Signed-off-by: Chandan Rajendra
> > ---
> > fs/Kconfig | 4 +
> > fs/Makefile| 4 +
> > fs/crypto/Kconfig | 1 +
> > fs/crypto/bio.c| 23 ++---
> > fs/crypto/crypto.c | 17 +--
> > fs/crypto/fscrypt_private.h| 3 +
> > fs/ext4/ext4.h | 2 -
> > fs/ext4/readpage.c | 183 +
> > fs/ext4/super.c| 9 +-
> > fs/f2fs/data.c | 148 --
> > fs/f2fs/super.c| 9 +-
> > fs/read_callbacks.c| 136
> > fs/verity/Kconfig | 1 +
> > fs/verity/verify.c | 12 +++
> > include/linux/fscrypt.h| 20 +---
> > include/linux/read_callbacks.h | 21
> > 16 files changed, 251 insertions(+), 342 deletions(-)
> > create mode 100644 fs/read_callbacks.c
> > create mode 100644 include/linux/read_callbacks.h
> >
> > diff --git a/fs/Kconfig b/fs/Kconfig
> > index 97f9eb8df713..03084f2dbeaf 100644
> > --- a/fs/Kconfig
> > +++ b/fs/Kconfig
> > @@ -308,6 +308,10 @@ config NFS_COMMON
> > depends on NFSD || NFS_FS || LOCKD
> > default y
> >
> > +config FS_READ_CALLBACKS
> > + bool
> > + default n
> > +
> > source "net/sunrpc/Kconfig"
> > source "fs/ceph/Kconfig"
> > source "fs/cifs/Kconfig"
> > diff --git a/fs/Makefile b/fs/Makefile
> > index 9dd2186e74b5..e0c0fce8cf40 100644
> > --- a/fs/Makefile
> > +++ b/fs/Makefile
> > @@ -21,6 +21,10 @@ else
> > obj-y += no-block.o
> > endif
> >
> > +ifeq ($(CONFIG_FS_READ_CALLBACKS),y)
> > +obj-y += read_callbacks.o
> > +endif
> > +
> > obj-$(CONFIG_PROC_FS) += proc_namespace.o
> >
> > obj-y += notify/
> > diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
> > index f0de238000c0..163c328bcbd4 100644
> > --- a/fs/crypto/Kconfig
> > +++ b/fs/crypto/Kconfig
> > @@ -8,6 +8,7 @@ config FS_ENCRYPTION
> > select CRYPTO_CTS
> > select CRYPTO_SHA256
> > select KEYS
> > + select FS_READ_CALLBACKS
> > help
> > Enable encryption of files and directories. This
> > feature is similar to ecryptfs, but it is more memory
> > diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
> > index 5759bcd018cd..27f5618174f2 100644
> > --- a/fs/crypto/bio.c
> > +++ b/fs/crypto/bio.c
> > @@ -24,6 +24,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +
> > #include "fscrypt_private.h"
> >
> > static void __fscrypt_decrypt_bio(struct bio *bio, bool done)
> > @@ -54,24 +56,15 @@ void fscrypt_decrypt_bio(struct bio *bio)
> > }
> > EXPORT_SYMBOL(fscrypt_decrypt_bio);
> >
> > -static void completion_pages(struct work_struct *work)
> > +void fscrypt_decrypt_work(struct work_struct *work)
> > {
> > - struct fscrypt_ctx *ctx =
> > - container_of(work, struct fscrypt_ctx, r.work);
> > - struct bio *bio = ctx->r.bio;
> > + struct read_callbacks_ctx *ctx =
> > + container_of(work, struct read_callbacks_ctx, work);
> >
> > - __fscrypt_decrypt_bio(bio, true);
> > - fscrypt_release_ctx(ctx);
> > - bio_put(bio);
> > -}
> > + fscrypt_decrypt_bio(ctx->bio);
> >
> > -void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx, struct bio *bio)
> > -{
> > - INIT_WORK(>r.work, completion_pages);
> > - ctx->r.bio = bio;
> > - fscrypt_enqueue_decrypt_work(>r.work);
> > + read_callbacks(ctx);
> > }
> > -EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
> >
> > void fscrypt_pullback_bio_page(struct page **page, bool restore)
> > {
> > @@ -87,7 +80,7 @@ void fscrypt_pullback_bio_page(struct page **page, bool
> > restore)
> > ctx = (struct fscrypt_ctx *)page_private(bounce_page);
> >
> > /* restore control page */
> > - *page = ctx->w.control_page;
> > + *page = ctx->control_page;
> >
> > if (restore)
> > fscrypt_restore_control_page(bounce_page);
> > diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> > index 3fc84bf2b1e5..ffa9302a7351 100644
> > --- a/fs/crypto/crypto.c
> > +++ b/fs/crypto/crypto.c
> > @@ -53,6 +53,7 @@ struct kmem_cache *fscrypt_info_cachep;
> >
> > void fscrypt_enqueue_decrypt_work(struct work_struct *work)
> > {
> > + INIT_WORK(work, fscrypt_decrypt_work);
> > queue_work(fscrypt_read_workqueue, work);
> > }
> > EXPORT_SYMBOL(fscrypt_enqueue_decrypt_work);
> > @@ -70,11 +71,11 @@ void
Re: [f2fs-dev] [PATCH V2 02/13] Consolidate "read callbacks" into a new file
On Tuesday, April 30, 2019 5:30:28 AM IST Eric Biggers wrote:
> Hi Chandan,
>
> On Sun, Apr 28, 2019 at 10:01:10AM +0530, Chandan Rajendra wrote:
> > The "read callbacks" code is used by both Ext4 and F2FS. Hence to
> > remove duplicity, this commit moves the code into
> > include/linux/read_callbacks.h and fs/read_callbacks.c.
> >
> > The corresponding decrypt and verity "work" functions have been moved
> > inside fscrypt and fsverity sources. With these in place, the read
> > callbacks code now has to just invoke enqueue functions provided by
> > fscrypt and fsverity.
> >
> > Signed-off-by: Chandan Rajendra
> > ---
> > fs/Kconfig | 4 +
> > fs/Makefile| 4 +
> > fs/crypto/Kconfig | 1 +
> > fs/crypto/bio.c| 23 ++---
> > fs/crypto/crypto.c | 17 +--
> > fs/crypto/fscrypt_private.h| 3 +
> > fs/ext4/ext4.h | 2 -
> > fs/ext4/readpage.c | 183 +
> > fs/ext4/super.c| 9 +-
> > fs/f2fs/data.c | 148 --
> > fs/f2fs/super.c| 9 +-
> > fs/read_callbacks.c| 136
> > fs/verity/Kconfig | 1 +
> > fs/verity/verify.c | 12 +++
> > include/linux/fscrypt.h| 20 +---
> > include/linux/read_callbacks.h | 21
> > 16 files changed, 251 insertions(+), 342 deletions(-)
> > create mode 100644 fs/read_callbacks.c
> > create mode 100644 include/linux/read_callbacks.h
> >
>
> For easier review, can you split this into multiple patches? Ideally the ext4
> and f2fs patches would be separate, but if that's truly not possible due to
> interdependencies it seems you could at least do:
>
> 1. Introduce the read_callbacks.
> 2. Convert encryption to use the read_callbacks.
> 3. Remove union from struct fscrypt_context.
>
> Also: just FYI, fs-verity isn't upstream yet, and in the past few months I
> haven't had much time to work on it. So you might consider arranging your
> series so that initially just fscrypt is supported. That will be useful on
> its
> own, for block_size < PAGE_SIZE support. Then fsverity can be added later.
>
> > diff --git a/fs/Kconfig b/fs/Kconfig
> > index 97f9eb8df713..03084f2dbeaf 100644
> > --- a/fs/Kconfig
> > +++ b/fs/Kconfig
> > @@ -308,6 +308,10 @@ config NFS_COMMON
> > depends on NFSD || NFS_FS || LOCKD
> > default y
> >
> > +config FS_READ_CALLBACKS
> > + bool
> > + default n
>
> 'default n' is unnecesary, since 'n' is already the default.
>
> > +
> > source "net/sunrpc/Kconfig"
> > source "fs/ceph/Kconfig"
> > source "fs/cifs/Kconfig"
> > diff --git a/fs/Makefile b/fs/Makefile
> > index 9dd2186e74b5..e0c0fce8cf40 100644
> > --- a/fs/Makefile
> > +++ b/fs/Makefile
> > @@ -21,6 +21,10 @@ else
> > obj-y += no-block.o
> > endif
> >
> > +ifeq ($(CONFIG_FS_READ_CALLBACKS),y)
> > +obj-y += read_callbacks.o
> > +endif
> > +
>
> This can be simplified to:
>
> obj-$(CONFIG_FS_READ_CALLBACKS) += read_callbacks.o
>
> > diff --git a/fs/read_callbacks.c b/fs/read_callbacks.c
> > new file mode 100644
> > index ..b6d5b95e67d7
> > --- /dev/null
> > +++ b/fs/read_callbacks.c
> > @@ -0,0 +1,136 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * This file tracks the state machine that needs to be executed after
> > reading
> > + * data from files that are encrypted and/or have verity metadata
> > associated
> > + * with them.
> > + */
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define NUM_PREALLOC_POST_READ_CTXS128
> > +
> > +static struct kmem_cache *read_callbacks_ctx_cache;
> > +static mempool_t *read_callbacks_ctx_pool;
> > +
> > +/* Read callback state machine steps */
> > +enum read_callbacks_step {
> > + STEP_INITIAL = 0,
> > + STEP_DECRYPT,
> > + STEP_VERITY,
> > +};
> > +
> > +void end_read_callbacks(struct bio *bio)
> > +{
> > + struct page *page;
> > + struct bio_vec *bv;
> > + int i;
> > + struct bvec_iter_all iter_all;
> > +
> > + bio_for_each_segment_all(bv, bio, i, iter_all) {
> > + page = bv->bv_page;
> > +
> > + BUG_ON(bio->bi_status);
> > +
> > + if (!PageError(page))
> > + SetPageUptodate(page);
> > +
> > + unlock_page(page);
> > + }
> > + if (bio->bi_private)
> > + put_read_callbacks_ctx(bio->bi_private);
> > + bio_put(bio);
> > +}
> > +EXPORT_SYMBOL(end_read_callbacks);
>
> end_read_callbacks() is only called by read_callbacks() just below, so it
> should
> be 'static'.
>
> > +
> > +struct read_callbacks_ctx *get_read_callbacks_ctx(struct inode *inode,
> > + struct bio *bio,
> > + pgoff_t index)
> > +{
> > + unsigned int read_callbacks_steps = 0;
>
> Rename
Re: [f2fs-dev] [PATCH V2 07/13] Add decryption support for sub-pagesized blocks
Hi Eric,
On Tuesday, April 30, 2019 6:08:18 AM IST Eric Biggers wrote:
> On Sun, Apr 28, 2019 at 10:01:15AM +0530, Chandan Rajendra wrote:
> > To support decryption of sub-pagesized blocks this commit adds code to,
> > 1. Track buffer head in "struct read_callbacks_ctx".
> > 2. Pass buffer head argument to all read callbacks.
> > 3. In the corresponding endio, loop across all the blocks mapped by the
> >page, decrypting each block in turn.
> >
> > Signed-off-by: Chandan Rajendra
> > ---
> > fs/buffer.c| 83 +-
> > fs/crypto/bio.c| 50 +---
> > fs/crypto/crypto.c | 19 +++-
> > fs/f2fs/data.c | 2 +-
> > fs/mpage.c | 2 +-
> > fs/read_callbacks.c| 53 ++
> > include/linux/buffer_head.h| 1 +
> > include/linux/read_callbacks.h | 5 +-
> > 8 files changed, 154 insertions(+), 61 deletions(-)
> >
> > diff --git a/fs/buffer.c b/fs/buffer.c
> > index ce357602f471..f324727e24bb 100644
> > --- a/fs/buffer.c
> > +++ b/fs/buffer.c
> > @@ -45,6 +45,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > static int fsync_buffers_list(spinlock_t *lock, struct list_head *list);
> > @@ -245,11 +246,7 @@ __find_get_block_slow(struct block_device *bdev,
> > sector_t block)
> > return ret;
> > }
> >
> > -/*
> > - * I/O completion handler for block_read_full_page() - pages
> > - * which come unlocked at the end of I/O.
> > - */
> > -static void end_buffer_async_read(struct buffer_head *bh, int uptodate)
> > +void end_buffer_page_read(struct buffer_head *bh)
>
> I think __end_buffer_async_read() would be a better name, since the *page*
> isn't
> necessarily done yet.
>
> > {
> > unsigned long flags;
> > struct buffer_head *first;
> > @@ -257,17 +254,7 @@ static void end_buffer_async_read(struct buffer_head
> > *bh, int uptodate)
> > struct page *page;
> > int page_uptodate = 1;
> >
> > - BUG_ON(!buffer_async_read(bh));
> > -
> > page = bh->b_page;
> > - if (uptodate) {
> > - set_buffer_uptodate(bh);
> > - } else {
> > - clear_buffer_uptodate(bh);
> > - buffer_io_error(bh, ", async page read");
> > - SetPageError(page);
> > - }
> > -
> > /*
> > * Be _very_ careful from here on. Bad things can happen if
> > * two buffer heads end IO at almost the same time and both
> > @@ -305,6 +292,44 @@ static void end_buffer_async_read(struct buffer_head
> > *bh, int uptodate)
> > local_irq_restore(flags);
> > return;
> > }
> > +EXPORT_SYMBOL(end_buffer_page_read);
>
> No need for EXPORT_SYMBOL() here, as this is only called by built-in code.
>
> > +
> > +/*
> > + * I/O completion handler for block_read_full_page() - pages
> > + * which come unlocked at the end of I/O.
> > + */
>
> This comment is no longer correct. Change to something like:
>
> /*
> * I/O completion handler for block_read_full_page(). Pages are unlocked
> after
> * the I/O completes and the read callbacks (if any) have executed.
> */
>
> > +static void end_buffer_async_read(struct buffer_head *bh, int uptodate)
> > +{
> > + struct page *page;
> > +
> > + BUG_ON(!buffer_async_read(bh));
> > +
> > +#if defined(CONFIG_FS_ENCRYPTION) || defined(CONFIG_FS_VERITY)
> > + if (uptodate && bh->b_private) {
> > + struct read_callbacks_ctx *ctx = bh->b_private;
> > +
> > + read_callbacks(ctx);
> > + return;
> > + }
> > +
> > + if (bh->b_private) {
> > + struct read_callbacks_ctx *ctx = bh->b_private;
> > +
> > + WARN_ON(uptodate);
> > + put_read_callbacks_ctx(ctx);
> > + }
> > +#endif
>
> These details should be handled in read_callbacks code, not here. AFAICS, all
> you need is a function read_callbacks_end_bh() that returns a bool indicating
> whether it handled the buffer_head or not:
>
> static void end_buffer_async_read(struct buffer_head *bh, int uptodate)
> {
> BUG_ON(!buffer_async_read(bh));
>
> if (read_callbacks_end_bh(bh, uptodate))
> return;
>
> page = bh->b_page;
> ...
> }
>
> Then read_callbacks_end_bh() would check ->b_private and uptodate, and call
> read_callbacks() or put_read_callbacks_ctx() as appropriate. When
> CONFIG_FS_READ_CALLBACKS=n it would be a stub that always returns false.
>
> > + page = bh->b_page;
> [...]
>
> > }
> > @@ -2292,11 +2323,21 @@ int block_read_full_page(struct page *page,
> > get_block_t *get_block)
> > * the underlying blockdev brought it uptodate (the sct fix).
> > */
> > for (i = 0; i < nr; i++) {
> > - bh = arr[i];
> > - if (buffer_uptodate(bh))
> > + bh = arr[i].bh;
> > + if (buffer_uptodate(bh)) {
> > end_buffer_async_read(bh, 1);
> > - else
> > + } else {
> > +#if
Re: [f2fs-dev] [PATCH V2 03/13] fsverity: Add call back to decide if verity check has to be performed
On Wednesday, May 1, 2019 2:40:38 AM IST Jeremy Sowden wrote:
> On 2019-04-28, at 10:01:11 +0530, Chandan Rajendra wrote:
> > Ext4 and F2FS store verity metadata in data extents (beyond
> > inode->i_size) associated with a file. But other filesystems might
> > choose alternative means to store verity metadata. Hence this commit
> > adds a callback function pointer to 'struct fsverity_operations' to
> > help in deciding if verity operation needs to performed against a
> > page-cache page holding file data.
> >
> > Signed-off-by: Chandan Rajendra
> > ---
> > fs/ext4/super.c | 6 ++
> > fs/f2fs/super.c | 6 ++
> > fs/read_callbacks.c | 4 +++-
> > include/linux/fsverity.h | 1 +
> > 4 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > index aba724f82cc3..63d73b360f1d 100644
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> > @@ -1428,10 +1428,16 @@ static struct page
> > *ext4_read_verity_metadata_page(struct inode *inode,
> > return read_mapping_page(inode->i_mapping, index, NULL);
> > }
> >
> > +static bool ext4_verity_required(struct inode *inode, pgoff_t index)
> > +{
> > + return index < (i_size_read(inode) + PAGE_SIZE - 1) >> PAGE_SHIFT;
> > +}
> > +
> > static const struct fsverity_operations ext4_verityops = {
> > .set_verity = ext4_set_verity,
> > .get_metadata_end = ext4_get_verity_metadata_end,
> > .read_metadata_page = ext4_read_verity_metadata_page,
> > + .verity_required= ext4_verity_required,
> > };
> > #endif /* CONFIG_FS_VERITY */
> >
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index 2f75f06c784a..cd1299e1f92d 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -2257,10 +2257,16 @@ static struct page
> > *f2fs_read_verity_metadata_page(struct inode *inode,
> > return read_mapping_page(inode->i_mapping, index, NULL);
> > }
> >
> > +static bool f2fs_verity_required(struct inode *inode, pgoff_t index)
> > +{
> > + return index < (i_size_read(inode) + PAGE_SIZE - 1) >> PAGE_SHIFT;
> > +}
> > +
> > static const struct fsverity_operations f2fs_verityops = {
> > .set_verity = f2fs_set_verity,
> > .get_metadata_end = f2fs_get_verity_metadata_end,
> > .read_metadata_page = f2fs_read_verity_metadata_page,
> > + .verity_required= f2fs_verity_required,
> > };
> > #endif /* CONFIG_FS_VERITY */
> >
> > diff --git a/fs/read_callbacks.c b/fs/read_callbacks.c
> > index b6d5b95e67d7..6dea54b0baa9 100644
> > --- a/fs/read_callbacks.c
> > +++ b/fs/read_callbacks.c
> > @@ -86,7 +86,9 @@ struct read_callbacks_ctx *get_read_callbacks_ctx(struct
> > inode *inode,
> > read_callbacks_steps |= 1 << STEP_DECRYPT;
> > #ifdef CONFIG_FS_VERITY
> > if (inode->i_verity_info != NULL &&
> > - (index < ((i_size_read(inode) + PAGE_SIZE - 1) >> PAGE_SHIFT)))
> > + ((inode->i_sb->s_vop->verity_required
> > + && inode->i_sb->s_vop->verity_required(inode, index))
> > + || (inode->i_sb->s_vop->verity_required == NULL)))
>
> I think this is a bit easier to follow:
>
> (inode->i_sb->s_vop->verity_required == NULL ||
> inode->i_sb->s_vop->verity_required(inode, index)))
Yes, you are right. I will implement the changes suggested by you.
--
chandan
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V2 02/13] Consolidate "read callbacks" into a new file
On Tuesday, April 30, 2019 11:35:08 PM IST Eric Biggers wrote: > On Sun, Apr 28, 2019 at 10:01:10AM +0530, Chandan Rajendra wrote: > > The "read callbacks" code is used by both Ext4 and F2FS. Hence to > > remove duplicity, this commit moves the code into > > include/linux/read_callbacks.h and fs/read_callbacks.c. > > > > The corresponding decrypt and verity "work" functions have been moved > > inside fscrypt and fsverity sources. With these in place, the read > > callbacks code now has to just invoke enqueue functions provided by > > fscrypt and fsverity. > > > > Signed-off-by: Chandan Rajendra > > --- > > fs/Kconfig | 4 + > > fs/Makefile| 4 + > > fs/crypto/Kconfig | 1 + > > fs/crypto/bio.c| 23 ++--- > > fs/crypto/crypto.c | 17 +-- > > fs/crypto/fscrypt_private.h| 3 + > > fs/ext4/ext4.h | 2 - > > fs/ext4/readpage.c | 183 + > > fs/ext4/super.c| 9 +- > > fs/f2fs/data.c | 148 -- > > fs/f2fs/super.c| 9 +- > > fs/read_callbacks.c| 136 > > fs/verity/Kconfig | 1 + > > fs/verity/verify.c | 12 +++ > > include/linux/fscrypt.h| 20 +--- > > include/linux/read_callbacks.h | 21 > > 16 files changed, 251 insertions(+), 342 deletions(-) > > create mode 100644 fs/read_callbacks.c > > create mode 100644 include/linux/read_callbacks.h > > > > diff --git a/fs/Kconfig b/fs/Kconfig > > index 97f9eb8df713..03084f2dbeaf 100644 > > --- a/fs/Kconfig > > +++ b/fs/Kconfig > > @@ -308,6 +308,10 @@ config NFS_COMMON > > depends on NFSD || NFS_FS || LOCKD > > default y > > > > +config FS_READ_CALLBACKS > > + bool > > + default n > > + > > source "net/sunrpc/Kconfig" > > source "fs/ceph/Kconfig" > > source "fs/cifs/Kconfig" > > This shouldn't be under the 'if NETWORK_FILESYSTEMS' block, since it has > nothing > to do with network filesystems. When trying to compile this I got: > > WARNING: unmet direct dependencies detected for FS_READ_CALLBACKS > Depends on [n]: NETWORK_FILESYSTEMS [=n] > Selected by [y]: > - FS_ENCRYPTION [=y] > - FS_VERITY [=y] > > Perhaps put it just below FS_IOMAP? > > > diff --git a/fs/Makefile b/fs/Makefile > > index 9dd2186e74b5..e0c0fce8cf40 100644 > > --- a/fs/Makefile > > +++ b/fs/Makefile > > @@ -21,6 +21,10 @@ else > > obj-y += no-block.o > > endif > > > > +ifeq ($(CONFIG_FS_READ_CALLBACKS),y) > > +obj-y += read_callbacks.o > > +endif > > + > > obj-$(CONFIG_PROC_FS) += proc_namespace.o > > > > obj-y += notify/ > > diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig > > index f0de238000c0..163c328bcbd4 100644 > > --- a/fs/crypto/Kconfig > > +++ b/fs/crypto/Kconfig > > @@ -8,6 +8,7 @@ config FS_ENCRYPTION > > select CRYPTO_CTS > > select CRYPTO_SHA256 > > select KEYS > > + select FS_READ_CALLBACKS > > help > > Enable encryption of files and directories. This > > feature is similar to ecryptfs, but it is more memory > > This selection needs to be conditional on BLOCK. > > select FS_READ_CALLBACKS if BLOCK > > Otherwise, building without BLOCK and with UBIFS encryption support fails. > > fs/read_callbacks.c: In function ‘end_read_callbacks’: > fs/read_callbacks.c:34:23: error: storage size of ‘iter_all’ isn’t known > struct bvec_iter_all iter_all; > ^~~~ > fs/read_callbacks.c:37:20: error: dereferencing pointer to incomplete > type ‘struct buffer_head’ > if (!PageError(bh->b_page)) > > [...] > I will fix this in the next version of this patchset. -- chandan ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V2 12/13] fscrypt_zeroout_range: Encrypt all zeroed out blocks of a page
On Tuesday, April 30, 2019 10:21:15 PM IST Eric Biggers wrote:
> On Sun, Apr 28, 2019 at 10:01:20AM +0530, Chandan Rajendra wrote:
> > For subpage-sized blocks, this commit adds code to encrypt all zeroed
> > out blocks mapped by a page.
> >
> > Signed-off-by: Chandan Rajendra
> > ---
> > fs/crypto/bio.c | 40 ++--
> > 1 file changed, 18 insertions(+), 22 deletions(-)
> >
> > diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
> > index 856f4694902d..46dd2ec50c7d 100644
> > --- a/fs/crypto/bio.c
> > +++ b/fs/crypto/bio.c
> > @@ -108,29 +108,23 @@ EXPORT_SYMBOL(fscrypt_pullback_bio_page);
> > int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
> > sector_t pblk, unsigned int len)
> > {
> > - struct fscrypt_ctx *ctx;
> > struct page *ciphertext_page = NULL;
> > struct bio *bio;
> > + u64 total_bytes, page_bytes;
>
> page_bytes should be 'unsigned int', since it's <= PAGE_SIZE.
>
> > int ret, err = 0;
> >
> > - BUG_ON(inode->i_sb->s_blocksize != PAGE_SIZE);
> > -
> > - ctx = fscrypt_get_ctx(inode, GFP_NOFS);
> > - if (IS_ERR(ctx))
> > - return PTR_ERR(ctx);
> > + total_bytes = len << inode->i_blkbits;
>
> Should cast len to 'u64' here, in case it's greater than UINT_MAX / blocksize.
>
> >
> > - ciphertext_page = fscrypt_alloc_bounce_page(ctx, GFP_NOWAIT);
> > - if (IS_ERR(ciphertext_page)) {
> > - err = PTR_ERR(ciphertext_page);
> > - goto errout;
> > - }
> > + while (total_bytes) {
> > + page_bytes = min_t(u64, total_bytes, PAGE_SIZE);
> >
> > - while (len--) {
> > - err = fscrypt_do_page_crypto(inode, FS_ENCRYPT, lblk,
> > -ZERO_PAGE(0), ciphertext_page,
> > -PAGE_SIZE, 0, GFP_NOFS);
> > - if (err)
> > + ciphertext_page = fscrypt_encrypt_page(inode, ZERO_PAGE(0),
> > + page_bytes, 0, lblk, GFP_NOFS);
> > + if (IS_ERR(ciphertext_page)) {
> > + err = PTR_ERR(ciphertext_page);
> > + ciphertext_page = NULL;
> > goto errout;
> > + }
>
> 'ciphertext_page' is leaked after each loop iteration. Did you mean to free
> it,
> or did you mean to reuse it for subsequent iterations?
Thanks for pointing this out. I actually meant to free it. I will see if I can
reuse ciphertext_page in my next patchset rather than freeing and allocating
it each time the loop is executed.
>
> >
> > bio = bio_alloc(GFP_NOWAIT, 1);
> > if (!bio) {
> > @@ -141,9 +135,8 @@ int fscrypt_zeroout_range(const struct inode *inode,
> > pgoff_t lblk,
> > bio->bi_iter.bi_sector =
> > pblk << (inode->i_sb->s_blocksize_bits - 9);
>
> This line uses ->s_blocksize_bits, but your new code uses ->i_blkbits. AFAIK
> they'll always be the same, but please pick one or the other to use.
I will fix this.
>
> > bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
> > - ret = bio_add_page(bio, ciphertext_page,
> > - inode->i_sb->s_blocksize, 0);
> > - if (ret != inode->i_sb->s_blocksize) {
> > + ret = bio_add_page(bio, ciphertext_page, page_bytes, 0);
> > + if (ret != page_bytes) {
> > /* should never happen! */
> > WARN_ON(1);
> > bio_put(bio);
> > @@ -156,12 +149,15 @@ int fscrypt_zeroout_range(const struct inode *inode,
> > pgoff_t lblk,
> > bio_put(bio);
> > if (err)
> > goto errout;
> > - lblk++;
> > - pblk++;
> > +
> > + lblk += page_bytes >> inode->i_blkbits;
> > + pblk += page_bytes >> inode->i_blkbits;
> > + total_bytes -= page_bytes;
> > }
> > err = 0;
> > errout:
> > - fscrypt_release_ctx(ctx);
> > + if (!IS_ERR_OR_NULL(ciphertext_page))
> > + fscrypt_restore_control_page(ciphertext_page);
> > return err;
> > }
> > EXPORT_SYMBOL(fscrypt_zeroout_range);
>
>
--
chandan
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [f2fs:dev 37/39] include/trace/events/f2fs.h:1287:1: sparse: sparse: cast from restricted vm_fault_t
tree: https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev
head: f8484026e829f44d3836054534c59bf7937ba864
commit: c632dd40b9cf038245195e9e54db4a816088ccb5 [37/39] f2fs: add tracepoint
for f2fs_filemap_fault()
reproduce:
# apt-get install sparse
git checkout c632dd40b9cf038245195e9e54db4a816088ccb5
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
sparse warnings: (new ones prefixed by >>)
>> include/trace/events/f2fs.h:1287:1: sparse: sparse: cast from restricted
>> vm_fault_t
include/trace/events/f2fs.h:1287:1: sparse: sparse: cast to restricted
vm_fault_t
include/trace/events/f2fs.h:1287:1: sparse: sparse: cast to restricted
vm_fault_t
include/trace/events/f2fs.h:1287:1: sparse: sparse: restricted vm_fault_t
degrades to integer
include/trace/events/f2fs.h:1287:1: sparse: sparse: restricted vm_fault_t
degrades to integer
include/asm-generic/atomic-instrumented.h:239:26: sparse: sparse: context
imbalance in 'f2fs_drop_inode' - unexpected unlock
vim +1287 include/trace/events/f2fs.h
1286
> 1287 TRACE_EVENT(f2fs_filemap_fault,
1288
1289 TP_PROTO(struct inode *inode, pgoff_t index, vm_fault_t ret),
1290
1291 TP_ARGS(inode, index, ret),
1292
1293 TP_STRUCT__entry(
1294 __field(dev_t, dev)
1295 __field(ino_t, ino)
1296 __field(pgoff_t, index)
1297 __field(vm_fault_t, ret)
1298 ),
1299
1300 TP_fast_assign(
1301 __entry->dev= inode->i_sb->s_dev;
1302 __entry->ino= inode->i_ino;
1303 __entry->index = index;
1304 __entry->ret= ret;
1305 ),
1306
1307 TP_printk("dev = (%d,%d), ino = %lu, index = %lu, ret = %lx",
1308 show_dev_ino(__entry),
1309 (unsigned long)__entry->index,
1310 (unsigned long)__entry->ret)
1311 );
1312
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
