[1/6] f2fs: check entire encrypted bigname when finding a dentry
diff mbox

Message ID 20170424170013.85175-2-ebiggers3@gmail.com
State Accepted
Headers show

Commit Message

Eric Biggers April 24, 2017, 5 p.m. UTC
From: Jaegeuk Kim <jaegeuk@kernel.org>

If user has no key under an encrypted dir, fscrypt gives digested dentries.
Previously, when looking up a dentry, f2fs only checks its hash value with
first 4 bytes of the digested dentry, which didn't handle hash collisions fully.
This patch enhances to check entire dentry bytes likewise ext4.

Eric reported how to reproduce this issue by:

 # seq -f "edir/abcdefghijklmnopqrstuvwxyz012345%.0f" 100000 | xargs touch
 # find edir -type f | xargs stat -c %i | sort | uniq | wc -l
100000
 # sync
 # echo 3 > /proc/sys/vm/drop_caches
 # keyctl new_session
 # find edir -type f | xargs stat -c %i | sort | uniq | wc -l
99999

Cc: <stable@vger.kernel.org>
Reported-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
(fixed f2fs_dentry_hash() to work even when the hash is 0)
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/f2fs/dir.c    | 37 +++++++++++++++++++++----------------
 fs/f2fs/f2fs.h   |  3 ++-
 fs/f2fs/hash.c   |  7 ++++++-
 fs/f2fs/inline.c |  4 ++--
 4 files changed, 31 insertions(+), 20 deletions(-)

Comments

Jaegeuk Kim April 25, 2017, 12:10 a.m. UTC | #1
Hi Eric,

This looks good to me.
I'll drop it from my tree, so please move forward through fscrypt.

Thanks,

