Theodore Ts'o <tytso@xxxxxxx> writes:
> On Fri, May 29, 2015 at 04:44:29PM -0400, Theodore Ts'o wrote:
>> 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;
>
> - Ted
>
> commit 3cde0c5d3125697c4b02c32054a378dc71ebfdb5
> Author: Theodore Ts'o <tytso@xxxxxxx>
> 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 <tytso@xxxxxxx>
>
> 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 in
> diff --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);
> }
>
> /*
Attachment:
signature.asc
Description: PGP signature
