2016-12-01 22:25:50

by Richard Weinberger

[permalink] [raw]
Subject: [PATCH 0/6] UBIFS related fscrypt updates

This series applies on top of Ted's fscrypt tree[0] addresses the review
comments from Eric.
Ted, it would be awesome to have this patches in the v4.10 merge window.

[0] git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git fscrypt

David Gstir (6):
fscrypt: Use correct index in decrypt path.
fscrypt: Release fscrypt context on in-place encryption
fscrypt: Cleanup fscrypt_{decrypt,encrypt}_page()
fscrypt: Cleanup page locking requirements for
fscrypt_{decrypt,encrypt}_page()
fscrypt: Deplay bounce page pool allocation until needed
fscrypt: Rename FS_WRITE_PATH_FL to FS_CTX_HAS_BOUNCE_BUFFER_FL

fs/crypto/crypto.c | 107 ++++++++++++++++++++++++++++-------------------
fs/crypto/keyinfo.c | 2 +-
fs/ext4/inode.c | 1 -
fs/f2fs/data.c | 1 -
fs/ubifs/crypto.c | 2 +-
include/linux/fscrypto.h | 14 +++----
6 files changed, 74 insertions(+), 53 deletions(-)

--
2.7.3


2016-12-01 22:17:23

by Richard Weinberger

[permalink] [raw]
Subject: [PATCH 6/6] fscrypt: Rename FS_WRITE_PATH_FL to FS_CTX_HAS_BOUNCE_BUFFER_FL

From: David Gstir <[email protected]>

... to better explain its purpose after introducing in-place encryption
without bounce buffer.

Signed-off-by: David Gstir <[email protected]>
Signed-off-by: Richard Weinberger <[email protected]>
---
fs/crypto/crypto.c | 6 +++---
include/linux/fscrypto.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index d694a3a93eb3..2469721f6130 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -63,7 +63,7 @@ void fscrypt_release_ctx(struct fscrypt_ctx *ctx)
{
unsigned long flags;

- if (ctx->flags & FS_WRITE_PATH_FL && ctx->w.bounce_page) {
+ 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;
}
@@ -121,7 +121,7 @@ struct fscrypt_ctx *fscrypt_get_ctx(const struct inode *inode, gfp_t gfp_flags)
} else {
ctx->flags &= ~FS_CTX_REQUIRES_FREE_ENCRYPT_FL;
}
- ctx->flags &= ~FS_WRITE_PATH_FL;
+ ctx->flags &= ~FS_CTX_HAS_BOUNCE_BUFFER_FL;
return ctx;
}
EXPORT_SYMBOL(fscrypt_get_ctx);
@@ -210,7 +210,7 @@ static struct page *alloc_bounce_page(struct fscrypt_ctx *ctx, gfp_t gfp_flags)
ctx->w.bounce_page = mempool_alloc(fscrypt_bounce_page_pool, gfp_flags);
if (ctx->w.bounce_page == NULL)
return ERR_PTR(-ENOMEM);
- ctx->flags |= FS_WRITE_PATH_FL;
+ ctx->flags |= FS_CTX_HAS_BOUNCE_BUFFER_FL;
return ctx->w.bounce_page;
}

diff --git a/include/linux/fscrypto.h b/include/linux/fscrypto.h
index 6f5c7e8fbd55..097ce5ccd08b 100644
--- a/include/linux/fscrypto.h
+++ b/include/linux/fscrypto.h
@@ -84,7 +84,7 @@ struct fscrypt_info {
};

#define FS_CTX_REQUIRES_FREE_ENCRYPT_FL 0x00000001
-#define FS_WRITE_PATH_FL 0x00000002
+#define FS_CTX_HAS_BOUNCE_BUFFER_FL 0x00000002