On 04/24, Eric Biggers wrote:
> From: Jaegeuk Kim <jaegeuk@kernel.org>
> 
> If user has no key under an encrypted dir, fscrypt gives digested dentries.
> Previously, when looking up a dentry, f2fs only checks its hash value with
> first 4 bytes of the digested dentry, which didn't handle hash collisions fully.
> This patch enhances to check entire dentry bytes likewise ext4.
> 
> Eric reported how to reproduce this issue by:
> 
>  # seq -f "edir/abcdefghijklmnopqrstuvwxyz012345%.0f" 100000 | xargs touch
>  # find edir -type f | xargs stat -c %i | sort | uniq | wc -l
> 100000
>  # sync
>  # echo 3 > /proc/sys/vm/drop_caches
>  # keyctl new_session
>  # find edir -type f | xargs stat -c %i | sort | uniq | wc -l
> 99999
> 
> Cc: <stable@vger.kernel.org>
> Reported-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> (fixed f2fs_dentry_hash() to work even when the hash is 0)
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  fs/f2fs/dir.c    | 37 +++++++++++++++++++++----------------
>  fs/f2fs/f2fs.h   |  3 ++-
>  fs/f2fs/hash.c   |  7 ++++++-
>  fs/f2fs/inline.c |  4 ++--
>  4 files changed, 31 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index c143dffcae6e..374e4b8f9b70 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -130,19 +130,29 @@ struct f2fs_dir_entry *find_target_dentry(struct fscrypt_name *fname,
>  			continue;
>  		}
>  
> -		/* encrypted case */
> +		if (de->hash_code != namehash)
> +			goto not_match;
> +
>  		de_name.name = d->filename[bit_pos];
>  		de_name.len = le16_to_cpu(de->name_len);
>  
> -		/* show encrypted name */
> -		if (fname->hash) {
> -			if (de->hash_code == cpu_to_le32(fname->hash))
> -				goto found;
> -		} else if (de_name.len == name->len &&
> -			de->hash_code == namehash &&
> -			!memcmp(de_name.name, name->name, name->len))
> +#ifdef CONFIG_F2FS_FS_ENCRYPTION
> +		if (unlikely(!name->name)) {
> +			if (fname->usr_fname->name[0] == '_') {
> +				if (de_name.len >= 16 &&
> +					!memcmp(de_name.name + de_name.len - 16,
> +						fname->crypto_buf.name + 8, 16))
> +					goto found;
> +				goto not_match;
> +			}
> +			name->name = fname->crypto_buf.name;
> +			name->len = fname->crypto_buf.len;
> +		}
> +#endif
> +		if (de_name.len == name->len &&
> +				!memcmp(de_name.name, name->name, name->len))
>  			goto found;
> -
> +not_match:
>  		if (max_slots && max_len > *max_slots)
>  			*max_slots = max_len;
>  		max_len = 0;
> @@ -170,12 +180,7 @@ static struct f2fs_dir_entry *find_in_level(struct inode *dir,
>  	struct f2fs_dir_entry *de = NULL;
>  	bool room = false;
>  	int max_slots;
> -	f2fs_hash_t namehash;
> -
> -	if(fname->hash)
> -		namehash = cpu_to_le32(fname->hash);
> -	else
> -		namehash = f2fs_dentry_hash(&name);
> +	f2fs_hash_t namehash = f2fs_dentry_hash(&name, fname);
>  
>  	nbucket = dir_buckets(level, F2FS_I(dir)->i_dir_level);
>  	nblock = bucket_blocks(level);
> @@ -527,7 +532,7 @@ int f2fs_add_regular_entry(struct inode *dir, const struct qstr *new_name,
>  
>  	level = 0;
>  	slots = GET_DENTRY_SLOTS(new_name->len);
> -	dentry_hash = f2fs_dentry_hash(new_name);
> +	dentry_hash = f2fs_dentry_hash(new_name, NULL);
>  
>  	current_depth = F2FS_I(dir)->i_current_depth;
>  	if (F2FS_I(dir)->chash == dentry_hash) {
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 562db8989a4e..5bc232e21a6e 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2145,7 +2145,8 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi);
>  /*
>   * hash.c
>   */
> -f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info);
> +f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info,
> +				struct fscrypt_name *fname);
>  
>  /*
>   * node.c
> diff --git a/fs/f2fs/hash.c b/fs/f2fs/hash.c
> index 71b7206c431e..eb2e031ea887 100644
> --- a/fs/f2fs/hash.c
> +++ b/fs/f2fs/hash.c
> @@ -70,7 +70,8 @@ static void str2hashbuf(const unsigned char *msg, size_t len,
>  		*buf++ = pad;
>  }
>  
> -f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info)
> +f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info,
> +				struct fscrypt_name *fname)
>  {
>  	__u32 hash;
>  	f2fs_hash_t f2fs_hash;
> @@ -79,6 +80,10 @@ f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info)
>  	const unsigned char *name = name_info->name;
>  	size_t len = name_info->len;
>  
> +	/* encrypted bigname case */
> +	if (fname && !fname->disk_name.name)
> +		return cpu_to_le32(fname->hash);
> +
>  	if (is_dot_dotdot(name_info))
>  		return 0;
>  
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index 0ccdefe9fdba..e4c527c4e7d0 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -298,7 +298,7 @@ struct f2fs_dir_entry *find_in_inline_dir(struct inode *dir,
>  		return NULL;
>  	}
>  
> -	namehash = f2fs_dentry_hash(&name);
> +	namehash = f2fs_dentry_hash(&name, fname);
>  
>  	inline_dentry = inline_data_addr(ipage);
>  
> @@ -533,7 +533,7 @@ int f2fs_add_inline_entry(struct inode *dir, const struct qstr *new_name,
>  
>  	f2fs_wait_on_page_writeback(ipage, NODE, true);
>  
> -	name_hash = f2fs_dentry_hash(new_name);
> +	name_hash = f2fs_dentry_hash(new_name, NULL);
>  	make_dentry_ptr_inline(NULL, &d, dentry_blk);
>  	f2fs_update_dentry(ino, mode, &d, new_name, name_hash, bit_pos);
>  
> -- 
> 2.12.2.816.g2cccc81164-goog
--
To unsubscribe from this list: send the line "unsubscribe linux-fscrypt" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Y. Ts'o April 30, 2017, 6:19 a.m. UTC | #2
On Mon, Apr 24, 2017 at 10:00:08AM -0700, Eric Biggers wrote:
> From: Jaegeuk Kim <jaegeuk@kernel.org>
> 
> If user has no key under an encrypted dir, fscrypt gives digested dentries.
> Previously, when looking up a dentry, f2fs only checks its hash value with
> first 4 bytes of the digested dentry, which didn't handle hash collisions fully.
> This patch enhances to check entire dentry bytes likewise ext4.
> 
> Eric reported how to reproduce this issue by:
> 
>  # seq -f "edir/abcdefghijklmnopqrstuvwxyz012345%.0f" 100000 | xargs touch
>  # find edir -type f | xargs stat -c %i | sort | uniq | wc -l
> 100000
>  # sync
>  # echo 3 > /proc/sys/vm/drop_caches
>  # keyctl new_session
>  # find edir -type f | xargs stat -c %i | sort | uniq | wc -l
> 99999
> 
> Cc: <stable@vger.kernel.org>
> Reported-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> (fixed f2fs_dentry_hash() to work even when the hash is 0)
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Thanks, applied to the fscrypt tree.

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-fscrypt" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Biggers May 3, 2017, 2:56 a.m. UTC | #3
Hi Jaegeuk,

On Mon, Apr 24, 2017 at 05:10:23PM -0700, Jaegeuk Kim wrote:
> Hi Eric,
> 
> This looks good to me.
> I'll drop it from my tree, so please move forward through fscrypt.
> 
> Thanks,

This is in fscrypt/master now (along with the other patches in the series), but
it's also in f2fs/dev.  Are you still planning to drop it from the f2fs tree?

- Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-fscrypt" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jaegeuk Kim May 3, 2017, 4:21 a.m. UTC | #4
Hi Eric,

On 05/02, Eric Biggers wrote:
> Hi Jaegeuk,
> 
> On Mon, Apr 24, 2017 at 05:10:23PM -0700, Jaegeuk Kim wrote:
> > Hi Eric,
> > 
> > This looks good to me.
> > I'll drop it from my tree, so please move forward through fscrypt.
> > 
> > Thanks,
> 
> This is in fscrypt/master now (along with the other patches in the series), but
> it's also in f2fs/dev.  Are you still planning to drop it from the f2fs tree?

Oh, yup. I dropped it.
Thank you for pointing this out.

Thanks,

> 
> - Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-fscrypt" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index c143dffcae6e..374e4b8f9b70 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -130,19 +130,29 @@  struct f2fs_dir_entry *find_target_dentry(struct fscrypt_name *fname,
 			continue;
 		}
 
