linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/13] fscrypto: cleanups and fixes
@ 2016-04-03  5:21 Eric Biggers
  2016-04-03  5:21 ` [PATCH 01/13] fscrypto: remove unnecessary includes Eric Biggers
                   ` (12 more replies)
  0 siblings, 13 replies; 15+ messages in thread
From: Eric Biggers @ 2016-04-03  5:21 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: tytso, Eric Biggers, mhalcrow, linux-kernel, linux-f2fs-devel,
	jaegeuk, linux-ext4

This patchset includes various cleanups for the new filesystem encryption
code as well as some bug fixes for the FS_IOC_SET_ENCRYPTION_POLICY ioctl.

Eric Biggers (13):
  fscrypto: remove unnecessary includes
  fscrypto: rename some functions for clarity
  fscrypto: rename functions to load and unload inode encryption info
  fscrypto: return bool instead of int where appropriate
  fscrypto: comment improvements and fixes
  fscrypto: crypto_alloc_skcipher() always returns an ERR_PTR(), never
    NULL
  fscrypto: simplify building key descriptor string
  fscrypto: use standard macros from kernel.h
  fscrypto: make fname_encrypt() actually return length of ciphertext
  fscrypto: restrict setting new policy to empty files and directories
    only
  fscrypto: restrict setting encryption policy to inode owner
  fscrypto: require write access to mount to set encryption policy
  fscrypto: improve error handling in fscrypt_set_policy()

 fs/crypto/crypto.c       |  25 +++++----
 fs/crypto/fname.c        |  72 ++++++++++++++++----------
 fs/crypto/keyinfo.c      |  47 ++++++++---------
 fs/crypto/policy.c       | 131 +++++++++++++++++++++++++++--------------------
 fs/f2fs/dir.c            |   4 +-
 fs/f2fs/f2fs.h           |   6 +--
 fs/f2fs/file.c           |  10 ++--
 fs/f2fs/inode.c          |   2 +-
 fs/f2fs/namei.c          |  10 ++--
 fs/f2fs/super.c          |   2 +-
 include/linux/fscrypto.h |  81 +++++++++++++++++++++--------
 11 files changed, 236 insertions(+), 154 deletions(-)

-- 
2.7.4


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 01/13] fscrypto: remove unnecessary includes
  2016-04-03  5:21 [PATCH 00/13] fscrypto: cleanups and fixes Eric Biggers
@ 2016-04-03  5:21 ` Eric Biggers
  2016-04-03  5:21 ` [PATCH 02/13] fscrypto: rename some functions for clarity Eric Biggers
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Biggers @ 2016-04-03  5:21 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: tytso, Eric Biggers, mhalcrow, linux-kernel, linux-f2fs-devel,
	jaegeuk, linux-ext4

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 fs/crypto/crypto.c  | 1 -
 fs/crypto/fname.c   | 2 --
 fs/crypto/keyinfo.c | 3 ---
 3 files changed, 6 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 06cd1a2..2c7923d 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -27,7 +27,6 @@
 #include <linux/bio.h>
 #include <linux/dcache.h>
 #include <linux/fscrypto.h>
-#include <linux/ecryptfs.h>
 
 static unsigned int num_prealloc_crypto_pages = 32;
 static unsigned int num_prealloc_crypto_ctxs = 128;
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 5d6d491..3108806 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -10,8 +10,6 @@
  * This has not yet undergone a rigorous security audit.
  */
 
-#include <keys/encrypted-type.h>
-#include <keys/user-type.h>
 #include <linux/scatterlist.h>
 #include <linux/ratelimit.h>
 #include <linux/fscrypto.h>
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 06f5aa4..03d2b0d 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -8,11 +8,8 @@
  * Written by Michael Halcrow, Ildar Muslukhov, and Uday Savagaonkar, 2015.
  */
 
-#include <keys/encrypted-type.h>
 #include <keys/user-type.h>
-#include <linux/random.h>
 #include <linux/scatterlist.h>
-#include <uapi/linux/keyctl.h>
 #include <linux/fscrypto.h>
 
 static void derive_crypt_complete(struct crypto_async_request *req, int rc)
-- 
2.7.4


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 02/13] fscrypto: rename some functions for clarity
  2016-04-03  5:21 [PATCH 00/13] fscrypto: cleanups and fixes Eric Biggers
  2016-04-03  5:21 ` [PATCH 01/13] fscrypto: remove unnecessary includes Eric Biggers
@ 2016-04-03  5:21 ` Eric Biggers
  2016-04-03  7:45   ` Theodore Ts'o
  2016-04-03  5:21 ` [PATCH 03/13] fscrypto: rename functions to load and unload inode encryption info Eric Biggers
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 15+ messages in thread
From: Eric Biggers @ 2016-04-03  5:21 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: tytso, Eric Biggers, mhalcrow, linux-kernel, linux-f2fs-devel,
	jaegeuk, linux-ext4

Rename fscrypt_complete() to page_crypt_complete().  This callback is
specifically for data pages; fscrypto also performs filename encryption.

Rename dir_crypt_complete() to fname_crypt_complete().  This callback is
also used for symlink targets, not just directory entries.

Rename fscrypt_process_policy() to fscrypt_set_policy().  The new name
better matches the ioctl, and it goes along with fscrypt_get_policy().

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 fs/crypto/crypto.c       | 11 ++++++-----
 fs/crypto/fname.c        | 11 +++++++----
 fs/crypto/policy.c       |  5 ++---
 fs/f2fs/f2fs.h           |  2 +-
 fs/f2fs/file.c           |  2 +-
 include/linux/fscrypto.h |  5 ++---
 6 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 2c7923d..6e550ec 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -125,11 +125,12 @@ struct fscrypt_ctx *fscrypt_get_ctx(struct inode *inode)
 EXPORT_SYMBOL(fscrypt_get_ctx);
 
 /**
- * fscrypt_complete() - The completion callback for page encryption
- * @req: The asynchronous encryption request context
- * @res: The result of the encryption operation
+ * page_crypt_complete() - The completion callback for page encryption and
+ *			   decryption
+ * @req: The asynchronous cipher request context
+ * @res: The result of the cipher operation
  */
-static void fscrypt_complete(struct crypto_async_request *req, int res)
+static void page_crypt_complete(struct crypto_async_request *req, int res)
 {
 	struct fscrypt_completion_result *ecr = req->data;
 
@@ -166,7 +167,7 @@ static int do_page_crypto(struct inode *inode,
 
 	skcipher_request_set_callback(
 		req, CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
-		fscrypt_complete, &ecr);
+		page_crypt_complete, &ecr);
 
 	BUILD_BUG_ON(FS_XTS_TWEAK_SIZE < sizeof(index));
 	memcpy(xts_tweak, &index, sizeof(index));
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 3108806..c3e3554 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -20,9 +20,12 @@ static u32 size_round_up(size_t size, size_t blksize)
 }
 
 /**
- * dir_crypt_complete() -
+ * fname_crypt_complete() - The completion callback for filename encryption and
+ *			    decryption
+ * @req: The asynchronous cipher request context
+ * @res: The result of the cipher operation
  */
-static void dir_crypt_complete(struct crypto_async_request *req, int res)
+static void fname_crypt_complete(struct crypto_async_request *req, int res)
 {
 	struct fscrypt_completion_result *ecr = req->data;
 
@@ -82,7 +85,7 @@ static int fname_encrypt(struct inode *inode,
 	}
 	skcipher_request_set_callback(req,
 			CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
-			dir_crypt_complete, &ecr);
+			fname_crypt_complete, &ecr);
 
 	/* Copy the input */
 	memcpy(workbuf, iname->name, iname->len);
@@ -144,7 +147,7 @@ static int fname_decrypt(struct inode *inode,
 	}
 	skcipher_request_set_callback(req,
 		CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
-		dir_crypt_complete, &ecr);
+		fname_crypt_complete, &ecr);
 
 	/* Initialize IV */
 	memset(iv, 0, FS_CRYPTO_BLOCK_SIZE);
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 0f9961e..e1d263d 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -92,8 +92,7 @@ static int create_encryption_context_from_policy(struct inode *inode,
 	return inode->i_sb->s_cop->set_context(inode, &ctx, sizeof(ctx), NULL);
 }
 
