From 0bdf8a8245fdea6f075a5fede833a5fcf1b3466c Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 4 Jul 2018 12:35:56 +0300 Subject: [PATCH 1/7] eCryptfs: fix a couple type promotion bugs ECRYPTFS_SIZE_AND_MARKER_BYTES is type size_t, so if "rc" is negative that gets type promoted to a high positive value and treated as success. Fixes: 778aeb42a708 ("eCryptfs: Cleanup and optimize ecryptfs_lookup_interpose()") Signed-off-by: Dan Carpenter [tyhicks: Use "if/else if" rather than "if/if"] Cc: stable@vger.kernel.org Signed-off-by: Tyler Hicks --- fs/ecryptfs/crypto.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index 4dd842f72846..708f931c36f1 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -1018,8 +1018,10 @@ int ecryptfs_read_and_validate_header_region(struct inode *inode) rc = ecryptfs_read_lower(file_size, 0, ECRYPTFS_SIZE_AND_MARKER_BYTES, inode); - if (rc < ECRYPTFS_SIZE_AND_MARKER_BYTES) - return rc >= 0 ? -EINVAL : rc; + if (rc < 0) + return rc; + else if (rc < ECRYPTFS_SIZE_AND_MARKER_BYTES) + return -EINVAL; rc = ecryptfs_validate_marker(marker); if (!rc) ecryptfs_i_size_init(file_size, inode); @@ -1381,8 +1383,10 @@ int ecryptfs_read_and_validate_xattr_region(struct dentry *dentry, ecryptfs_inode_to_lower(inode), ECRYPTFS_XATTR_NAME, file_size, ECRYPTFS_SIZE_AND_MARKER_BYTES); - if (rc < ECRYPTFS_SIZE_AND_MARKER_BYTES) - return rc >= 0 ? -EINVAL : rc; + if (rc < 0) + return rc; + else if (rc < ECRYPTFS_SIZE_AND_MARKER_BYTES) + return -EINVAL; rc = ecryptfs_validate_marker(marker); if (!rc) ecryptfs_i_size_init(file_size, inode); From 4b47a8b51e7bc0bcd1fa8e546a6333a04ab760d8 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 22 Aug 2018 13:43:59 +0300 Subject: [PATCH 2/7] ecryptfs: re-order a condition for static checkers Static checkers complain that we are using "s->i" as an offset before we check whether it is within bounds. It doesn't matter much but we can easily swap the order of the checks to make everyone happy. Signed-off-by: Dan Carpenter Signed-off-by: Tyler Hicks --- fs/ecryptfs/keystore.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c index e74fe84d0886..624ff4409c61 100644 --- a/fs/ecryptfs/keystore.c +++ b/fs/ecryptfs/keystore.c @@ -1063,8 +1063,9 @@ ecryptfs_parse_tag_70_packet(char **filename, size_t *filename_size, "rc = [%d]\n", __func__, rc); goto out_free_unlock; } - while (s->decrypted_filename[s->i] != '\0' - && s->i < s->block_aligned_filename_size) + + while (s->i < s->block_aligned_filename_size && + s->decrypted_filename[s->i] != '\0') s->i++; if (s->i == s->block_aligned_filename_size) { printk(KERN_WARNING "%s: Invalid tag 70 packet; could not " From d43388dea04b18f516bd7c837d9222fe7309b12d Mon Sep 17 00:00:00 2001 From: Robbie Ko Date: Tue, 21 Aug 2018 16:17:40 +0800 Subject: [PATCH 3/7] eCryptfs: fix permission denied with ecryptfs_xattr mount option when create readonly file When the ecryptfs_xattr mount option is turned on, the ecryptfs metadata will be written to xattr via vfs_setxattr, which will check the WRITE permissions. However, this will cause denial of permission when creating a file withoug write permission. So fix this by calling __vfs_setxattr directly to skip permission check. Signed-off-by: Robbie Ko [tyhicks: Copy up lower inode attributes when successful] Signed-off-by: Tyler Hicks --- fs/ecryptfs/crypto.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index 708f931c36f1..bc2376726090 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -37,6 +37,7 @@ #include #include #include +#include #include "ecryptfs_kernel.h" #define DECRYPT 0 @@ -1131,9 +1132,21 @@ ecryptfs_write_metadata_to_xattr(struct dentry *ecryptfs_dentry, char *page_virt, size_t size) { int rc; + struct dentry *lower_dentry = ecryptfs_dentry_to_lower(ecryptfs_dentry); + struct inode *lower_inode = d_inode(lower_dentry); - rc = ecryptfs_setxattr(ecryptfs_dentry, ecryptfs_inode, - ECRYPTFS_XATTR_NAME, page_virt, size, 0); + if (!(lower_inode->i_opflags & IOP_XATTR)) { + rc = -EOPNOTSUPP; + goto out; + } + + inode_lock(lower_inode); + rc = __vfs_setxattr(lower_dentry, lower_inode, ECRYPTFS_XATTR_NAME, + page_virt, size, 0); + if (!rc && ecryptfs_inode) + fsstack_copy_attr_all(ecryptfs_inode, lower_inode); + inode_unlock(lower_inode); +out: return rc; } From 96827c3044cf2562af2f5f09988f48acdbee25c7 Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Fri, 17 May 2019 12:45:15 +0200 Subject: [PATCH 4/7] ecryptfs: use print_hex_dump_bytes for hexdump The Kernel has nice hexdump facilities, use them rather a homebrew hexdump function. Signed-off-by: Sascha Hauer Signed-off-by: Tyler Hicks --- fs/ecryptfs/debug.c | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/fs/ecryptfs/debug.c b/fs/ecryptfs/debug.c index 3d2bdf546ec6..ee9d8ac4a809 100644 --- a/fs/ecryptfs/debug.c +++ b/fs/ecryptfs/debug.c @@ -97,25 +97,9 @@ void ecryptfs_dump_auth_tok(struct ecryptfs_auth_tok *auth_tok) */ void ecryptfs_dump_hex(char *data, int bytes) { - int i = 0; - int add_newline = 1; - if (ecryptfs_verbosity < 1) return; - if (bytes != 0) { - printk(KERN_DEBUG "0x%.2x.", (unsigned char)data[i]); - i++; - } - while (i < bytes) { - printk("0x%.2x.", (unsigned char)data[i]); - i++; - if (i % 16 == 0) { - printk("\n"); - add_newline = 0; - } else - add_newline = 1; - } - if (add_newline) - printk("\n"); -} + print_hex_dump(KERN_DEBUG, "ecryptfs: ", DUMP_PREFIX_OFFSET, 16, 1, + data, bytes, false); +} From 29a51df0609c74e7163d41334021166d6a34d083 Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Mon, 27 May 2019 21:28:14 +0800 Subject: [PATCH 5/7] ecryptfs: remove unnessesary null check in ecryptfs_keyring_auth_tok_for_sig request_key and ecryptfs_get_encrypted_key never return a NULL pointer, so no need do a null check. Signed-off-by: YueHaibing Signed-off-by: Tyler Hicks --- fs/ecryptfs/keystore.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c index 624ff4409c61..770cb8f6f697 100644 --- a/fs/ecryptfs/keystore.c +++ b/fs/ecryptfs/keystore.c @@ -1627,9 +1627,9 @@ int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key, int rc = 0; (*auth_tok_key) = request_key(&key_type_user, sig, NULL); - if (!(*auth_tok_key) || IS_ERR(*auth_tok_key)) { + if (IS_ERR(*auth_tok_key)) { (*auth_tok_key) = ecryptfs_get_encrypted_key(sig); - if (!(*auth_tok_key) || IS_ERR(*auth_tok_key)) { + if (IS_ERR(*auth_tok_key)) { printk(KERN_ERR "Could not find key with description: [%s]\n", sig); rc = process_request_key_err(PTR_ERR(*auth_tok_key)); From c036061be907b8bcf8c030cf4d2fb97a04d6f5d1 Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Fri, 14 Jun 2019 23:51:17 +0800 Subject: [PATCH 6/7] ecryptfs: Make ecryptfs_xattr_handler static Fix sparse warning: fs/ecryptfs/inode.c:1138:28: warning: symbol 'ecryptfs_xattr_handler' was not declared. Should it be static? Reported-by: Hulk Robot Signed-off-by: YueHaibing Signed-off-by: Tyler Hicks --- fs/ecryptfs/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index 5c36ceecb5c1..c9b5b59cbd48 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -1135,7 +1135,7 @@ static int ecryptfs_xattr_set(const struct xattr_handler *handler, } } -const struct xattr_handler ecryptfs_xattr_handler = { +static const struct xattr_handler ecryptfs_xattr_handler = { .prefix = "", /* match anything */ .get = ecryptfs_xattr_get, .set = ecryptfs_xattr_set, From 7451c54abc9139585492605d9e91dec2d26c6457 Mon Sep 17 00:00:00 2001 From: Hariprasad Kelam Date: Tue, 2 Jul 2019 23:17:24 +0530 Subject: [PATCH 7/7] ecryptfs: Change return type of ecryptfs_process_flags Change return type of ecryptfs_process_flags from int to void as it never fails. fixes below issue reported by coccicheck s/ecryptfs/crypto.c:870:5-7: Unneeded variable: "rc". Return "0" on line 883 Signed-off-by: Hariprasad Kelam [tyhicks: Remove the return value line from the function documentation] Signed-off-by: Tyler Hicks --- fs/ecryptfs/crypto.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index bc2376726090..d8e47b75bc57 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -875,13 +875,10 @@ static struct ecryptfs_flag_map_elem ecryptfs_flag_map[] = { * @crypt_stat: The cryptographic context * @page_virt: Source data to be parsed * @bytes_read: Updated with the number of bytes read - * - * Returns zero on success; non-zero if the flag set is invalid */ -static int ecryptfs_process_flags(struct ecryptfs_crypt_stat *crypt_stat, +static void ecryptfs_process_flags(struct ecryptfs_crypt_stat *crypt_stat, char *page_virt, int *bytes_read) { - int rc = 0; int i; u32 flags; @@ -894,7 +891,6 @@ static int ecryptfs_process_flags(struct ecryptfs_crypt_stat *crypt_stat, /* Version is in top 8 bits of the 32-bit flag vector */ crypt_stat->file_version = ((flags >> 24) & 0xFF); (*bytes_read) = 4; - return rc; } /** @@ -1320,12 +1316,7 @@ static int ecryptfs_read_headers_virt(char *page_virt, if (!(crypt_stat->flags & ECRYPTFS_I_SIZE_INITIALIZED)) ecryptfs_i_size_init(page_virt, d_inode(ecryptfs_dentry)); offset += MAGIC_ECRYPTFS_MARKER_SIZE_BYTES; - rc = ecryptfs_process_flags(crypt_stat, (page_virt + offset), - &bytes_read); - if (rc) { - ecryptfs_printk(KERN_WARNING, "Error processing flags\n"); - goto out; - } + ecryptfs_process_flags(crypt_stat, (page_virt + offset), &bytes_read); if (crypt_stat->file_version > ECRYPTFS_SUPPORTED_FILE_VERSION) { ecryptfs_printk(KERN_WARNING, "File version is [%d]; only " "file version [%d] is supported by this "