Re: [PATCH] ext4 crypto: handle ENOKEY correctly
From: Dmitry Monakhov <hidden>
Date: 2015-06-01 10:01:46
Subsystem:
ext4 file system, filesystems (vfs and infrastructure), the rest · Maintainers:
"Theodore Ts'o", Alexander Viro, Christian Brauner, Linus Torvalds
Theodore Ts'o [off-list ref] writes:
On Fri, May 29, 2015 at 04:44:29PM -0400, Theodore Ts'o wrote:quoted
I don't think that's the right way to go. We should add checks to ext4_file_open, sure. But the problem is that i_crypt_info can get set to NULL after the file is succesfully opened. So we need to handle i_crypt_info being NULL everywhere. So the BUG_ON() in ext4_get_crypto_ctx() needs to be replaced with: if (ci == NULL) return ERR_PTR(-ENOKEY);This is what I had in mind....
ACK. with one note, you forget to convert all callers of ext4_get_crypto_ctx() to new error convention. Please see patch below.
diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
index a9fd028..6f9d95e 100644
--- a/fs/ext4/crypto.c
+++ b/fs/ext4/crypto.c@@ -467,8 +467,9 @@ int ext4_decrypt_one(struct inode *inode, struct page *page) struct ext4_crypto_ctx *ctx = ext4_get_crypto_ctx(inode); - if (!ctx) - return -ENOMEM; + if (IS_ERR(ctx)) + return PTR_ERR(ctx); + ret = ext4_decrypt(ctx, page); ext4_release_crypto_ctx(ctx); return ret;
quoted hunk ↗ jump to hunk
- Ted commit 3cde0c5d3125697c4b02c32054a378dc71ebfdb5 Author: Theodore Ts'o [off-list ref] Date: Sun May 31 08:12:34 2015 -0400 ext4 crypto: handle unexpected lack of encryption keys Fix up attempts by users to try to write to a file when they don't have access to the encryption key. Signed-off-by: Theodore Ts'o [off-list ref]diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c index ac2419c..6634478 100644 --- a/fs/ext4/crypto.c +++ b/fs/ext4/crypto.c@@ -104,7 +104,8 @@ struct ext4_crypto_ctx *ext4_get_crypto_ctx(struct inode *inode) unsigned long flags; struct ext4_crypt_info *ci = EXT4_I(inode)->i_crypt_info; - BUG_ON(ci == NULL); + if (ci == NULL) + return ERR_PTR(-ENOKEY); /* * We first try getting the ctx from a free list because indiff --git a/fs/ext4/crypto_policy.c b/fs/ext4/crypto_policy.c index a1d434d..02c4e5d 100644 --- a/fs/ext4/crypto_policy.c +++ b/fs/ext4/crypto_policy.c@@ -183,7 +183,8 @@ int ext4_inherit_context(struct inode *parent, struct inode *child) if (res < 0) return res; ci = EXT4_I(parent)->i_crypt_info; - BUG_ON(ci == NULL); + if (ci == NULL) + return -ENOKEY; ctx.format = EXT4_ENCRYPTION_CONTEXT_FORMAT_V1; if (DUMMY_ENCRYPTION_ENABLED(EXT4_SB(parent->i_sb))) {diff --git a/fs/ext4/file.c b/fs/ext4/file.c index 875ca6b..ac517f1 100644 --- a/fs/ext4/file.c +++ b/fs/ext4/file.c@@ -226,6 +226,8 @@ static int ext4_file_mmap(struct file *file, struct vm_area_struct *vma) int err = ext4_get_encryption_info(inode); if (err) return 0; + if (ext4_encryption_info(inode) == NULL) + return -ENOKEY; } file_accessed(file); if (IS_DAX(file_inode(file))) {@@ -278,6 +280,13 @@ static int ext4_file_open(struct inode * inode, struct file * filp) ext4_journal_stop(handle); } } + if (ext4_encrypted_inode(inode)) { + ret = ext4_get_encryption_info(inode); + if (ret) + return -EACCES; + if (ext4_encryption_info(inode) == NULL) + return -ENOKEY; + } /* * Set up the jbd2_inode if we are opening the inode for * writing and the journal is present@@ -287,13 +296,7 @@ static int ext4_file_open(struct inode * inode, struct file * filp) if (ret < 0) return ret; } - ret = dquot_file_open(inode, filp); - if (!ret && ext4_encrypted_inode(inode)) { - ret = ext4_get_encryption_info(inode); - if (ret) - ret = -EACCES; - } - return ret; + return dquot_file_open(inode, filp); } /*
Attachments
- signature.asc [application/pgp-signature] 472 bytes