-int fscrypt_process_policy(struct inode *inode,
-				const struct fscrypt_policy *policy)
+int fscrypt_set_policy(struct inode *inode, const struct fscrypt_policy *policy)
 {
 	if (policy->version != 0)
 		return -EINVAL;
@@ -113,7 +112,7 @@ int fscrypt_process_policy(struct inode *inode,
 	       __func__);
 	return -EINVAL;
 }
-EXPORT_SYMBOL(fscrypt_process_policy);
+EXPORT_SYMBOL(fscrypt_set_policy);
 
 int fscrypt_get_policy(struct inode *inode, struct fscrypt_policy *policy)
 {
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index bbe2cd1..970678d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2172,7 +2172,7 @@ static inline bool f2fs_may_encrypt(struct inode *inode)
 #define fscrypt_pullback_bio_page	fscrypt_notsupp_pullback_bio_page
 #define fscrypt_restore_control_page	fscrypt_notsupp_restore_control_page
 #define fscrypt_zeroout_range		fscrypt_notsupp_zeroout_range
-#define fscrypt_process_policy		fscrypt_notsupp_process_policy
+#define fscrypt_set_policy		fscrypt_notsupp_set_policy
 #define fscrypt_get_policy		fscrypt_notsupp_get_policy
 #define fscrypt_has_permitted_context	fscrypt_notsupp_has_permitted_context
 #define fscrypt_inherit_context		fscrypt_notsupp_inherit_context
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index b41c357..ea2c9a9 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1542,7 +1542,7 @@ static int f2fs_ioc_set_encryption_policy(struct file *filp, unsigned long arg)
 		return -EFAULT;
 
 	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
-	return fscrypt_process_policy(inode, &policy);
+	return fscrypt_set_policy(inode, &policy);
 }
 
 static int f2fs_ioc_get_encryption_policy(struct file *filp, unsigned long arg)
diff --git a/include/linux/fscrypto.h b/include/linux/fscrypto.h
index cd91f75..4e7bc69 100644
--- a/include/linux/fscrypto.h
+++ b/include/linux/fscrypto.h
@@ -273,8 +273,7 @@ extern void fscrypt_restore_control_page(struct page *);
 extern int fscrypt_zeroout_range(struct inode *, pgoff_t, sector_t,
 						unsigned int);
 /* policy.c */
-extern int fscrypt_process_policy(struct inode *,
-					const struct fscrypt_policy *);
+extern int fscrypt_set_policy(struct inode *, const struct fscrypt_policy *);
 extern int fscrypt_get_policy(struct inode *, struct fscrypt_policy *);
 extern int fscrypt_has_permitted_context(struct inode *, struct inode *);
 extern int fscrypt_inherit_context(struct inode *, struct inode *,
@@ -343,7 +342,7 @@ static inline int fscrypt_notsupp_zeroout_range(struct inode *i, pgoff_t p,
 }
 
 /* policy.c */
-static inline int fscrypt_notsupp_process_policy(struct inode *i,
+static inline int fscrypt_notsupp_set_policy(struct inode *i,
 				const struct fscrypt_policy *p)
 {
 	return -EOPNOTSUPP;
-- 
2.7.4


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 03/13] fscrypto: rename functions to load and unload inode encryption info
  2016-04-03  5:21 [PATCH 00/13] fscrypto: cleanups and fixes Eric Biggers
  2016-04-03  5:21 ` [PATCH 01/13] fscrypto: remove unnecessary includes Eric Biggers
  2016-04-03  5:21 ` [PATCH 02/13] fscrypto: rename some functions for clarity Eric Biggers
@ 2016-04-03  5:21 ` Eric Biggers
  2016-04-03  5:21 ` [PATCH 04/13] fscrypto: return bool instead of int where appropriate Eric Biggers
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Biggers @ 2016-04-03  5:21 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: tytso, Eric Biggers, mhalcrow, linux-kernel, linux-f2fs-devel,
	jaegeuk, linux-ext4

Perform the following renamings:

	fscrypt_get_encryption_info() => fscrypt_load_encryption_info()
	fscrypt_put_encryption_info() => fscrypt_unload_encryption_info()
	get_crypt_info => load_crypt_info()
	put_crypt_info() => free_crypt_info()

The new names better reflect the actual behavior where the "load"
function just loads the fscrypt_info into i_crypto_info; it doesn't
actually return it to the caller.  There is also no reference counting
involved, as might be suggested by the verbs "get" and "put".

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 fs/crypto/fname.c        |  2 +-
 fs/crypto/keyinfo.c      | 23 ++++++++++++-----------
 fs/crypto/policy.c       |  8 ++++----
 fs/f2fs/dir.c            |  4 ++--
 fs/f2fs/f2fs.h           |  4 ++--
 fs/f2fs/file.c           |  8 ++++----
 fs/f2fs/inode.c          |  2 +-
 fs/f2fs/namei.c          | 10 +++++-----
 fs/f2fs/super.c          |  2 +-
 include/linux/fscrypto.h | 11 ++++++-----
 10 files changed, 38 insertions(+), 36 deletions(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index c3e3554..ceaa815 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -361,7 +361,7 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
 		fname->disk_name.len = iname->len;
 		return 0;
 	}
-	ret = get_crypt_info(dir);
+	ret = load_crypt_info(dir);
 	if (ret && ret != -EOPNOTSUPP)
 		return ret;
 
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 03d2b0d..e0b3281 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -75,7 +75,7 @@ out:
 	return res;
 }
 
-static void put_crypt_info(struct fscrypt_info *ci)
+static void free_crypt_info(struct fscrypt_info *ci)
 {
 	if (!ci)
 		return;
@@ -85,7 +85,7 @@ static void put_crypt_info(struct fscrypt_info *ci)
 	kmem_cache_free(fscrypt_info_cachep, ci);
 }
 
-int get_crypt_info(struct inode *inode)
+int load_crypt_info(struct inode *inode)
 {
 	struct fscrypt_info *crypt_info;
 	u8 full_key_descriptor[FS_KEY_DESC_PREFIX_SIZE +
@@ -112,7 +112,7 @@ retry:
 		if (!crypt_info->ci_keyring_key ||
 				key_validate(crypt_info->ci_keyring_key) == 0)
 			return 0;
-		fscrypt_put_encryption_info(inode, crypt_info);
+		fscrypt_unload_encryption_info(inode, crypt_info);
 		goto retry;
 	}
 
@@ -224,7 +224,7 @@ got_key:
 
 	memzero_explicit(raw_key, sizeof(raw_key));
 	if (cmpxchg(&inode->i_crypt_info, NULL, crypt_info) != NULL) {
-		put_crypt_info(crypt_info);
+		free_crypt_info(crypt_info);
 		goto retry;
 	}
 	return 0;
@@ -232,12 +232,13 @@ got_key:
 out:
 	if (res == -ENOKEY)
 		res = 0;
-	put_crypt_info(crypt_info);
+	free_crypt_info(crypt_info);
 	memzero_explicit(raw_key, sizeof(raw_key));
 	return res;
 }
 
-void fscrypt_put_encryption_info(struct inode *inode, struct fscrypt_info *ci)
+void fscrypt_unload_encryption_info(struct inode *inode,
+				    struct fscrypt_info *ci)
 {
 	struct fscrypt_info *prev;
 
@@ -250,11 +251,11 @@ void fscrypt_put_encryption_info(struct inode *inode, struct fscrypt_info *ci)
 	if (prev != ci)
 		return;
 
-	put_crypt_info(ci);
+	free_crypt_info(ci);
 }
-EXPORT_SYMBOL(fscrypt_put_encryption_info);
+EXPORT_SYMBOL(fscrypt_unload_encryption_info);
 
-int fscrypt_get_encryption_info(struct inode *inode)
+int fscrypt_load_encryption_info(struct inode *inode)
 {
 	struct fscrypt_info *ci = inode->i_crypt_info;
 
@@ -263,7 +264,7 @@ int fscrypt_get_encryption_info(struct inode *inode)
 		 (ci->ci_keyring_key->flags & ((1 << KEY_FLAG_INVALIDATED) |
 					       (1 << KEY_FLAG_REVOKED) |
 					       (1 << KEY_FLAG_DEAD)))))
-		return get_crypt_info(inode);
+		return load_crypt_info(inode);
 	return 0;
 }
-EXPORT_SYMBOL(fscrypt_get_encryption_info);
+EXPORT_SYMBOL(fscrypt_load_encryption_info);
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index e1d263d..03a2f50 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -155,10 +155,10 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
 	/* if the child directory is not encrypted, this is always a problem */
 	if (!parent->i_sb->s_cop->is_encrypted(child))
 		return 0;