-		/* encrypted case */
+		if (de->hash_code != namehash)
+			goto not_match;
+
 		de_name.name = d->filename[bit_pos];
 		de_name.len = le16_to_cpu(de->name_len);
 
-		/* show encrypted name */
-		if (fname->hash) {
-			if (de->hash_code == cpu_to_le32(fname->hash))
-				goto found;
-		} else if (de_name.len == name->len &&
-			de->hash_code == namehash &&
-			!memcmp(de_name.name, name->name, name->len))
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
+		if (unlikely(!name->name)) {
+			if (fname->usr_fname->name[0] == '_') {
+				if (de_name.len >= 16 &&
+					!memcmp(de_name.name + de_name.len - 16,
+						fname->crypto_buf.name + 8, 16))
+					goto found;
+				goto not_match;
+			}
+			name->name = fname->crypto_buf.name;
+			name->len = fname->crypto_buf.len;
+		}
+#endif
+		if (de_name.len == name->len &&
+				!memcmp(de_name.name, name->name, name->len))
 			goto found;
-
+not_match:
 		if (max_slots && max_len > *max_slots)
 			*max_slots = max_len;
 		max_len = 0;
@@ -170,12 +180,7 @@  static struct f2fs_dir_entry *find_in_level(struct inode *dir,
 	struct f2fs_dir_entry *de = NULL;
 	bool room = false;
 	int max_slots;
-	f2fs_hash_t namehash;
-
-	if(fname->hash)
-		namehash = cpu_to_le32(fname->hash);
-	else
-		namehash = f2fs_dentry_hash(&name);
+	f2fs_hash_t namehash = f2fs_dentry_hash(&name, fname);
 
 	nbucket = dir_buckets(level, F2FS_I(dir)->i_dir_level);
 	nblock = bucket_blocks(level);
@@ -527,7 +532,7 @@  int f2fs_add_regular_entry(struct inode *dir, const struct qstr *new_name,
 
 	level = 0;
 	slots = GET_DENTRY_SLOTS(new_name->len);
-	dentry_hash = f2fs_dentry_hash(new_name);
+	dentry_hash = f2fs_dentry_hash(new_name, NULL);
 
 	current_depth = F2FS_I(dir)->i_current_depth;
 	if (F2FS_I(dir)->chash == dentry_hash) {
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 562db8989a4e..5bc232e21a6e 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2145,7 +2145,8 @@  int sanity_check_ckpt(struct f2fs_sb_info *sbi);
 /*
  * hash.c
  */
-f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info);
+f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info,
+				struct fscrypt_name *fname);
 
 /*
  * node.c
diff --git a/fs/f2fs/hash.c b/fs/f2fs/hash.c
index 71b7206c431e..eb2e031ea887 100644
--- a/fs/f2fs/hash.c
+++ b/fs/f2fs/hash.c
@@ -70,7 +70,8 @@  static void str2hashbuf(const unsigned char *msg, size_t len,
 		*buf++ = pad;
 }
 
-f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info)
+f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info,
+				struct fscrypt_name *fname)
 {
 	__u32 hash;
 	f2fs_hash_t f2fs_hash;
@@ -79,6 +80,10 @@  f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info)
 	const unsigned char *name = name_info->name;
 	size_t len = name_info->len;
 
+	/* encrypted bigname case */
+	if (fname && !fname->disk_name.name)
+		return cpu_to_le32(fname->hash);
+
 	if (is_dot_dotdot(name_info))
 		return 0;
 
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 0ccdefe9fdba..e4c527c4e7d0 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -298,7 +298,7 @@  struct f2fs_dir_entry *find_in_inline_dir(struct inode *dir,
 		return NULL;
 	}
 
-	namehash = f2fs_dentry_hash(&name);
+	namehash = f2fs_dentry_hash(&name, fname);
 
 	inline_dentry = inline_data_addr(ipage);
 
@@ -533,7 +533,7 @@  int f2fs_add_inline_entry(struct inode *dir, const struct qstr *new_name,
 
 	f2fs_wait_on_page_writeback(ipage, NODE, true);
 
-	name_hash = f2fs_dentry_hash(new_name);
+	name_hash = f2fs_dentry_hash(new_name, NULL);
 	make_dentry_ptr_inline(NULL, &d, dentry_blk);
 	f2fs_update_dentry(ino, mode, &d, new_name, name_hash, bit_pos);