struct fscrypt_ctx {
union {
--
2.7.3

2016-12-01 22:17:22

by Richard Weinberger

[permalink] [raw]
Subject: [PATCH 5/6] fscrypt: Deplay bounce page pool allocation until needed

From: David Gstir <[email protected]>

Since fscrypt users can now indicated if fscrypt_encrypt_page() should
use a bounce page, we can delay the bounce page pool initialization util
it is really needed. That is until fscrypt_operations has no
FS_CFLG_OWN_PAGES flag set.

Signed-off-by: David Gstir <[email protected]>
Signed-off-by: Richard Weinberger <[email protected]>
---
fs/crypto/crypto.c | 7 +++++--
fs/crypto/keyinfo.c | 2 +-
include/linux/fscrypto.h | 2 +-
3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 2f41eee963e1..d694a3a93eb3 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -521,17 +521,20 @@ static void fscrypt_destroy(void)

/**
* fscrypt_initialize() - allocate major buffers for fs encryption.
+ * @cop_flags: fscrypt operations flags
*
* We only call this when we start accessing encrypted files, since it
* results in memory getting allocated that wouldn't otherwise be used.
*
* Return: Zero on success, non-zero otherwise.
*/
-int fscrypt_initialize(void)
+int fscrypt_initialize(unsigned int cop_flags)
{
int i, res = -ENOMEM;

- if (fscrypt_bounce_page_pool)
+ /* No need to allocate a bounce page pool if there already is one or
+ * this FS won't use it. */
+ if (cop_flags & FS_CFLG_OWN_PAGES || fscrypt_bounce_page_pool)
return 0;

mutex_lock(&fscrypt_init_mutex);
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 67fb6d8876d0..5951f4ebf2a9 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -188,7 +188,7 @@ int get_crypt_info(struct inode *inode)
u8 *raw_key = NULL;
int res;

- res = fscrypt_initialize();
+ res = fscrypt_initialize(inode->i_sb->s_cop->flags);
if (res)
return res;

diff --git a/include/linux/fscrypto.h b/include/linux/fscrypto.h
index 65be74f3afb6..6f5c7e8fbd55 100644
--- a/include/linux/fscrypto.h
+++ b/include/linux/fscrypto.h
@@ -244,7 +244,7 @@ static inline void fscrypt_set_d_op(struct dentry *dentry)
#if IS_ENABLED(CONFIG_FS_ENCRYPTION)
/* crypto.c */
extern struct kmem_cache *fscrypt_info_cachep;
-int fscrypt_initialize(void);
+int fscrypt_initialize(unsigned int);

extern struct fscrypt_ctx *fscrypt_get_ctx(const struct inode *, gfp_t);
extern void fscrypt_release_ctx(struct fscrypt_ctx *);
--
2.7.3

2016-12-01 22:25:48

by Richard Weinberger

[permalink] [raw]
Subject: [PATCH 2/6] fscrypt: Release fscrypt context on in-place encryption

From: David Gstir <[email protected]>

In case of in-place encryption ctx must be released right away.
Otherwise ctx is never freed.

Fixes: 1c7dcf69eea3 ("fscrypt: Add in-place encryption mode")
Signed-off-by: David Gstir <[email protected]>
Signed-off-by: Richard Weinberger <[email protected]>
---
fs/crypto/crypto.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 3c1124beae28..1b877fcec1c1 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -270,6 +270,8 @@ struct page *fscrypt_encrypt_page(const struct inode *inode,
SetPagePrivate(ciphertext_page);
set_page_private(ciphertext_page, (unsigned long)ctx);
lock_page(ciphertext_page);
+ } else {
+ fscrypt_release_ctx(ctx);
}
return ciphertext_page;

--
2.7.3

2016-12-01 22:25:54

by Richard Weinberger

[permalink] [raw]
Subject: [PATCH 3/6] fscrypt: Cleanup fscrypt_{decrypt,encrypt}_page()

From: David Gstir <[email protected]>

- Improve documentation
- Add BUG_ON(len == 0) to avoid accidental switch of offs and len
parameters
- Improve variable names for readability
- No need to set ctx->w.control_page in fscrypt_encrypt_page in case of
in-place encryption.

Signed-off-by: David Gstir <[email protected]>
Signed-off-by: Richard Weinberger <[email protected]>
---
fs/crypto/crypto.c | 83 ++++++++++++++++++++++++++++--------------------
include/linux/fscrypto.h | 8 ++---
2 files changed, 52 insertions(+), 39 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 1b877fcec1c1..3c3c854e5f77 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -147,9 +147,9 @@ typedef enum {
} fscrypt_direction_t;

static int do_page_crypto(const struct inode *inode,
- fscrypt_direction_t rw, pgoff_t index,
+ fscrypt_direction_t rw, u64 lblk_num,
struct page *src_page, struct page *dest_page,
- unsigned int src_len, unsigned int src_offset,
+ unsigned int len, unsigned int offs,
gfp_t gfp_flags)
{
struct {
@@ -163,6 +163,8 @@ static int do_page_crypto(const struct inode *inode,
struct crypto_skcipher *tfm = ci->ci_ctfm;
int res = 0;

+ BUG_ON(len == 0);
+
req = skcipher_request_alloc(tfm, gfp_flags);
if (!req) {
printk_ratelimited(KERN_ERR
@@ -176,14 +178,14 @@ static int do_page_crypto(const struct inode *inode,
page_crypt_complete, &ecr);

BUILD_BUG_ON(sizeof(xts_tweak) != FS_XTS_TWEAK_SIZE);
- xts_tweak.index = cpu_to_le64(index);
+ xts_tweak.index = cpu_to_le64(lblk_num);
memset(xts_tweak.padding, 0, sizeof(xts_tweak.padding));

sg_init_table(&dst, 1);
- sg_set_page(&dst, dest_page, src_len, src_offset);
+ sg_set_page(&dst, dest_page, len, offs);
sg_init_table(&src, 1);
- sg_set_page(&src, src_page, src_len, src_offset);
- skcipher_request_set_crypt(req, &src, &dst, src_len, &xts_tweak);
+ sg_set_page(&src, src_page, len, offs);
+ skcipher_request_set_crypt(req, &src, &dst, len, &xts_tweak);
if (rw == FS_DECRYPT)
res = crypto_skcipher_decrypt(req);
else
@@ -214,37 +216,46 @@ static struct page *alloc_bounce_page(struct fscrypt_ctx *ctx, gfp_t gfp_flags)

/**
* 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.
- * @plaintext_len: Length of plaintext within page
- * @plaintext_offset: Offset of plaintext within page
- * @index: Index for encryption. This is mainly the page index, but
- * but might be different for multiple calls on same page.
- * @gfp_flags: The gfp flag for memory allocation
+ * @inode: The inode for which the encryption should take place
+ * @page: The page to encrypt. Must be locked for bounce-page
+ * encryption.
+ * @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 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
- * new ciphertext_page is allocated and returned.
+ * Encrypts @page using the ctx encryption context. Performs encryption
+ * either in-place or into a newly allocated bounce page.
+ * Called on the page write path.
*
- * Called on the page write path. The caller must call
+ * Bounce page allocation is the default.
+ * In this case, the contents of @page are encrypted and stored in an
+ * allocated bounce page. @page has to be locked and the caller must call
* fscrypt_restore_control_page() on the returned ciphertext page to
* release the bounce buffer and the encryption context.
*
- * Return: An allocated page with the encrypted content on success. Else, an
+ * In-place encryption is used by setting the FS_CFLG_INPLACE_ENCRYPTION flag in
+ * fscrypt_operations. Here, the input-page is returned with its content
+ * encrypted.
+ *
+ * Return: A page with the encrypted content on success. Else, an
* error value or NULL.
*/
struct page *fscrypt_encrypt_page(const struct inode *inode,
- struct page *plaintext_page,
- unsigned int plaintext_len,
- unsigned int plaintext_offset,
- pgoff_t index, gfp_t gfp_flags)
+ struct page *page,
+ unsigned int len,
+ unsigned int offs,
+ u64 lblk_num, gfp_t gfp_flags)

{
struct fscrypt_ctx *ctx;
- struct page *ciphertext_page = plaintext_page;
+ struct page *ciphertext_page = page;
int err;

- BUG_ON(plaintext_len % FS_CRYPTO_BLOCK_SIZE != 0);
+ BUG_ON(len % FS_CRYPTO_BLOCK_SIZE != 0);

ctx = fscrypt_get_ctx(inode, gfp_flags);
if (IS_ERR(ctx))
@@ -255,12 +266,13 @@ struct page *fscrypt_encrypt_page(const struct inode *inode,
ciphertext_page = alloc_bounce_page(ctx, gfp_flags);
if (IS_ERR(ciphertext_page))
goto errout;
+
+ ctx->w.control_page = page;
}

- ctx->w.control_page = plaintext_page;
- err = do_page_crypto(inode, FS_ENCRYPT, index,
- plaintext_page, ciphertext_page,
- plaintext_len, plaintext_offset,
+ err = do_page_crypto(inode, FS_ENCRYPT, lblk_num,
+ page, ciphertext_page,
+ len, offs,
gfp_flags);
if (err) {
ciphertext_page = ERR_PTR(err);
@@ -283,11 +295,12 @@ EXPORT_SYMBOL(fscrypt_encrypt_page);

/**
* fscrypt_decrypt_page() - Decrypts a page in-place
- * @inode: Encrypted inode 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.
- * @index: Index for encryption.
+ * @inode: The corresponding inode for the page to decrypt.
+ * @page: The page to decrypt. Must be locked in case
+ * it is a writeback page.
+ * @len: Number of bytes in @page to be decrypted.
+ * @offs: Start of data in @page.
+ * @lblk_num: Logical block number.
*
* Decrypts page in-place using the ctx encryption context.
*
@@ -296,9 +309,9 @@ EXPORT_SYMBOL(fscrypt_encrypt_page);
* Return: Zero on success, non-zero otherwise.
*/
int fscrypt_decrypt_page(const struct inode *inode, struct page *page,
- unsigned int len, unsigned int offs, pgoff_t index)
+ unsigned int len, unsigned int offs, u64 lblk_num)
{
- return do_page_crypto(inode, FS_DECRYPT, index, page, page, len, offs,
+ return do_page_crypto(inode, FS_DECRYPT, lblk_num, page, page, len, offs,
GFP_NOFS);
}
EXPORT_SYMBOL(fscrypt_decrypt_page);
diff --git a/include/linux/fscrypto.h b/include/linux/fscrypto.h
index 98c71e973a96..e9648b62eca6 100644
--- a/include/linux/fscrypto.h
+++ b/include/linux/fscrypto.h
@@ -250,9 +250,9 @@ extern struct fscrypt_ctx *fscrypt_get_ctx(const struct inode *, gfp_t);
extern void fscrypt_release_ctx(struct fscrypt_ctx *);
extern struct page *fscrypt_encrypt_page(const struct inode *, struct page *,
unsigned int, unsigned int,
- pgoff_t, gfp_t);
+ u64, gfp_t);
extern int fscrypt_decrypt_page(const struct inode *, struct page *, unsigned int,
- unsigned int, pgoff_t);
+ unsigned int, u64);
extern void fscrypt_decrypt_bio_pages(struct fscrypt_ctx *, struct bio *);
extern void fscrypt_pullback_bio_page(struct page **, bool);
extern void fscrypt_restore_control_page(struct page *);
@@ -299,14 +299,14 @@ static inline struct page *fscrypt_notsupp_encrypt_page(const struct inode *i,
struct page *p,
unsigned int len,
unsigned int offs,
- pgoff_t index, gfp_t f)
+ u64 lblk_num, gfp_t f)
{
return ERR_PTR(-EOPNOTSUPP);
}

static inline int fscrypt_notsupp_decrypt_page(const struct inode *i, struct page *p,
unsigned int len, unsigned int offs,
- pgoff_t index)
+ u64 lblk_num)
{
return -EOPNOTSUPP;
}
--
2.7.3

2016-12-01 22:25:52

by Richard Weinberger

[permalink] [raw]
Subject: [PATCH 4/6] fscrypt: Cleanup page locking requirements for fscrypt_{decrypt,encrypt}_page()

From: David Gstir <[email protected]>

Rename the FS_CFLG_INPLACE_ENCRYPTION flag to FS_CFLG_OWN_PAGES which,
when set, indicates that the fs uses pages under its own control as
opposed to writeback pages which require locking and a bounce buffer for
encryption.

Signed-off-by: David Gstir <[email protected]>
Signed-off-by: Richard Weinberger <[email protected]>
---
fs/crypto/crypto.c | 13 +++++++++----
fs/ext4/inode.c | 1 -
fs/f2fs/data.c | 1 -
fs/ubifs/crypto.c | 2 +-
include/linux/fscrypto.h | 2 +-
5 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 3c3c854e5f77..2f41eee963e1 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -237,7 +237,7 @@ static struct page *alloc_bounce_page(struct fscrypt_ctx *ctx, gfp_t gfp_flags)
* fscrypt_restore_control_page() on the returned ciphertext page to
* release the bounce buffer and the encryption context.
*
- * In-place encryption is used by setting the FS_CFLG_INPLACE_ENCRYPTION flag in
+ * In-place encryption is used by setting the FS_CFLG_OWN_PAGES flag in
* fscrypt_operations. Here, the input-page is returned with its content
* encrypted.
*
@@ -261,7 +261,9 @@ struct page *fscrypt_encrypt_page(const struct inode *inode,
if (IS_ERR(ctx))
return (struct page *)ctx;

- if (!(inode->i_sb->s_cop->flags & FS_CFLG_INPLACE_ENCRYPTION)) {
+ if (!(inode->i_sb->s_cop->flags & FS_CFLG_OWN_PAGES)) {
+ BUG_ON(!PageLocked(page));
+
/* The encryption operation will require a bounce page. */
ciphertext_page = alloc_bounce_page(ctx, gfp_flags);
if (IS_ERR(ciphertext_page))
@@ -278,7 +280,7 @@ struct page *fscrypt_encrypt_page(const struct inode *inode,
ciphertext_page = ERR_PTR(err);
goto errout;
}
- if (!(inode->i_sb->s_cop->flags & FS_CFLG_INPLACE_ENCRYPTION)) {
+ if (!(inode->i_sb->s_cop->flags & FS_CFLG_OWN_PAGES)) {
SetPagePrivate(ciphertext_page);
set_page_private(ciphertext_page, (unsigned long)ctx);
lock_page(ciphertext_page);
@@ -297,7 +299,7 @@ EXPORT_SYMBOL(fscrypt_encrypt_page);
* 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.
+ * it is a writeback page (FS_CFLG_OWN_PAGES unset).
* @len: Number of bytes in @page to be decrypted.
* @offs: Start of data in @page.
* @lblk_num: Logical block number.
@@ -311,6 +313,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));
+
return do_page_crypto(inode, FS_DECRYPT, lblk_num, page, page, len, offs,
GFP_NOFS);
}
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 1485ac273bfb..fb2b514f675b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3744,7 +3744,6 @@ static int __ext4_block_zero_page_range(handle_t *handle,
/* We expect the key to be set. */
BUG_ON(!fscrypt_has_encryption_key(inode));
BUG_ON(blocksize != PAGE_SIZE);
- BUG_ON(!PageLocked(page));
WARN_ON_ONCE(fscrypt_decrypt_page(page->mapping->host,
page, PAGE_SIZE, 0, page->index));
}
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 435590c4b341..9f0ba90b92e4 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1194,7 +1194,6 @@ int do_write_data_page(struct f2fs_io_info *fio)
f2fs_wait_on_encrypted_page_writeback(F2FS_I_SB(inode),
fio->old_blkaddr);
retry_encrypt:
- BUG_ON(!PageLocked(fio->page));
fio->encrypted_page = fscrypt_encrypt_page(inode, fio->page,
PAGE_SIZE, 0,
fio->page->index,
diff --git a/fs/ubifs/crypto.c b/fs/ubifs/crypto.c
index aefa3c30b73b..3402720f2b28 100644
--- a/fs/ubifs/crypto.c
+++ b/fs/ubifs/crypto.c
@@ -87,7 +87,7 @@ int ubifs_decrypt(const struct inode *inode, struct ubifs_data_node *dn,
}

struct fscrypt_operations ubifs_crypt_operations = {
- .flags = FS_CFLG_INPLACE_ENCRYPTION,
+ .flags = FS_CFLG_OWN_PAGES,
.get_context = ubifs_crypt_get_context,
.set_context = ubifs_crypt_set_context,
.is_encrypted = __ubifs_crypt_is_encrypted,
diff --git a/include/linux/fscrypto.h b/include/linux/fscrypto.h
index e9648b62eca6..65be74f3afb6 100644
--- a/include/linux/fscrypto.h
+++ b/include/linux/fscrypto.h
@@ -156,7 +156,7 @@ struct fscrypt_name {
/*
* fscrypt superblock flags
*/
-#define FS_CFLG_INPLACE_ENCRYPTION (1U << 1)
+#define FS_CFLG_OWN_PAGES (1U << 1)

/*
* crypto opertions for filesystems
--
2.7.3

2016-12-01 22:25:49

by Richard Weinberger

[permalink] [raw]
Subject: [PATCH 1/6] fscrypt: Use correct index in decrypt path.

From: David Gstir <[email protected]>

Actually use the fs-provided index instead of always using page->index
which is only set for page-cache pages.

Fixes: 9c4bb8a3a9b4 ("fscrypt: Let fs select encryption index/tweak")
Signed-off-by: David Gstir <[email protected]>
Signed-off-by: Richard Weinberger <[email protected]>
---
fs/crypto/crypto.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index b6029785714c..3c1124beae28 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -296,7 +296,7 @@ EXPORT_SYMBOL(fscrypt_encrypt_page);
int fscrypt_decrypt_page(const struct inode *inode, struct page *page,
unsigned int len, unsigned int offs, pgoff_t index)
{
- return do_page_crypto(inode, FS_DECRYPT, page->index, page, page, len, offs,
+ return do_page_crypto(inode, FS_DECRYPT, index, page, page, len, offs,
GFP_NOFS);
}
EXPORT_SYMBOL(fscrypt_decrypt_page);
--
2.7.3

2016-12-02 08:12:56

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH 2/6] fscrypt: Release fscrypt context on in-place encryption

On Thu, Dec 01, 2016 at 11:14:54PM +0100, Richard Weinberger wrote:
> From: David Gstir <[email protected]>
>
> In case of in-place encryption ctx must be released right away.
> Otherwise ctx is never freed.
>

I didn't notice this before, but the fscrypt_ctx isn't actually used at all in
the "own pages" crypto case. So there's really no need to allocate one. How
about we just handle the "own pages" case at the beginning of
fscrypt_encrypt_page():

if (inode->i_sb->s_cop->flags & FS_CFLG_OWN_PAGES) {
err = do_page_crypto(inode, FS_ENCRYPT, lblk_num, page, page,
len, offs, gfp_flags);
if (err)
return ERR_PTR(err);
return page;
}

(and the rest essentially the same as it used to be)

Eric

2016-12-02 08:19:40

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH 3/6] fscrypt: Cleanup fscrypt_{decrypt,encrypt}_page()

On Thu, Dec 01, 2016 at 11:14:55PM +0100, Richard Weinberger wrote:
> + * @lblk_num: Logical block number. This must be unique for multiple
> + * calls with same page.

Must be unique for all calls with the same *inode*, except when overwriting
previously written data.

Eric

2016-12-02 15:50:47

by David Gstir

[permalink] [raw]
Subject: Re: [PATCH 3/6] fscrypt: Cleanup fscrypt_{decrypt,encrypt}_page()


> On 02.12.2016, at 09:19, Eric Biggers <[email protected]> wrote:
>
> On Thu, Dec 01, 2016 at 11:14:55PM +0100, Richard Weinberger wrote:
>> + * @lblk_num: Logical block number. This must be unique for multiple
>> + * calls with same page.
>
> Must be unique for all calls with the same *inode*, except when overwriting
> previously written data.

Yes, of course! Will fix that.

Thanks,
David

2016-12-03 21:57:14

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 0/6] UBIFS related fscrypt updates

On Thu, Dec 01, 2016 at 11:14:52PM +0100, Richard Weinberger wrote:
> This series applies on top of Ted's fscrypt tree[0] addresses the review
> comments from Eric.
> Ted, it would be awesome to have this patches in the v4.10 merge window.

Your patch 4/6 won't apply on my branch because it references
fs/ubifs/crypto.c (which of course isn't in my tree). I could drop
it, but I want to make sure you then make the appropriate changes in
your tree.

Also, I think Eric Bigger's changes make sense.

As far as whether or not they make it for the 4.10 merge window.
Linus could be opening the merge window tomorrow, so it's getting a
bit late for that.... if he decides to do a -rc8, we could probably
do it.

- Ted