-	res = fscrypt_get_encryption_info(parent);
+	res = fscrypt_load_encryption_info(parent);
 	if (res)
 		return 0;
-	res = fscrypt_get_encryption_info(child);
+	res = fscrypt_load_encryption_info(child);
 	if (res)
 		return 0;
 	parent_ci = parent->i_crypt_info;
@@ -196,7 +196,7 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child,
 	if (!parent->i_sb->s_cop->set_context)
 		return -EOPNOTSUPP;
 
-	res = fscrypt_get_encryption_info(parent);
+	res = fscrypt_load_encryption_info(parent);
 	if (res < 0)
 		return res;
 
@@ -223,6 +223,6 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child,
 						sizeof(ctx), fs_data);
 	if (res)
 		return res;
-	return preload ? fscrypt_get_encryption_info(child): 0;
+	return preload ? fscrypt_load_encryption_info(child) : 0;
 }
 EXPORT_SYMBOL(fscrypt_inherit_context);
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 80641ad..5f77455 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -844,7 +844,7 @@ static int f2fs_readdir(struct file *file, struct dir_context *ctx)
 	int err = 0;
 
 	if (f2fs_encrypted_inode(inode)) {
-		err = fscrypt_get_encryption_info(inode);
+		err = fscrypt_load_encryption_info(inode);
 		if (err && err != -ENOKEY)
 			return err;
 
@@ -895,7 +895,7 @@ out:
 static int f2fs_dir_open(struct inode *inode, struct file *filp)
 {
 	if (f2fs_encrypted_inode(inode))
-		return fscrypt_get_encryption_info(inode) ? -EACCES : 0;
+		return fscrypt_load_encryption_info(inode) ? -EACCES : 0;
 	return 0;
 }
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 970678d..2956440 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2176,8 +2176,8 @@ static inline bool f2fs_may_encrypt(struct inode *inode)
 #define fscrypt_get_policy		fscrypt_notsupp_get_policy
 #define fscrypt_has_permitted_context	fscrypt_notsupp_has_permitted_context
 #define fscrypt_inherit_context		fscrypt_notsupp_inherit_context
-#define fscrypt_get_encryption_info	fscrypt_notsupp_get_encryption_info
-#define fscrypt_put_encryption_info	fscrypt_notsupp_put_encryption_info
+#define fscrypt_load_encryption_info	fscrypt_notsupp_load_encryption_info
+#define fscrypt_unload_encryption_info	fscrypt_notsupp_unload_encryption_info
 #define fscrypt_setup_filename		fscrypt_notsupp_setup_filename
 #define fscrypt_free_filename		fscrypt_notsupp_free_filename
 #define fscrypt_fname_encrypted_size	fscrypt_notsupp_fname_encrypted_size
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index ea2c9a9..cf691ae 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -421,7 +421,7 @@ static int f2fs_file_mmap(struct file *file, struct vm_area_struct *vma)
 	int err;
 
 	if (f2fs_encrypted_inode(inode)) {
-		err = fscrypt_get_encryption_info(inode);
+		err = fscrypt_load_encryption_info(inode);
 		if (err)
 			return 0;
 		if (!f2fs_encrypted_inode(inode))
@@ -444,7 +444,7 @@ static int f2fs_file_open(struct inode *inode, struct file *filp)
 	struct inode *dir = filp->f_path.dentry->d_parent->d_inode;
 
 	if (!ret && f2fs_encrypted_inode(inode)) {
-		ret = fscrypt_get_encryption_info(inode);
+		ret = fscrypt_load_encryption_info(inode);
 		if (ret)
 			return -EACCES;
 		if (!fscrypt_has_encryption_key(inode))
@@ -679,7 +679,7 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
 
 	if (attr->ia_valid & ATTR_SIZE) {
 		if (f2fs_encrypted_inode(inode) &&
-				fscrypt_get_encryption_info(inode))
+				fscrypt_load_encryption_info(inode))
 			return -EACCES;
 
 		if (attr->ia_size <= i_size_read(inode)) {
@@ -1870,7 +1870,7 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 
 	if (f2fs_encrypted_inode(inode) &&
 				!fscrypt_has_encryption_key(inode) &&
-				fscrypt_get_encryption_info(inode))
+				fscrypt_load_encryption_info(inode))
 		return -EACCES;
 
 	inode_lock(inode);
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index cb269c4..cc3fe60 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -389,7 +389,7 @@ no_delete:
 		}
 	}
 out_clear:
-	fscrypt_put_encryption_info(inode, NULL);
+	fscrypt_unload_encryption_info(inode, NULL);
 	clear_inode(inode);
 }
 
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 7876f10..bd42674 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -263,7 +263,7 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
 	unsigned int root_ino = F2FS_ROOT_INO(F2FS_I_SB(dir));
 
 	if (f2fs_encrypted_inode(dir)) {
-		int res = fscrypt_get_encryption_info(dir);
+		int res = fscrypt_load_encryption_info(dir);
 
 		/*
 		 * DCACHE_ENCRYPTED_WITH_KEY is set if the dentry is
@@ -380,7 +380,7 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 	int err;
 
 	if (f2fs_encrypted_inode(dir)) {
-		err = fscrypt_get_encryption_info(dir);
+		err = fscrypt_load_encryption_info(dir);
 		if (err)
 			return err;
 
@@ -424,7 +424,7 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 			goto err_out;
 		}
 
-		err = fscrypt_get_encryption_info(inode);
+		err = fscrypt_load_encryption_info(inode);
 		if (err)
 			goto err_out;
 
@@ -616,7 +616,7 @@ out:
 static int f2fs_tmpfile(struct inode *dir, struct dentry *dentry, umode_t mode)
 {
 	if (f2fs_encrypted_inode(dir)) {
-		int err = fscrypt_get_encryption_info(dir);
+		int err = fscrypt_load_encryption_info(dir);
 		if (err)
 			return err;
 	}
@@ -1006,7 +1006,7 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry,
 	if (!dentry)
 		return ERR_PTR(-ECHILD);
 
-	res = fscrypt_get_encryption_info(inode);
+	res = fscrypt_load_encryption_info(inode);
 	if (res)
 		return ERR_PTR(res);
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 15bb81f..72126cc 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -503,7 +503,7 @@ static int f2fs_drop_inode(struct inode *inode)
 
 			sb_end_intwrite(inode->i_sb);
 
-			fscrypt_put_encryption_info(inode, NULL);
+			fscrypt_unload_encryption_info(inode, NULL);
 			spin_lock(&inode->i_lock);
 			atomic_dec(&inode->i_count);
 		}
diff --git a/include/linux/fscrypto.h b/include/linux/fscrypto.h
index 4e7bc69..1bf00a5 100644
--- a/include/linux/fscrypto.h
+++ b/include/linux/fscrypto.h
@@ -279,9 +279,10 @@ extern int fscrypt_has_permitted_context(struct inode *, struct inode *);
 extern int fscrypt_inherit_context(struct inode *, struct inode *,
 					void *, bool);
 /* keyinfo.c */
-extern int get_crypt_info(struct inode *);
-extern int fscrypt_get_encryption_info(struct inode *);
-extern void fscrypt_put_encryption_info(struct inode *, struct fscrypt_info *);
+extern int load_crypt_info(struct inode *);
+extern int fscrypt_load_encryption_info(struct inode *);
+extern void fscrypt_unload_encryption_info(struct inode *,
+					   struct fscrypt_info *);
 
 /* fname.c */
 extern int fscrypt_setup_filename(struct inode *, const struct qstr *,
@@ -367,12 +368,12 @@ static inline int fscrypt_notsupp_inherit_context(struct inode *p,
 }
 
 /* keyinfo.c */
-static inline int fscrypt_notsupp_get_encryption_info(struct inode *i)
+static inline int fscrypt_notsupp_load_encryption_info(struct inode *i)
 {
 	return -EOPNOTSUPP;
 }
 
-static inline void fscrypt_notsupp_put_encryption_info(struct inode *i,
+static inline void fscrypt_notsupp_unload_encryption_info(struct inode *i,
 					struct fscrypt_info *f)
 {
 	return;
-- 
2.7.4


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 04/13] fscrypto: return bool instead of int where appropriate
  2016-04-03  5:21 [PATCH 00/13] fscrypto: cleanups and fixes Eric Biggers
                   ` (2 preceding siblings ...)
  2016-04-03  5:21 ` [PATCH 03/13] fscrypto: rename functions to load and unload inode encryption info Eric Biggers
@ 2016-04-03  5:21 ` Eric Biggers
  2016-04-03  5:21 ` [PATCH 05/13] fscrypto: comment improvements and fixes Eric Biggers
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Biggers @ 2016-04-03  5:21 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: tytso, Eric Biggers, mhalcrow, linux-kernel, linux-f2fs-devel,
	jaegeuk, linux-ext4

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 fs/crypto/policy.c       | 24 ++++++++++++------------
 include/linux/fscrypto.h | 12 ++++++------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 03a2f50..93244b5 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -12,10 +12,10 @@
 #include <linux/string.h>
 #include <linux/fscrypto.h>
 
-static int inode_has_encryption_context(struct inode *inode)
+static bool inode_has_encryption_context(struct inode *inode)
 {
 	if (!inode->i_sb->s_cop->get_context)
-		return 0;
+		return false;
 	return (inode->i_sb->s_cop->get_context(inode, NULL, 0L) > 0);
 }
 
@@ -23,18 +23,18 @@ static int inode_has_encryption_context(struct inode *inode)
  * check whether the policy is consistent with the encryption context
  * for the inode
  */
-static int is_encryption_context_consistent_with_policy(struct inode *inode,
+static bool is_encryption_context_consistent_with_policy(struct inode *inode,
 				const struct fscrypt_policy *policy)
 {
 	struct fscrypt_context ctx;
 	int res;
 
 	if (!inode->i_sb->s_cop->get_context)
-		return 0;
+		return false;
 
 	res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
 	if (res != sizeof(ctx))
-		return 0;
+		return false;
 
 	return (memcmp(ctx.master_key_descriptor, policy->master_key_descriptor,
 			FS_KEY_DESCRIPTOR_SIZE) == 0 &&
@@ -139,7 +139,7 @@ int fscrypt_get_policy(struct inode *inode, struct fscrypt_policy *policy)
 }
 EXPORT_SYMBOL(fscrypt_get_policy);
 
-int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
+bool fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
 {
 	struct fscrypt_info *parent_ci, *child_ci;
 	int res;
@@ -151,22 +151,22 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
 
 	/* no restrictions if the parent directory is not encrypted */
 	if (!parent->i_sb->s_cop->is_encrypted(parent))
-		return 1;
+		return true;
 	/* if the child directory is not encrypted, this is always a problem */
 	if (!parent->i_sb->s_cop->is_encrypted(child))
-		return 0;
+		return false;
 	res = fscrypt_load_encryption_info(parent);
 	if (res)
-		return 0;
+		return false;
 	res = fscrypt_load_encryption_info(child);
 	if (res)
-		return 0;
+		return false;
 	parent_ci = parent->i_crypt_info;
 	child_ci = child->i_crypt_info;
 	if (!parent_ci && !child_ci)
-		return 1;
+		return true;
 	if (!parent_ci || !child_ci)
-		return 0;
+		return false;
 
 	return (memcmp(parent_ci->ci_master_key,
 			child_ci->ci_master_key,
diff --git a/include/linux/fscrypto.h b/include/linux/fscrypto.h
index 1bf00a5..9a32d7e 100644
--- a/include/linux/fscrypto.h
+++ b/include/linux/fscrypto.h
@@ -177,7 +177,7 @@ struct fscrypt_operations {
 	int (*get_context)(struct inode *, void *, size_t);
 	int (*prepare_context)(struct inode *);
 	int (*set_context)(struct inode *, const void *, size_t, void *);
-	int (*dummy_context)(struct inode *);
+	bool (*dummy_context)(struct inode *);
 	bool (*is_encrypted)(struct inode *);
 	bool (*empty_dir)(struct inode *);
 	unsigned (*max_namelen)(struct inode *);
@@ -229,12 +229,12 @@ static inline struct page *fscrypt_control_page(struct page *page)
 #endif
 }
 
-static inline int fscrypt_has_encryption_key(struct inode *inode)
+static inline bool fscrypt_has_encryption_key(struct inode *inode)
 {
 #if IS_ENABLED(CONFIG_FS_ENCRYPTION)
 	return (inode->i_crypt_info != NULL);
 #else
-	return 0;
+	return false;
 #endif
 }
 
@@ -275,7 +275,7 @@ extern int fscrypt_zeroout_range(struct inode *, pgoff_t, sector_t,
 /* policy.c */
 extern int fscrypt_set_policy(struct inode *, const struct fscrypt_policy *);
 extern int fscrypt_get_policy(struct inode *, struct fscrypt_policy *);
-extern int fscrypt_has_permitted_context(struct inode *, struct inode *);
+extern bool fscrypt_has_permitted_context(struct inode *, struct inode *);
 extern int fscrypt_inherit_context(struct inode *, struct inode *,
 					void *, bool);
 /* keyinfo.c */
@@ -355,10 +355,10 @@ static inline int fscrypt_notsupp_get_policy(struct inode *i,
 	return -EOPNOTSUPP;
 }
 
-static inline int fscrypt_notsupp_has_permitted_context(struct inode *p,
+static inline bool fscrypt_notsupp_has_permitted_context(struct inode *p,
 				struct inode *i)
 {
-	return 0;
+	return false;
 }
 
 static inline int fscrypt_notsupp_inherit_context(struct inode *p,
-- 
2.7.4


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 05/13] fscrypto: comment improvements and fixes
  2016-04-03  5:21 [PATCH 00/13] fscrypto: cleanups and fixes Eric Biggers
                   ` (3 preceding siblings ...)
  2016-04-03  5:21 ` [PATCH 04/13] fscrypto: return bool instead of int where appropriate Eric Biggers
@ 2016-04-03  5:21 ` Eric Biggers
  2016-04-03  5:21 ` [PATCH 06/13] fscrypto: crypto_alloc_skcipher() always returns an ERR_PTR(), never NULL Eric Biggers
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Biggers @ 2016-04-03  5:21 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: tytso, Eric Biggers, mhalcrow, linux-kernel, linux-f2fs-devel,
	jaegeuk, linux-ext4

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 fs/crypto/crypto.c       | 13 ++++++------
 fs/crypto/fname.c        | 33 ++++++++++++++++++++++++++----
 fs/crypto/keyinfo.c      |  8 ++++++++
 include/linux/fscrypto.h | 53 ++++++++++++++++++++++++++++++++++++++++++------
 4 files changed, 90 insertions(+), 17 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 6e550ec..836c0f2 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -83,8 +83,8 @@ EXPORT_SYMBOL(fscrypt_release_ctx);
  *
  * Allocates and initializes an encryption context.
  *
- * Return: An allocated and initialized encryption context on success; error
- * value or NULL otherwise.
+ * Return: An allocated and initialized encryption context on success; an error
+ * value otherwise.
  */
 struct fscrypt_ctx *fscrypt_get_ctx(struct inode *inode)
 {
@@ -222,7 +222,7 @@ static struct page *alloc_bounce_page(struct fscrypt_ctx *ctx)
  * release the bounce buffer and the encryption context.
  *
  * Return: An allocated page with the encrypted content on success. Else, an
- * error value or NULL.
+ * error value.
  */
 struct page *fscrypt_encrypt_page(struct inode *inode,
 				struct page *plaintext_page)
@@ -261,10 +261,10 @@ errout:
 EXPORT_SYMBOL(fscrypt_encrypt_page);
 
 /**
- * f2crypt_decrypt_page() - Decrypts a page in-place
+ * fscrypt_decrypt_page() - Decrypts a page in-place
  * @page: The page to decrypt. Must be locked.
  *
- * Decrypts page in-place using the ctx encryption context.
+ * Decrypts a page in-place using the host inode's encryption context.
  *
  * Called from the read completion callback.
  *
@@ -358,7 +358,6 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 					  (1 << KEY_FLAG_DEAD))))
 		ci = NULL;
 
-	/* this should eventually be an flag in d_flags */
 	spin_lock(&dentry->d_lock);
 	cached_with_key = dentry->d_flags & DCACHE_ENCRYPTED_WITH_KEY;
 	spin_unlock(&dentry->d_lock);
@@ -368,7 +367,7 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 	 * If the dentry was cached without the key, and it is a
 	 * negative dentry, it might be a valid name.  We can't check
 	 * if the key has since been made available due to locking
-	 * reasons, so we fail the validation so ext4_lookup() can do
+	 * reasons, so we fail the validation so ->lookup() can do
 	 * this check.
 	 *
 	 * We also fail the validation if the dentry was created with
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index ceaa815..cd0eae8 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -178,8 +178,11 @@ static const char *lookup_table =
 /**
  * digest_encode() -
  *
- * Encodes the input digest using characters from the set [a-zA-Z0-9_+].
+ * Encodes the input digest using characters from the set [A-Za-z0-9+,].
  * The encoded string is roughly 4/3 times the size of the input string.
+ *
+ * Note that the result of this encoding is for presentation purposes only; it
+ * is not persisted in the filesystem.
  */
 static int digest_encode(const char *src, int len, char *dst)
 {
@@ -239,7 +242,7 @@ u32 fscrypt_fname_encrypted_size(struct inode *inode, u32 ilen)
 EXPORT_SYMBOL(fscrypt_fname_encrypted_size);
 
 /**
- * fscrypt_fname_crypto_alloc_obuff() -
+ * fscrypt_fname_alloc_buffer() -
  *
  * Allocates an output buffer that is sufficient for the crypto operation
  * specified by the context and the direction.
@@ -264,7 +267,7 @@ int fscrypt_fname_alloc_buffer(struct inode *inode,
 EXPORT_SYMBOL(fscrypt_fname_alloc_buffer);
 
 /**
- * fscrypt_fname_crypto_free_buffer() -
+ * fscrypt_fname_free_buffer() -
  *
  * Frees the buffer allocated for crypto operation.
  */
@@ -280,6 +283,12 @@ EXPORT_SYMBOL(fscrypt_fname_free_buffer);
 /**
  * fscrypt_fname_disk_to_usr() - converts a filename from disk space to user
  * space
+ *
+ * The caller must have used fscrypt_fname_alloc_buffer() to allocate sufficient
+ * memory for the @oname string.  Also, fscrypt_load_encryption_info() must have
+ * been already called on the inode.
+ *
+ * Return: the length of the user space name or a negative errno value.
  */
 int fscrypt_fname_disk_to_usr(struct inode *inode,
 			u32 hash, u32 minor_hash,
@@ -290,6 +299,7 @@ int fscrypt_fname_disk_to_usr(struct inode *inode,
 	char buf[24];
 	int ret;
 
+	/* Leave . and .. alone. */
 	if (fscrypt_is_dot_dotdot(&qname)) {
 		oname->name[0] = '.';
 		oname->name[iname->len - 1] = '.';
@@ -300,14 +310,19 @@ int fscrypt_fname_disk_to_usr(struct inode *inode,
 	if (iname->len < FS_CRYPTO_BLOCK_SIZE)
 		return -EUCLEAN;
 
+	/* Decrypt the name if we have the key. */
 	if (inode->i_crypt_info)
 		return fname_decrypt(inode, iname, oname);
 
 	if (iname->len <= FS_FNAME_CRYPTO_DIGEST_SIZE) {
+		/* Short name: encode the encrypted name. */
 		ret = digest_encode(iname->name, iname->len, oname->name);
 		oname->len = ret;
 		return ret;
 	}
+
+	/* Long name: encode the name hash (if a directory entry, not a symlink)
+	 * concatenated with the last 16 bytes of the encrypted name. */
 	if (hash) {
 		memcpy(buf, &hash, 4);
 		memcpy(buf + 4, &minor_hash, 4);
@@ -325,23 +340,33 @@ EXPORT_SYMBOL(fscrypt_fname_disk_to_usr);
 /**
  * fscrypt_fname_usr_to_disk() - converts a filename from user space to disk
  * space
+ *
+ * The caller must have used fscrypt_fname_alloc_buffer() to allocate sufficient
+ * memory for the @oname string.  Also, fscrypt_load_encryption_info() must have
+ * been already called on the inode.
+ *
+ * Return: the length of the disk space name or a negative errno value.
  */
 int fscrypt_fname_usr_to_disk(struct inode *inode,
 			const struct qstr *iname,
 			struct fscrypt_str *oname)
 {
+	/* Leave . and .. alone. */
 	if (fscrypt_is_dot_dotdot(iname)) {
 		oname->name[0] = '.';
 		oname->name[iname->len - 1] = '.';
 		oname->len = iname->len;
 		return oname->len;
 	}
+
+	/* Encrypt the name if we have the key. */
 	if (inode->i_crypt_info)
 		return fname_encrypt(inode, iname, oname);
+
 	/*
 	 * Without a proper key, a user is not allowed to modify the filenames
 	 * in a directory. Consequently, a user space name cannot be mapped to
-	 * a disk-space name
+	 * a disk-space name.
 	 */
 	return -EACCES;
 }
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index e0b3281..33296c0 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -255,6 +255,14 @@ void fscrypt_unload_encryption_info(struct inode *inode,
 }
 EXPORT_SYMBOL(fscrypt_unload_encryption_info);
 
+/*
+ * Try to load the fscrypt_info for an encrypted inode into memory.
+ *
+ * Return: 0 if the fscrypt_info was loaded or was already loaded, or also 0 if
+ * the master encryption key is not available (use fscrypt_has_encryption_key()
+ * to distingush the two cases); or a negative errno value if another error
+ * occurred.
+ */
 int fscrypt_load_encryption_info(struct inode *inode)
 {
 	struct fscrypt_info *ci = inode->i_crypt_info;
diff --git a/include/linux/fscrypto.h b/include/linux/fscrypto.h
index 9a32d7e..f29dc8c 100644
--- a/include/linux/fscrypto.h
+++ b/include/linux/fscrypto.h
@@ -36,9 +36,8 @@
 #define FS_ENCRYPTION_MODE_AES_256_CTS		4
 
 /**
- * Encryption context for inode
+ * struct fscrypt_context: the on-disk format of an inode's encryption metadata
  *
- * Protector format:
  *  1 byte: Protector format (1 = this version)
  *  1 byte: File contents encryption mode
  *  1 byte: File names encryption mode
@@ -67,13 +66,36 @@ struct fscrypt_context {
 #define FS_KEY_DESC_PREFIX		"fscrypt:"
 #define FS_KEY_DESC_PREFIX_SIZE		8
 
-/* This is passed in from userspace into the kernel keyring */
+/**
+ * struct fscrypt_key - payload of a master encryption key, as passed into a
+ *			kernel keyring from userspace
+ *
+ * @mode: Currently unused
+ * @raw: Raw bytes of the key
+ * @size: Length of the key (number of bytes in @raw actually used)
+ */
 struct fscrypt_key {
 	u32 mode;
 	u8 raw[FS_MAX_KEY_SIZE];
 	u32 size;
 } __packed;
 
+/**
+ * struct fscrypt_info - the cipher, encryption modes, and master key reference
+ *			 for an inode
+ *
+ * @ci_data_mode: Encryption mode to use for the contents of regular files
+ * @ci_filename_mode: Encryption mode to use for filenames and symlink targets
+ * @ci_flags: See FS_POLICY_FLAGS_*.
+ * @ci_ctfm: Allocated symmetric cipher for this inode, using the inode's
+ *	     unique encryption key which is derived from the master key and the
+ *	     per-inode nonce.  If the inode is a regular file, this cipher uses
+ *	     the data encryption mode, whereas if the inode is a directory or
+ *	     symlink, this cipher uses the filename encryption mode.
+ * @ci_keyring_key: Reference to the master key in a kernel keyring, or NULL if
+ *		    the test-only "dummy" encryption mode is in effect
+ * @ci_master_key: The descriptor of the master key, in binary form
+ */
 struct fscrypt_info {
 	u8 ci_data_mode;
 	u8 ci_filename_mode;
@@ -86,13 +108,19 @@ struct fscrypt_info {
 #define FS_CTX_REQUIRES_FREE_ENCRYPT_FL		0x00000001
 #define FS_WRITE_PATH_FL			0x00000002
 
+/**
+ * struct fscrypt_ctx - holds pages being decrypted or encrypted
+ *
+ * This is a reusable structure, not tied to any particular inode.
+ */
 struct fscrypt_ctx {
 	union {
-		struct {
+		struct { /* Writing (encryption) */
 			struct page *bounce_page;	/* Ciphertext page */
 			struct page *control_page;	/* Original page  */
 		} w;
-		struct {
+
+		struct { /* Reading (decryption) */
 			struct bio *bio;
 			struct work_struct work;
 		} r;
@@ -171,7 +199,20 @@ struct fscrypt_name {
 #define fname_len(p)		((p)->disk_name.len)
 
 /*
- * crypto opertions for filesystems
+ * struct fscrypt_operations - crypto operations for filesystems
+ *
+ * @get_context - Retrieve the inode's persistent encryption metadata into the
+ *		  provided buffer.  Semantics are like getxattr().
+ * @set_context - Set the inode's persistent encryption metadata.  Return 0 or a
+ *		  negative errno value.
+ * @dummy_context - Test whether the inode uses the test-only "dummy" encryption
+ *		    mode
+ * @is_encrypted - Test whether the inode is encrypted
+ * @empty_dir - Test whether the directory inode is empty
+ * @max_namelen - Return the maximum length of an unencrypted name we might have
+ *		  to encrypt for the inode.  Note that for symlinks, we encrypt
+ *		  the symlink *target* which typically can be longer than a
+ *		  single filename.
  */
 struct fscrypt_operations {
 	int (*get_context)(struct inode *, void *, size_t);
-- 
2.7.4


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 06/13] fscrypto: crypto_alloc_skcipher() always returns an ERR_PTR(), never NULL
  2016-04-03  5:21 [PATCH 00/13] fscrypto: cleanups and fixes Eric Biggers
                   ` (4 preceding siblings ...)
  2016-04-03  5:21 ` [PATCH 05/13] fscrypto: comment improvements and fixes Eric Biggers
@ 2016-04-03  5:21 ` Eric Biggers
  2016-04-03  5:21 ` [PATCH 07/13] fscrypto: simplify building key descriptor string Eric Biggers
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Biggers @ 2016-04-03  5:21 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: tytso, Eric Biggers, mhalcrow, linux-kernel, linux-f2fs-devel,
	jaegeuk, linux-ext4

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 fs/crypto/keyinfo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 33296c0..cb06351 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -208,8 +208,8 @@ retry:
 		goto out;
 got_key:
 	ctfm = crypto_alloc_skcipher(cipher_str, 0, 0);
-	if (!ctfm || IS_ERR(ctfm)) {
-		res = ctfm ? PTR_ERR(ctfm) : -ENOMEM;
+	if (IS_ERR(ctfm)) {
+		res = PTR_ERR(ctfm);
 		printk(KERN_DEBUG
 		       "%s: error %d (inode %u) allocating crypto tfm\n",
 		       __func__, res, (unsigned) inode->i_ino);
-- 
2.7.4


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 07/13] fscrypto: simplify building key descriptor string
  2016-04-03  5:21 [PATCH 00/13] fscrypto: cleanups and fixes Eric Biggers
                   ` (5 preceding siblings ...)
  2016-04-03  5:21 ` [PATCH 06/13] fscrypto: crypto_alloc_skcipher() always returns an ERR_PTR(), never NULL Eric Biggers
@ 2016-04-03  5:21 ` Eric Biggers
  2016-04-03  5:21 ` [PATCH 08/13] fscrypto: use standard macros from kernel.h Eric Biggers
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Biggers @ 2016-04-03  5:21 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: tytso, Eric Biggers, mhalcrow, linux-kernel, linux-f2fs-devel,
	jaegeuk, linux-ext4

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 fs/crypto/keyinfo.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index cb06351..5fcee4d 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -164,13 +164,8 @@ retry:
 		memset(raw_key, 0x42, FS_AES_256_XTS_KEY_SIZE);
 		goto got_key;
 	}
-	memcpy(full_key_descriptor, FS_KEY_DESC_PREFIX,
-					FS_KEY_DESC_PREFIX_SIZE);
-	sprintf(full_key_descriptor + FS_KEY_DESC_PREFIX_SIZE,
-					"%*phN", FS_KEY_DESCRIPTOR_SIZE,
-					ctx.master_key_descriptor);
-	full_key_descriptor[FS_KEY_DESC_PREFIX_SIZE +
-					(2 * FS_KEY_DESCRIPTOR_SIZE)] = '\0';
+	sprintf(full_key_descriptor, FS_KEY_DESC_PREFIX "%*phN",
+		FS_KEY_DESCRIPTOR_SIZE, ctx.master_key_descriptor);
 	keyring_key = request_key(&key_type_logon, full_key_descriptor, NULL);
 	if (IS_ERR(keyring_key)) {
 		res = PTR_ERR(keyring_key);
-- 
2.7.4


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 08/13] fscrypto: use standard macros from kernel.h
  2016-04-03  5:21 [PATCH 00/13] fscrypto: cleanups and fixes Eric Biggers
                   ` (6 preceding siblings ...)
  2016-04-03  5:21 ` [PATCH 07/13] fscrypto: simplify building key descriptor string Eric Biggers
@ 2016-04-03  5:21 ` Eric Biggers
  2016-04-03  5:22 ` [PATCH 09/13] fscrypto: make fname_encrypt() actually return length of ciphertext Eric Biggers
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Biggers @ 2016-04-03  5:21 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: tytso, Eric Biggers, mhalcrow, linux-kernel, linux-f2fs-devel,
	jaegeuk, linux-ext4

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 fs/crypto/fname.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index cd0eae8..e5c6959 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -14,11 +14,6 @@
 #include <linux/ratelimit.h>
 #include <linux/fscrypto.h>
 
-static u32 size_round_up(size_t size, size_t blksize)
-{
-	return ((size + blksize - 1) / blksize) * blksize;
-}
-
 /**
  * fname_crypt_complete() - The completion callback for filename encryption and
  *			    decryption
@@ -61,10 +56,9 @@ static int fname_encrypt(struct inode *inode,
 	if (iname->len <= 0 || iname->len > lim)
 		return -EIO;
 
-	ciphertext_len = (iname->len < FS_CRYPTO_BLOCK_SIZE) ?
-					FS_CRYPTO_BLOCK_SIZE : iname->len;
-	ciphertext_len = size_round_up(ciphertext_len, padding);
-	ciphertext_len = (ciphertext_len > lim) ? lim : ciphertext_len;
+	ciphertext_len = max_t(u32, iname->len, FS_CRYPTO_BLOCK_SIZE);
+	ciphertext_len = round_up(ciphertext_len, padding);
+	ciphertext_len = min(ciphertext_len, lim);
 
 	if (ciphertext_len <= sizeof(buf)) {
 		workbuf = buf;
@@ -235,9 +229,8 @@ u32 fscrypt_fname_encrypted_size(struct inode *inode, u32 ilen)
 
 	if (ci)
 		padding = 4 << (ci->ci_flags & FS_POLICY_FLAGS_PAD_MASK);
-	if (ilen < FS_CRYPTO_BLOCK_SIZE)
-		ilen = FS_CRYPTO_BLOCK_SIZE;
-	return size_round_up(ilen, padding);
+	ilen = max_t(u32, ilen, FS_CRYPTO_BLOCK_SIZE);
+	return round_up(ilen, padding);
 }
 EXPORT_SYMBOL(fscrypt_fname_encrypted_size);
 
-- 
2.7.4


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 09/13] fscrypto: make fname_encrypt() actually return length of ciphertext
  2016-04-03  5:21 [PATCH 00/13] fscrypto: cleanups and fixes Eric Biggers
                   ` (7 preceding siblings ...)
  2016-04-03  5:21 ` [PATCH 08/13] fscrypto: use standard macros from kernel.h Eric Biggers
@ 2016-04-03  5:22 ` Eric Biggers
  2016-04-03  5:22 ` [PATCH 10/13] fscrypto: restrict setting new policy to empty files and directories only Eric Biggers
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Biggers @ 2016-04-03  5:22 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: tytso, Eric Biggers, mhalcrow, linux-kernel, linux-f2fs-devel,
	jaegeuk, linux-ext4

This makes the return value match the comment.  Previously it would
actually return 0 if encryption was successful.  (No callers currently
care.)

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 fs/crypto/fname.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index e5c6959..5b10b73 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -100,12 +100,13 @@ static int fname_encrypt(struct inode *inode,
 	}
 	kfree(alloc_buf);
 	skcipher_request_free(req);
-	if (res < 0)
+	if (res < 0) {
 		printk_ratelimited(KERN_ERR
 				"%s: Error (error code %d)\n", __func__, res);
-
+		return res;
+	}
 	oname->len = ciphertext_len;
-	return res;
+	return ciphertext_len;
 }
 
 /*
-- 
2.7.4


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 10/13] fscrypto: restrict setting new policy to empty files and directories only
  2016-04-03  5:21 [PATCH 00/13] fscrypto: cleanups and fixes Eric Biggers
                   ` (8 preceding siblings ...)
  2016-04-03  5:22 ` [PATCH 09/13] fscrypto: make fname_encrypt() actually return length of ciphertext Eric Biggers
@ 2016-04-03  5:22 ` Eric Biggers
  2016-04-03  5:22 ` [PATCH 11/13] fscrypto: restrict setting encryption policy to inode owner Eric Biggers
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Biggers @ 2016-04-03  5:22 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: tytso, Eric Biggers, mhalcrow, linux-kernel, linux-f2fs-devel,
	jaegeuk, linux-ext4

On f2fs, a user could create a regular file of small positive size and
issue FS_IOC_SET_ENCRYPTION_POLICY to set its encryption policy.
However, this did not behave as expected because the existing data was
not actually encrypted by the ioctl.

Fix this by only permitting an encryption policy to be created for empty
regular files and directories.

For a correct solution, it is necessary to conduct the operation under
the inode lock; otherwise the inode's size might be changed concurrently.

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 fs/crypto/policy.c | 42 ++++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 93244b5..cb5ba27 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -94,23 +94,41 @@ static int create_encryption_context_from_policy(struct inode *inode,
 
 int fscrypt_set_policy(struct inode *inode, const struct fscrypt_policy *policy)
 {
+	int ret = 0;
+
 	if (policy->version != 0)
 		return -EINVAL;
 
+	inode_lock(inode);
+
 	if (!inode_has_encryption_context(inode)) {
-		if (!inode->i_sb->s_cop->empty_dir)
-			return -EOPNOTSUPP;
-		if (!inode->i_sb->s_cop->empty_dir(inode))
-			return -ENOTEMPTY;
-		return create_encryption_context_from_policy(inode, policy);
+		/* A new policy may only be set on an empty directory or an
+		 * empty regular file. */
+		ret = -EINVAL;
+		if (S_ISDIR(inode->i_mode)) {
+			if (!inode->i_sb->s_cop->empty_dir)
+				ret = -EOPNOTSUPP;
+			else if (inode->i_sb->s_cop->empty_dir(inode))
+				ret = 0;
+			else
+				ret = -ENOTEMPTY;
+		} else if (S_ISREG(inode->i_mode)) {
+			ret = -ENOTEMPTY;
+			if (inode->i_size == 0)
+				ret = 0;
+		}
+		if (!ret) {
+			ret = create_encryption_context_from_policy(inode,
+								    policy);
+		}
+	} else if (!is_encryption_context_consistent_with_policy(inode, policy)) {
+		printk(KERN_WARNING
+		       "%s: Policy inconsistent with encryption context\n",
+		       __func__);
+		ret = -EINVAL;
 	}
-
-	if (is_encryption_context_consistent_with_policy(inode, policy))
-		return 0;
-
-	printk(KERN_WARNING "%s: Policy inconsistent with encryption context\n",
-	       __func__);
-	return -EINVAL;
+	inode_unlock(inode);
+	return ret;
 }
 EXPORT_SYMBOL(fscrypt_set_policy);
 
-- 
2.7.4


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 11/13] fscrypto: restrict setting encryption policy to inode owner
  2016-04-03  5:21 [PATCH 00/13] fscrypto: cleanups and fixes Eric Biggers
                   ` (9 preceding siblings ...)
  2016-04-03  5:22 ` [PATCH 10/13] fscrypto: restrict setting new policy to empty files and directories only Eric Biggers
@ 2016-04-03  5:22 ` Eric Biggers
  2016-04-03  5:22 ` [PATCH 12/13] fscrypto: require write access to mount to set encryption policy Eric Biggers
  2016-04-03  5:22 ` [PATCH 13/13] fscrypto: improve error handling in fscrypt_set_policy() Eric Biggers
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Biggers @ 2016-04-03  5:22 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: tytso, Eric Biggers, mhalcrow, linux-kernel, linux-f2fs-devel,
	jaegeuk, linux-ext4

On a filesystem with encryption enabled, a user could set an encryption
policy on any empty directory to which they have readonly access.  This
is a potential security issue since such a directory might be owned by
another user, and the new encryption policy may prevent that user from
creating files in their own directory.

Fix this by requiring inode_owner_or_capable() permission to set an
encryption policy.

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 fs/crypto/policy.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index cb5ba27..3f5c275 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -96,6 +96,9 @@ int fscrypt_set_policy(struct inode *inode, const struct fscrypt_policy *policy)
 {
 	int ret = 0;
 
+	if (!inode_owner_or_capable(inode))
+		return -EACCES;
+
 	if (policy->version != 0)
 		return -EINVAL;
 
-- 
2.7.4


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 12/13] fscrypto: require write access to mount to set encryption policy
  2016-04-03  5:21 [PATCH 00/13] fscrypto: cleanups and fixes Eric Biggers
                   ` (10 preceding siblings ...)
  2016-04-03  5:22 ` [PATCH 11/13] fscrypto: restrict setting encryption policy to inode owner Eric Biggers
@ 2016-04-03  5:22 ` Eric Biggers
  2016-04-03  5:22 ` [PATCH 13/13] fscrypto: improve error handling in fscrypt_set_policy() Eric Biggers
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Biggers @ 2016-04-03  5:22 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: tytso, Eric Biggers, mhalcrow, linux-kernel, linux-f2fs-devel,
	jaegeuk, linux-ext4

Since setting an encryption policy requires writing data to the
filesystem, it should be guarded by mnt_want_write/mnt_drop_write.
Otherwise, a user could cause a write to a readonly or frozen filesystem.

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 fs/crypto/policy.c       | 11 +++++++++--
 fs/f2fs/file.c           |  2 +-
 include/linux/fscrypto.h |  4 ++--
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 3f5c275..6a767e6 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -11,6 +11,7 @@
 #include <linux/random.h>
 #include <linux/string.h>
 #include <linux/fscrypto.h>
+#include <linux/mount.h>
 
 static bool inode_has_encryption_context(struct inode *inode)
 {
@@ -92,9 +93,10 @@ static int create_encryption_context_from_policy(struct inode *inode,
 	return inode->i_sb->s_cop->set_context(inode, &ctx, sizeof(ctx), NULL);
 }
 
-int fscrypt_set_policy(struct inode *inode, const struct fscrypt_policy *policy)
+int fscrypt_set_policy(struct file *file, const struct fscrypt_policy *policy)
 {
-	int ret = 0;
+	struct inode *inode = file_inode(file);
+	int ret;
 
 	if (!inode_owner_or_capable(inode))
 		return -EACCES;
@@ -102,6 +104,10 @@ int fscrypt_set_policy(struct inode *inode, const struct fscrypt_policy *policy)
 	if (policy->version != 0)
 		return -EINVAL;
 
+	ret = mnt_want_write_file(file);
+	if (ret)
+		return ret;
+
 	inode_lock(inode);
 
 	if (!inode_has_encryption_context(inode)) {
@@ -131,6 +137,7 @@ int fscrypt_set_policy(struct inode *inode, const struct fscrypt_policy *policy)
 		ret = -EINVAL;
 	}
 	inode_unlock(inode);
+	mnt_drop_write_file(file);
 	return ret;
 }
 EXPORT_SYMBOL(fscrypt_set_policy);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index cf691ae..d4837280 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1542,7 +1542,7 @@ static int f2fs_ioc_set_encryption_policy(struct file *filp, unsigned long arg)
 		return -EFAULT;
 
 	f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
-	return fscrypt_set_policy(inode, &policy);
+	return fscrypt_set_policy(filp, &policy);
 }
 
 static int f2fs_ioc_get_encryption_policy(struct file *filp, unsigned long arg)
diff --git a/include/linux/fscrypto.h b/include/linux/fscrypto.h
index f29dc8c..130bf23 100644
--- a/include/linux/fscrypto.h
+++ b/include/linux/fscrypto.h
@@ -314,7 +314,7 @@ extern void fscrypt_restore_control_page(struct page *);
 extern int fscrypt_zeroout_range(struct inode *, pgoff_t, sector_t,
 						unsigned int);
 /* policy.c */
-extern int fscrypt_set_policy(struct inode *, const struct fscrypt_policy *);
+extern int fscrypt_set_policy(struct file *, const struct fscrypt_policy *);
 extern int fscrypt_get_policy(struct inode *, struct fscrypt_policy *);
 extern bool fscrypt_has_permitted_context(struct inode *, struct inode *);
 extern int fscrypt_inherit_context(struct inode *, struct inode *,
@@ -384,7 +384,7 @@ static inline int fscrypt_notsupp_zeroout_range(struct inode *i, pgoff_t p,
 }
 
 /* policy.c */
-static inline int fscrypt_notsupp_set_policy(struct inode *i,
+static inline int fscrypt_notsupp_set_policy(struct file *f,
 				const struct fscrypt_policy *p)
 {
 	return -EOPNOTSUPP;
-- 
2.7.4


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 13/13] fscrypto: improve error handling in fscrypt_set_policy()
  2016-04-03  5:21 [PATCH 00/13] fscrypto: cleanups and fixes Eric Biggers
                   ` (11 preceding siblings ...)
  2016-04-03  5:22 ` [PATCH 12/13] fscrypto: require write access to mount to set encryption policy Eric Biggers
@ 2016-04-03  5:22 ` Eric Biggers
  12 siblings, 0 replies; 15+ messages in thread
From: Eric Biggers @ 2016-04-03  5:22 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: tytso, Eric Biggers, mhalcrow, linux-kernel, linux-f2fs-devel,
	jaegeuk, linux-ext4

In some cases, the previous code did not correctly propagate errors to
the caller.  Also, only one call to ->get_context() is required.

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 fs/crypto/policy.c | 64 +++++++++++++++++++++++++-----------------------------
 1 file changed, 29 insertions(+), 35 deletions(-)

diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 6a767e6..83421fe 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -13,37 +13,17 @@
 #include <linux/fscrypto.h>
 #include <linux/mount.h>
 
-static bool inode_has_encryption_context(struct inode *inode)
-{
-	if (!inode->i_sb->s_cop->get_context)
-		return false;
-	return (inode->i_sb->s_cop->get_context(inode, NULL, 0L) > 0);
-}
-
-/*
- * check whether the policy is consistent with the encryption context
- * for the inode
- */
-static bool is_encryption_context_consistent_with_policy(struct inode *inode,
+static bool is_encryption_context_consistent_with_policy(
+				const struct fscrypt_context *ctx,
 				const struct fscrypt_policy *policy)
 {
-	struct fscrypt_context ctx;
-	int res;
-
-	if (!inode->i_sb->s_cop->get_context)
-		return false;
-
-	res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
-	if (res != sizeof(ctx))
-		return false;
-
-	return (memcmp(ctx.master_key_descriptor, policy->master_key_descriptor,
-			FS_KEY_DESCRIPTOR_SIZE) == 0 &&
-			(ctx.flags == policy->flags) &&
-			(ctx.contents_encryption_mode ==
-			 policy->contents_encryption_mode) &&
-			(ctx.filenames_encryption_mode ==
-			 policy->filenames_encryption_mode));
+	return memcmp(ctx->master_key_descriptor, policy->master_key_descriptor,
+		      FS_KEY_DESCRIPTOR_SIZE) == 0 &&
+		(ctx->flags == policy->flags) &&
+		(ctx->contents_encryption_mode ==
+		 policy->contents_encryption_mode) &&
+		(ctx->filenames_encryption_mode ==
+		 policy->filenames_encryption_mode);
 }
 
 static int create_encryption_context_from_policy(struct inode *inode,
@@ -96,6 +76,7 @@ static int create_encryption_context_from_policy(struct inode *inode,
 int fscrypt_set_policy(struct file *file, const struct fscrypt_policy *policy)
 {
 	struct inode *inode = file_inode(file);
+	struct fscrypt_context existing;
 	int ret;
 
 	if (!inode_owner_or_capable(inode))
@@ -110,7 +91,25 @@ int fscrypt_set_policy(struct file *file, const struct fscrypt_policy *policy)
 
 	inode_lock(inode);
 
-	if (!inode_has_encryption_context(inode)) {
+	ret = -ENODATA;
+	if (inode->i_sb->s_cop->get_context) {
+		ret = inode->i_sb->s_cop->get_context(inode, &existing,
+						      sizeof(existing));
+	}
+	if (ret != -ENODATA) {
+		/* An existing policy cannot be changed.  However, an attempt to
+		 * set an identical policy will succeed. */
+		if (ret == sizeof(existing) &&
+		    is_encryption_context_consistent_with_policy(&existing,
+								 policy)) {
+			ret = 0;
+		} else if (ret >= 0 || ret == -ERANGE) {
+			printk(KERN_WARNING
+			       "%s: Policy inconsistent with encryption context\n",
+			       __func__);
+			ret = -EINVAL;
+		}
+	} else {
 		/* A new policy may only be set on an empty directory or an
 		 * empty regular file. */
 		ret = -EINVAL;
@@ -130,11 +129,6 @@ int fscrypt_set_policy(struct file *file, const struct fscrypt_policy *policy)
 			ret = create_encryption_context_from_policy(inode,
 								    policy);
 		}
-	} else if (!is_encryption_context_consistent_with_policy(inode, policy)) {
-		printk(KERN_WARNING
-		       "%s: Policy inconsistent with encryption context\n",
-		       __func__);
-		ret = -EINVAL;
 	}
 	inode_unlock(inode);
 	mnt_drop_write_file(file);
-- 
2.7.4


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 02/13] fscrypto: rename some functions for clarity
  2016-04-03  5:21 ` [PATCH 02/13] fscrypto: rename some functions for clarity Eric Biggers
@ 2016-04-03  7:45   ` Theodore Ts'o
  0 siblings, 0 replies; 15+ messages in thread
From: Theodore Ts'o @ 2016-04-03  7:45 UTC (permalink / raw)
  To: Eric Biggers
  Cc: mhalcrow, linux-kernel, linux-f2fs-devel, linux-fsdevel, jaegeuk,
	linux-ext4

On Sun, Apr 03, 2016 at 12:21:53AM -0500, Eric Biggers wrote:
> Rename fscrypt_complete() to page_crypt_complete().  This callback is
> specifically for data pages; fscrypto also performs filename encryption.
> 
> Rename dir_crypt_complete() to fname_crypt_complete().  This callback is
> also used for symlink targets, not just directory entries.
> 
> Rename fscrypt_process_policy() to fscrypt_set_policy().  The new name
> better matches the ioctl, and it goes along with fscrypt_get_policy().
> 
> Signed-off-by: Eric Biggers <ebiggers3@gmail.com>

I'd really rather not renaming any globally visible functions until
after ext4 is converted over to use fs/crypto, just to avoid confusion.

Cheers,

					- Ted

------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-04-03  7:45 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-03  5:21 [PATCH 00/13] fscrypto: cleanups and fixes Eric Biggers
2016-04-03  5:21 ` [PATCH 01/13] fscrypto: remove unnecessary includes Eric Biggers
2016-04-03  5:21 ` [PATCH 02/13] fscrypto: rename some functions for clarity Eric Biggers
2016-04-03  7:45   ` Theodore Ts'o
2016-04-03  5:21 ` [PATCH 03/13] fscrypto: rename functions to load and unload inode encryption info Eric Biggers
2016-04-03  5:21 ` [PATCH 04/13] fscrypto: return bool instead of int where appropriate Eric Biggers
2016-04-03  5:21 ` [PATCH 05/13] fscrypto: comment improvements and fixes Eric Biggers
2016-04-03  5:21 ` [PATCH 06/13] fscrypto: crypto_alloc_skcipher() always returns an ERR_PTR(), never NULL Eric Biggers
2016-04-03  5:21 ` [PATCH 07/13] fscrypto: simplify building key descriptor string Eric Biggers
2016-04-03  5:21 ` [PATCH 08/13] fscrypto: use standard macros from kernel.h Eric Biggers
2016-04-03  5:22 ` [PATCH 09/13] fscrypto: make fname_encrypt() actually return length of ciphertext Eric Biggers
2016-04-03  5:22 ` [PATCH 10/13] fscrypto: restrict setting new policy to empty files and directories only Eric Biggers
2016-04-03  5:22 ` [PATCH 11/13] fscrypto: restrict setting encryption policy to inode owner Eric Biggers
2016-04-03  5:22 ` [PATCH 12/13] fscrypto: require write access to mount to set encryption policy Eric Biggers
2016-04-03  5:22 ` [PATCH 13/13] fscrypto: improve error handling in fscrypt_set_policy() Eric Biggers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).