lkml.org 
[lkml]   [2016]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 03/29] fscrypt: Enable partial page encryption
On Sun, Nov 13, 2016 at 10:20:46PM +0100, Richard Weinberger wrote:
> From: David Gstir <david@sigma-star.at>
>
> Not all filesystems work on full pages, thus we should allow them to
> hand partial pages to fscrypt for en/decryption.
>
> Signed-off-by: David Gstir <david@sigma-star.at>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
> fs/crypto/crypto.c | 42 ++++++++++++++++++++++++++----------------
> fs/ext4/inode.c | 6 ++++--
> fs/ext4/page-io.c | 2 +-
> fs/f2fs/data.c | 2 ++
> include/linux/fscrypto.h | 16 +++++++++++-----
> 5 files changed, 44 insertions(+), 24 deletions(-)
>
> diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> index 222a70520565..e170aa05011d 100644
> --- a/fs/crypto/crypto.c
> +++ b/fs/crypto/crypto.c
> @@ -149,6 +149,7 @@ typedef enum {
> static int do_page_crypto(struct inode *inode,
> fscrypt_direction_t rw, pgoff_t index,
> struct page *src_page, struct page *dest_page,
> + unsigned int src_len, unsigned int src_offset,
> gfp_t gfp_flags)

The naming of 'src_len' and 'src_offset', and 'plaintext_len' and
'plaintext_offset' below, is misleading because the length and offset actually
apply to the destination too. Shouldn't they be 'len' and 'offset', or 'len'
and 'offs' like fscrypt_decrypt_page()?

I'm also a little concerned that users will mix up the src_len and src_offset
arguments and end up "encrypting" 0 bytes at offset PAGE_SIZE. Adding a
'BUG_ON(len == 0)' may be appropriate.

> /**
> * fscypt_encrypt_page() - Encrypts a page
> - * @inode: The inode for which the encryption should take place
> - * @plaintext_page: The page to encrypt. Must be locked.
> - * @gfp_flags: The gfp flag for memory allocation
> + * @inode: The inode for which the encryption should take place
> + * @plaintext_page: The page to encrypt. Must be locked.
> + * @plaintext_len: Length of plaintext within page
> + * @plaintext_offset: Offset of plaintext within page
> + * @gfp_flags: The gfp flag for memory allocation
> *
> * Encrypts plaintext_page using the ctx encryption context. If
> * the filesystem supports it, encryption is performed in-place, otherwise a
> @@ -229,13 +232,17 @@ static struct page *alloc_bounce_page(struct fscrypt_ctx *ctx, gfp_t gfp_flags)
> * error value or NULL.
> */
> struct page *fscrypt_encrypt_page(struct inode *inode,
> - struct page *plaintext_page, gfp_t gfp_flags)
> + struct page *plaintext_page,
> + unsigned int plaintext_len,
> + unsigned int plaintext_offset,
> + gfp_t gfp_flags)
> +
> {
> struct fscrypt_ctx *ctx;
> struct page *ciphertext_page = plaintext_page;
> int err;
>
> - BUG_ON(!PageLocked(plaintext_page));
> + BUG_ON(plaintext_len % FS_CRYPTO_BLOCK_SIZE != 0);

What is going on with PageLocked()? Is it still a requirement? If not the
function comment needs to be fixed.

> -int fscrypt_decrypt_page(struct inode *inode, struct page *page)
> +int fscrypt_decrypt_page(struct inode *inode, struct page *page,
> + unsigned int len, unsigned int offs)
> {
> - BUG_ON(!PageLocked(page));
> -
> - return do_page_crypto(inode, FS_DECRYPT, page->index, page, page,
> + return do_page_crypto(inode, FS_DECRYPT, page->index, page, page, len, offs,
> GFP_NOFS);
> }

Same with PageLocked(). Is it still a requirement? If not the function comment
needs to be fixed.

Eric

\
 
 \ /
  Last update: 2016-11-15 19:32    [W:0.333 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site