Re: [PATCH v2 3/3] f2fs: Handle casefolding with Encryption

Re: [PATCH v2 3/3] f2fs: Handle casefolding with Encryption

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3487 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <20201117040315.28548-4-drosen@google.com>
References: <20201117040315.28548-4-drosen@google.com>
TO: Daniel Rosenberg <drosen@google.com>

Hi Daniel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on f2fs/dev-test]
[also build test WARNING on ext4/dev linus/master v5.10-rc4 next-20201117]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daniel-Rosenberg/Add-support-for-Encryption-and-Casefolding-in-F2FS/20201117-120753
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test
:::::: branch date: 9 hours ago
:::::: commit date: 9 hours ago
compiler: aarch64-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


cppcheck possible warnings: (new ones prefixed by >>, may not real problems)

>> fs/f2fs/dir.c:206:3: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing it? [uselessAssignmentPtrArg]
     dentry_page = ERR_CAST(res);
     ^
   fs/f2fs/f2fs.h:2200:15: warning: Local variable valid_node_count shadows outer function [shadowFunction]
    unsigned int valid_node_count, user_block_count;
                 ^
   fs/f2fs/f2fs.h:2296:28: note: Shadowed declaration
   static inline unsigned int valid_node_count(struct f2fs_sb_info *sbi)
                              ^
   fs/f2fs/f2fs.h:2200:15: note: Shadow variable
    unsigned int valid_node_count, user_block_count;
                 ^

vim +206 fs/f2fs/dir.c

6b4ea0160ae236a Jaegeuk Kim      2012-11-14  191  
2c2eb7a300cd7c6 Daniel Rosenberg 2019-07-23  192  static struct f2fs_dir_entry *find_in_block(struct inode *dir,
2c2eb7a300cd7c6 Daniel Rosenberg 2019-07-23  193  				struct page *dentry_page,
43c780ba26244e4 Eric Biggers     2020-05-07  194  				const struct f2fs_filename *fname,
17f930e0a649e15 Chao Yu          2020-09-26  195  				int *max_slots)
4e6ebf6d4935914 Jaegeuk Kim      2014-10-13  196  {
4e6ebf6d4935914 Jaegeuk Kim      2014-10-13  197  	struct f2fs_dentry_block *dentry_blk;
7b3cd7d6f026784 Jaegeuk Kim      2014-10-18  198  	struct f2fs_dentry_ptr d;
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  199  	struct f2fs_dir_entry *res;
4e6ebf6d4935914 Jaegeuk Kim      2014-10-13  200  
bdbc90fa55af632 Yunlong Song     2018-02-28  201  	dentry_blk = (struct f2fs_dentry_block *)page_address(dentry_page);
7b3cd7d6f026784 Jaegeuk Kim      2014-10-18  202  
2c2eb7a300cd7c6 Daniel Rosenberg 2019-07-23  203  	make_dentry_ptr_block(dir, &d, dentry_blk);
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  204  	res = f2fs_find_target_dentry(&d, fname, max_slots);
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  205  	if (IS_ERR(res)) {
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17 @206  		dentry_page = ERR_CAST(res);
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  207  		res = NULL;
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  208  	}
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  209  	return res;
4e6ebf6d4935914 Jaegeuk Kim      2014-10-13  210  }
4e6ebf6d4935914 Jaegeuk Kim      2014-10-13  211  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Re: [PATCH v2 3/3] f2fs: Handle casefolding with Encryption

[Eric Biggers]

On Tue, Nov 17, 2020 at 10:22:28PM -0800, Daniel Rosenberg wrote:
> > > @@ -273,10 +308,14 @@ struct f2fs_dir_entry *f2fs_find_target_dentry(const struct f2fs_dentry_ptr *d,
> > >                       continue;
> > >               }
> > >
> > > -             if (de->hash_code == fname->hash &&
> > > -                 f2fs_match_name(d->inode, fname, d->filename[bit_pos],
> > > -                                 le16_to_cpu(de->name_len)))
> > > -                     goto found;
> > > +             if (de->hash_code == fname->hash) {
> > > +                     res = f2fs_match_name(d->inode, fname, d->filename[bit_pos],
> > > +                                 le16_to_cpu(de->name_len));
> > > +                     if (res < 0)
> > > +                             return ERR_PTR(res);
> > > +                     else if (res)
> > > +                             goto found;
> > > +             }
> >
> > Overly long line here.  Also 'else if' is unnecessary, just use 'if'.
> >
> > - Eric
> The 0 case is important, since that reflects that the name was not found.

I meant doing the following:

	if (res < 0)
		return ERR_PTR(res);
	if (res)
		goto found;

It doesn't really matter, but usually kernel code doesn't use 'else' after an
early return.

- Eric

Re: [PATCH v2 3/3] f2fs: Handle casefolding with Encryption

[Eric Biggers]

On Tue, Nov 17, 2020 at 10:22:28PM -0800, Daniel Rosenberg wrote:
> > > @@ -273,10 +308,14 @@ struct f2fs_dir_entry *f2fs_find_target_dentry(const struct f2fs_dentry_ptr *d,
> > >                       continue;
> > >               }
> > >
> > > -             if (de->hash_code == fname->hash &&
> > > -                 f2fs_match_name(d->inode, fname, d->filename[bit_pos],
> > > -                                 le16_to_cpu(de->name_len)))
> > > -                     goto found;
> > > +             if (de->hash_code == fname->hash) {
> > > +                     res = f2fs_match_name(d->inode, fname, d->filename[bit_pos],
> > > +                                 le16_to_cpu(de->name_len));
> > > +                     if (res < 0)
> > > +                             return ERR_PTR(res);
> > > +                     else if (res)
> > > +                             goto found;
> > > +             }
> >
> > Overly long line here.  Also 'else if' is unnecessary, just use 'if'.
> >
> > - Eric
> The 0 case is important, since that reflects that the name was not found.

I meant doing the following:

	if (res < 0)
		return ERR_PTR(res);
	if (res)
		goto found;

It doesn't really matter, but usually kernel code doesn't use 'else' after an
early return.

- Eric

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 3/3] f2fs: Handle casefolding with Encryption

[Daniel Rosenberg]

On Tue, Nov 17, 2020 at 10:50 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
>
> What is the assignment to dentry_page supposed to be accomplishing?  It looks
> like it's meant to pass up errors from f2fs_find_target_dentry(), but it doesn't
> do that.

Woops. Fixed that for the next version.

>
> > @@ -222,14 +250,20 @@ static bool f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
> >                * fall back to treating them as opaque byte sequences.
> >                */
> >               if (sb_has_strict_encoding(sb) || name->len != entry.len)
> > -                     return false;
> > -             return !memcmp(name->name, entry.name, name->len);
> > +                     res = 0;
> > +             else
> > +                     res = memcmp(name->name, entry.name, name->len) == 0;
> > +     } else {
> > +             /* utf8_strncasecmp_folded returns 0 on match */
> > +             res = (res == 0);
> >       }
>
> The following might be easier to understand:
>
>         /*
>          * In strict mode, ignore invalid names.  In non-strict mode, fall back
>          * to treating them as opaque byte sequences.
>          */
>         if (res < 0 && !sb_has_strict_encoding(sb)) {
>                 res = name->len == entry.len &&
>                       memcmp(name->name, entry.name, name->len) == 0;
>         } else {
>                 /* utf8_strncasecmp_folded returns 0 on match */
>                 res = (res == 0);
>         }
>
Thanks, that is a fair bit nicer.

> > @@ -273,10 +308,14 @@ struct f2fs_dir_entry *f2fs_find_target_dentry(const struct f2fs_dentry_ptr *d,
> >                       continue;
> >               }
> >
> > -             if (de->hash_code == fname->hash &&
> > -                 f2fs_match_name(d->inode, fname, d->filename[bit_pos],
> > -                                 le16_to_cpu(de->name_len)))
> > -                     goto found;
> > +             if (de->hash_code == fname->hash) {
> > +                     res = f2fs_match_name(d->inode, fname, d->filename[bit_pos],
> > +                                 le16_to_cpu(de->name_len));
> > +                     if (res < 0)
> > +                             return ERR_PTR(res);
> > +                     else if (res)
> > +                             goto found;
> > +             }
>
> Overly long line here.  Also 'else if' is unnecessary, just use 'if'.
>
> - Eric
The 0 case is important, since that reflects that the name was not found.
-Daniel

Re: [PATCH v2 3/3] f2fs: Handle casefolding with Encryption

[Daniel Rosenberg]

On Tue, Nov 17, 2020 at 10:50 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
>
> What is the assignment to dentry_page supposed to be accomplishing?  It looks
> like it's meant to pass up errors from f2fs_find_target_dentry(), but it doesn't
> do that.

Woops. Fixed that for the next version.

>
> > @@ -222,14 +250,20 @@ static bool f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
> >                * fall back to treating them as opaque byte sequences.
> >                */
> >               if (sb_has_strict_encoding(sb) || name->len != entry.len)
> > -                     return false;
> > -             return !memcmp(name->name, entry.name, name->len);
> > +                     res = 0;
> > +             else
> > +                     res = memcmp(name->name, entry.name, name->len) == 0;
> > +     } else {
> > +             /* utf8_strncasecmp_folded returns 0 on match */
> > +             res = (res == 0);
> >       }
>
> The following might be easier to understand:
>
>         /*
>          * In strict mode, ignore invalid names.  In non-strict mode, fall back
>          * to treating them as opaque byte sequences.
>          */
>         if (res < 0 && !sb_has_strict_encoding(sb)) {
>                 res = name->len == entry.len &&
>                       memcmp(name->name, entry.name, name->len) == 0;
>         } else {
>                 /* utf8_strncasecmp_folded returns 0 on match */
>                 res = (res == 0);
>         }
>
Thanks, that is a fair bit nicer.

> > @@ -273,10 +308,14 @@ struct f2fs_dir_entry *f2fs_find_target_dentry(const struct f2fs_dentry_ptr *d,
> >                       continue;
> >               }
> >
> > -             if (de->hash_code == fname->hash &&
> > -                 f2fs_match_name(d->inode, fname, d->filename[bit_pos],
> > -                                 le16_to_cpu(de->name_len)))
> > -                     goto found;
> > +             if (de->hash_code == fname->hash) {
> > +                     res = f2fs_match_name(d->inode, fname, d->filename[bit_pos],
> > +                                 le16_to_cpu(de->name_len));
> > +                     if (res < 0)
> > +                             return ERR_PTR(res);
> > +                     else if (res)
> > +                             goto found;
> > +             }
>
> Overly long line here.  Also 'else if' is unnecessary, just use 'if'.
>
> - Eric
The 0 case is important, since that reflects that the name was not found.
-Daniel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 3/3] f2fs: Handle casefolding with Encryption

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3307 bytes --]

Hi Daniel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on f2fs/dev-test]
[also build test WARNING on ext4/dev linus/master v5.10-rc4 next-20201117]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daniel-Rosenberg/Add-support-for-Encryption-and-Casefolding-in-F2FS/20201117-120753
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test
:::::: branch date: 9 hours ago
:::::: commit date: 9 hours ago
compiler: aarch64-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <rong.a.chen@intel.com>


cppcheck possible warnings: (new ones prefixed by >>, may not real problems)

>> fs/f2fs/dir.c:206:3: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing it? [uselessAssignmentPtrArg]
     dentry_page = ERR_CAST(res);
     ^
   fs/f2fs/f2fs.h:2200:15: warning: Local variable valid_node_count shadows outer function [shadowFunction]
    unsigned int valid_node_count, user_block_count;
                 ^
   fs/f2fs/f2fs.h:2296:28: note: Shadowed declaration
   static inline unsigned int valid_node_count(struct f2fs_sb_info *sbi)
                              ^
   fs/f2fs/f2fs.h:2200:15: note: Shadow variable
    unsigned int valid_node_count, user_block_count;
                 ^

vim +206 fs/f2fs/dir.c

6b4ea0160ae236a Jaegeuk Kim      2012-11-14  191  
2c2eb7a300cd7c6 Daniel Rosenberg 2019-07-23  192  static struct f2fs_dir_entry *find_in_block(struct inode *dir,
2c2eb7a300cd7c6 Daniel Rosenberg 2019-07-23  193  				struct page *dentry_page,
43c780ba26244e4 Eric Biggers     2020-05-07  194  				const struct f2fs_filename *fname,
17f930e0a649e15 Chao Yu          2020-09-26  195  				int *max_slots)
4e6ebf6d4935914 Jaegeuk Kim      2014-10-13  196  {
4e6ebf6d4935914 Jaegeuk Kim      2014-10-13  197  	struct f2fs_dentry_block *dentry_blk;
7b3cd7d6f026784 Jaegeuk Kim      2014-10-18  198  	struct f2fs_dentry_ptr d;
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  199  	struct f2fs_dir_entry *res;
4e6ebf6d4935914 Jaegeuk Kim      2014-10-13  200  
bdbc90fa55af632 Yunlong Song     2018-02-28  201  	dentry_blk = (struct f2fs_dentry_block *)page_address(dentry_page);
7b3cd7d6f026784 Jaegeuk Kim      2014-10-18  202  
2c2eb7a300cd7c6 Daniel Rosenberg 2019-07-23  203  	make_dentry_ptr_block(dir, &d, dentry_blk);
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  204  	res = f2fs_find_target_dentry(&d, fname, max_slots);
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  205  	if (IS_ERR(res)) {
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17 @206  		dentry_page = ERR_CAST(res);
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  207  		res = NULL;
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  208  	}
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  209  	return res;
4e6ebf6d4935914 Jaegeuk Kim      2014-10-13  210  }
4e6ebf6d4935914 Jaegeuk Kim      2014-10-13  211  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Re: [PATCH v2 3/3] f2fs: Handle casefolding with Encryption

[Eric Biggers]

On Tue, Nov 17, 2020 at 04:03:15AM +0000, Daniel Rosenberg wrote:
> Expand f2fs's casefolding support to include encrypted directories.  To
> index casefolded+encrypted directories, we use the SipHash of the
> casefolded name, keyed by a key derived from the directory's fscrypt
> master key.  This ensures that the dirhash doesn't leak information
> about the plaintext filenames.
> 
> Encryption keys are unavailable during roll-forward recovery, so we
> can't compute the dirhash when recovering a new dentry in an encrypted +
> casefolded directory.  To avoid having to force a checkpoint when a new
> file is fsync'ed, store the dirhash on-disk appended to i_name.
> 
> This patch incorporates work by Eric Biggers <ebiggers@google.com>
> and Jaegeuk Kim <jaegeuk@kernel.org>.
> 
> Co-developed-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Daniel Rosenberg <drosen@google.com>
> ---
>  fs/f2fs/dir.c      | 89 +++++++++++++++++++++++++++++++++++++---------
>  fs/f2fs/f2fs.h     |  8 +++--
>  fs/f2fs/hash.c     | 11 +++++-
>  fs/f2fs/inline.c   |  4 +++
>  fs/f2fs/recovery.c | 12 ++++++-
>  fs/f2fs/super.c    |  6 ----
>  6 files changed, 103 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index 71fdf5076461..0adc6bcfb5c0 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -5,6 +5,7 @@
>   * Copyright (c) 2012 Samsung Electronics Co., Ltd.
>   *             http://www.samsung.com/
>   */
> +#include <asm/unaligned.h>
>  #include <linux/fs.h>
>  #include <linux/f2fs_fs.h>
>  #include <linux/sched/signal.h>
> @@ -195,26 +196,53 @@ static struct f2fs_dir_entry *find_in_block(struct inode *dir,
>  {
>  	struct f2fs_dentry_block *dentry_blk;
>  	struct f2fs_dentry_ptr d;
> +	struct f2fs_dir_entry *res;
>  
>  	dentry_blk = (struct f2fs_dentry_block *)page_address(dentry_page);
>  
>  	make_dentry_ptr_block(dir, &d, dentry_blk);
> -	return f2fs_find_target_dentry(&d, fname, max_slots);
> +	res = f2fs_find_target_dentry(&d, fname, max_slots);
> +	if (IS_ERR(res)) {
> +		dentry_page = ERR_CAST(res);
> +		res = NULL;
> +	}
> +	return res;
>  }

What is the assignment to dentry_page supposed to be accomplishing?  It looks
like it's meant to pass up errors from f2fs_find_target_dentry(), but it doesn't
do that.

> @@ -222,14 +250,20 @@ static bool f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
>  		 * fall back to treating them as opaque byte sequences.
>  		 */
>  		if (sb_has_strict_encoding(sb) || name->len != entry.len)
> -			return false;
> -		return !memcmp(name->name, entry.name, name->len);
> +			res = 0;
> +		else
> +			res = memcmp(name->name, entry.name, name->len) == 0;
> +	} else {
> +		/* utf8_strncasecmp_folded returns 0 on match */
> +		res = (res == 0);
>  	}

The following might be easier to understand:

	/*
	 * In strict mode, ignore invalid names.  In non-strict mode, fall back
	 * to treating them as opaque byte sequences.
	 */
	if (res < 0 && !sb_has_strict_encoding(sb)) {
		res = name->len == entry.len &&
		      memcmp(name->name, entry.name, name->len) == 0;
	} else {
		/* utf8_strncasecmp_folded returns 0 on match */
		res = (res == 0);
	}

> @@ -273,10 +308,14 @@ struct f2fs_dir_entry *f2fs_find_target_dentry(const struct f2fs_dentry_ptr *d,
>  			continue;
>  		}
>  
> -		if (de->hash_code == fname->hash &&
> -		    f2fs_match_name(d->inode, fname, d->filename[bit_pos],
> -				    le16_to_cpu(de->name_len)))
> -			goto found;
> +		if (de->hash_code == fname->hash) {
> +			res = f2fs_match_name(d->inode, fname, d->filename[bit_pos],
> +				    le16_to_cpu(de->name_len));
> +			if (res < 0)
> +				return ERR_PTR(res);
> +			else if (res)
> +				goto found;
> +		}

Overly long line here.  Also 'else if' is unnecessary, just use 'if'.

- Eric

Re: [PATCH v2 3/3] f2fs: Handle casefolding with Encryption

[Eric Biggers]

On Tue, Nov 17, 2020 at 04:03:15AM +0000, Daniel Rosenberg wrote:
> Expand f2fs's casefolding support to include encrypted directories.  To
> index casefolded+encrypted directories, we use the SipHash of the
> casefolded name, keyed by a key derived from the directory's fscrypt
> master key.  This ensures that the dirhash doesn't leak information
> about the plaintext filenames.
> 
> Encryption keys are unavailable during roll-forward recovery, so we
> can't compute the dirhash when recovering a new dentry in an encrypted +
> casefolded directory.  To avoid having to force a checkpoint when a new
> file is fsync'ed, store the dirhash on-disk appended to i_name.
> 
> This patch incorporates work by Eric Biggers <ebiggers@google.com>
> and Jaegeuk Kim <jaegeuk@kernel.org>.
> 
> Co-developed-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Daniel Rosenberg <drosen@google.com>
> ---
>  fs/f2fs/dir.c      | 89 +++++++++++++++++++++++++++++++++++++---------
>  fs/f2fs/f2fs.h     |  8 +++--
>  fs/f2fs/hash.c     | 11 +++++-
>  fs/f2fs/inline.c   |  4 +++
>  fs/f2fs/recovery.c | 12 ++++++-
>  fs/f2fs/super.c    |  6 ----
>  6 files changed, 103 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index 71fdf5076461..0adc6bcfb5c0 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -5,6 +5,7 @@
>   * Copyright (c) 2012 Samsung Electronics Co., Ltd.
>   *             http://www.samsung.com/
>   */
> +#include <asm/unaligned.h>
>  #include <linux/fs.h>
>  #include <linux/f2fs_fs.h>
>  #include <linux/sched/signal.h>
> @@ -195,26 +196,53 @@ static struct f2fs_dir_entry *find_in_block(struct inode *dir,
>  {
>  	struct f2fs_dentry_block *dentry_blk;
>  	struct f2fs_dentry_ptr d;
> +	struct f2fs_dir_entry *res;
>  
>  	dentry_blk = (struct f2fs_dentry_block *)page_address(dentry_page);
>  
>  	make_dentry_ptr_block(dir, &d, dentry_blk);
> -	return f2fs_find_target_dentry(&d, fname, max_slots);
> +	res = f2fs_find_target_dentry(&d, fname, max_slots);
> +	if (IS_ERR(res)) {
> +		dentry_page = ERR_CAST(res);
> +		res = NULL;
> +	}
> +	return res;
>  }

What is the assignment to dentry_page supposed to be accomplishing?  It looks
like it's meant to pass up errors from f2fs_find_target_dentry(), but it doesn't
do that.

> @@ -222,14 +250,20 @@ static bool f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
>  		 * fall back to treating them as opaque byte sequences.
>  		 */
>  		if (sb_has_strict_encoding(sb) || name->len != entry.len)
> -			return false;
> -		return !memcmp(name->name, entry.name, name->len);
> +			res = 0;
> +		else
> +			res = memcmp(name->name, entry.name, name->len) == 0;
> +	} else {
> +		/* utf8_strncasecmp_folded returns 0 on match */
> +		res = (res == 0);
>  	}

The following might be easier to understand:

	/*
	 * In strict mode, ignore invalid names.  In non-strict mode, fall back
	 * to treating them as opaque byte sequences.
	 */
	if (res < 0 && !sb_has_strict_encoding(sb)) {
		res = name->len == entry.len &&
		      memcmp(name->name, entry.name, name->len) == 0;
	} else {
		/* utf8_strncasecmp_folded returns 0 on match */
		res = (res == 0);
	}

> @@ -273,10 +308,14 @@ struct f2fs_dir_entry *f2fs_find_target_dentry(const struct f2fs_dentry_ptr *d,
>  			continue;
>  		}
>  
> -		if (de->hash_code == fname->hash &&
> -		    f2fs_match_name(d->inode, fname, d->filename[bit_pos],
> -				    le16_to_cpu(de->name_len)))
> -			goto found;
> +		if (de->hash_code == fname->hash) {
> +			res = f2fs_match_name(d->inode, fname, d->filename[bit_pos],
> +				    le16_to_cpu(de->name_len));
> +			if (res < 0)
> +				return ERR_PTR(res);
> +			else if (res)
> +				goto found;
> +		}

Overly long line here.  Also 'else if' is unnecessary, just use 'if'.

- Eric

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 3/3] f2fs: Handle casefolding with Encryption

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 3013 bytes --]

Hi Daniel,

url:    https://github.com/0day-ci/linux/commits/Daniel-Rosenberg/Add-support-for-Encryption-and-Casefolding-in-F2FS/20201117-120753 
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git  dev-test
compiler: aarch64-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

cppcheck possible warnings: (new ones prefixed by >>, may not real problems)

>> fs/f2fs/dir.c:206:3: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing it? [uselessAssignmentPtrArg]
     dentry_page = ERR_CAST(res);
     ^
   fs/f2fs/f2fs.h:2200:15: warning: Local variable valid_node_count shadows outer function [shadowFunction]
    unsigned int valid_node_count, user_block_count;
                 ^
   fs/f2fs/f2fs.h:2296:28: note: Shadowed declaration
   static inline unsigned int valid_node_count(struct f2fs_sb_info *sbi)
                              ^
   fs/f2fs/f2fs.h:2200:15: note: Shadow variable
    unsigned int valid_node_count, user_block_count;
                 ^

vim +206 fs/f2fs/dir.c

2c2eb7a300cd7c6 Daniel Rosenberg 2019-07-23  192  static struct f2fs_dir_entry *find_in_block(struct inode *dir,
2c2eb7a300cd7c6 Daniel Rosenberg 2019-07-23  193  				struct page *dentry_page,
43c780ba26244e4 Eric Biggers     2020-05-07  194  				const struct f2fs_filename *fname,
17f930e0a649e15 Chao Yu          2020-09-26  195  				int *max_slots)
4e6ebf6d4935914 Jaegeuk Kim      2014-10-13  196  {
4e6ebf6d4935914 Jaegeuk Kim      2014-10-13  197  	struct f2fs_dentry_block *dentry_blk;
7b3cd7d6f026784 Jaegeuk Kim      2014-10-18  198  	struct f2fs_dentry_ptr d;
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  199  	struct f2fs_dir_entry *res;
4e6ebf6d4935914 Jaegeuk Kim      2014-10-13  200  
bdbc90fa55af632 Yunlong Song     2018-02-28  201  	dentry_blk = (struct f2fs_dentry_block *)page_address(dentry_page);
7b3cd7d6f026784 Jaegeuk Kim      2014-10-18  202  
2c2eb7a300cd7c6 Daniel Rosenberg 2019-07-23  203  	make_dentry_ptr_block(dir, &d, dentry_blk);
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  204  	res = f2fs_find_target_dentry(&d, fname, max_slots);
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  205  	if (IS_ERR(res)) {
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17 @206  		dentry_page = ERR_CAST(res);
                                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  207  		res = NULL;
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  208  	}
6f0a1d1d8d24e7e Daniel Rosenberg 2020-11-17  209  	return res;
4e6ebf6d4935914 Jaegeuk Kim      2014-10-13  210  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org 
_______________________________________________
kbuild mailing list -- kbuild(a)lists.01.org
To unsubscribe send an email to kbuild-leave(a)lists.01.org

[PATCH v2 3/3] f2fs: Handle casefolding with Encryption

[Daniel Rosenberg]

Expand f2fs's casefolding support to include encrypted directories.  To
index casefolded+encrypted directories, we use the SipHash of the
casefolded name, keyed by a key derived from the directory's fscrypt
master key.  This ensures that the dirhash doesn't leak information
about the plaintext filenames.

Encryption keys are unavailable during roll-forward recovery, so we
can't compute the dirhash when recovering a new dentry in an encrypted +
casefolded directory.  To avoid having to force a checkpoint when a new
file is fsync'ed, store the dirhash on-disk appended to i_name.

This patch incorporates work by Eric Biggers <ebiggers@google.com>
and Jaegeuk Kim <jaegeuk@kernel.org>.

Co-developed-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Daniel Rosenberg <drosen@google.com>
---
 fs/f2fs/dir.c      | 89 +++++++++++++++++++++++++++++++++++++---------
 fs/f2fs/f2fs.h     |  8 +++--
 fs/f2fs/hash.c     | 11 +++++-
 fs/f2fs/inline.c   |  4 +++
 fs/f2fs/recovery.c | 12 ++++++-
 fs/f2fs/super.c    |  6 ----
 6 files changed, 103 insertions(+), 27 deletions(-)

diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 71fdf5076461..0adc6bcfb5c0 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2012 Samsung Electronics Co., Ltd.
  *             http://www.samsung.com/
  */
+#include <asm/unaligned.h>
 #include <linux/fs.h>
 #include <linux/f2fs_fs.h>
 #include <linux/sched/signal.h>
@@ -195,26 +196,53 @@ static struct f2fs_dir_entry *find_in_block(struct inode *dir,
 {
 	struct f2fs_dentry_block *dentry_blk;
 	struct f2fs_dentry_ptr d;
+	struct f2fs_dir_entry *res;
 
 	dentry_blk = (struct f2fs_dentry_block *)page_address(dentry_page);
 
 	make_dentry_ptr_block(dir, &d, dentry_blk);
-	return f2fs_find_target_dentry(&d, fname, max_slots);
+	res = f2fs_find_target_dentry(&d, fname, max_slots);
+	if (IS_ERR(res)) {
+		dentry_page = ERR_CAST(res);
+		res = NULL;
+	}
+	return res;
 }
 
 #ifdef CONFIG_UNICODE
 /*
  * Test whether a case-insensitive directory entry matches the filename
  * being searched for.
+ *
+ * Returns 1 for a match, 0 for no match, and -errno on an error.
  */
-static bool f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
+static int f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
 			       const u8 *de_name, u32 de_name_len)
 {
 	const struct super_block *sb = dir->i_sb;
 	const struct unicode_map *um = sb->s_encoding;
+	struct fscrypt_str decrypted_name = FSTR_INIT(NULL, de_name_len);
 	struct qstr entry = QSTR_INIT(de_name, de_name_len);
 	int res;
 
+	if (IS_ENCRYPTED(dir)) {
+		const struct fscrypt_str encrypted_name =
+			FSTR_INIT((u8 *)de_name, de_name_len);
+
+		if (WARN_ON_ONCE(!fscrypt_has_encryption_key(dir)))
+			return -EINVAL;
+
+		decrypted_name.name = kmalloc(de_name_len, GFP_KERNEL);
+		if (!decrypted_name.name)
+			return -ENOMEM;
+		res = fscrypt_fname_disk_to_usr(dir, 0, 0, &encrypted_name,
+						&decrypted_name);
+		if (res < 0)
+			goto out;
+		entry.name = decrypted_name.name;
+		entry.len = decrypted_name.len;
+	}
+
 	res = utf8_strncasecmp_folded(um, name, &entry);
 	if (res < 0) {
 		/*
@@ -222,14 +250,20 @@ static bool f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
 		 * fall back to treating them as opaque byte sequences.
 		 */
 		if (sb_has_strict_encoding(sb) || name->len != entry.len)
-			return false;
-		return !memcmp(name->name, entry.name, name->len);
+			res = 0;
+		else
+			res = memcmp(name->name, entry.name, name->len) == 0;
+	} else {
+		/* utf8_strncasecmp_folded returns 0 on match */
+		res = (res == 0);
 	}
-	return res == 0;
+out:
+	kfree(decrypted_name.name);
+	return res;
 }
 #endif /* CONFIG_UNICODE */
 
-static inline bool f2fs_match_name(const struct inode *dir,
+static inline int f2fs_match_name(const struct inode *dir,
 				   const struct f2fs_filename *fname,
 				   const u8 *de_name, u32 de_name_len)
 {
@@ -256,6 +290,7 @@ struct f2fs_dir_entry *f2fs_find_target_dentry(const struct f2fs_dentry_ptr *d,
 	struct f2fs_dir_entry *de;
 	unsigned long bit_pos = 0;
 	int max_len = 0;
+	int res = 0;
 
 	if (max_slots)
 		*max_slots = 0;
@@ -273,10 +308,14 @@ struct f2fs_dir_entry *f2fs_find_target_dentry(const struct f2fs_dentry_ptr *d,
 			continue;
 		}
 
-		if (de->hash_code == fname->hash &&
-		    f2fs_match_name(d->inode, fname, d->filename[bit_pos],
-				    le16_to_cpu(de->name_len)))
-			goto found;
+		if (de->hash_code == fname->hash) {
+			res = f2fs_match_name(d->inode, fname, d->filename[bit_pos],
+				    le16_to_cpu(de->name_len));
+			if (res < 0)
+				return ERR_PTR(res);
+			else if (res)
+				goto found;
+		}
 
 		if (max_slots && max_len > *max_slots)
 			*max_slots = max_len;
@@ -448,17 +487,39 @@ void f2fs_set_link(struct inode *dir, struct f2fs_dir_entry *de,
 	f2fs_put_page(page, 1);
 }
 
-static void init_dent_inode(const struct f2fs_filename *fname,
+static void init_dent_inode(struct inode *dir, struct inode *inode,
+			    const struct f2fs_filename *fname,
 			    struct page *ipage)
 {
 	struct f2fs_inode *ri;
 
+	if (!fname) /* tmpfile case? */
+		return;
+
 	f2fs_wait_on_page_writeback(ipage, NODE, true, true);
 
 	/* copy name info. to this inode page */
 	ri = F2FS_INODE(ipage);
 	ri->i_namelen = cpu_to_le32(fname->disk_name.len);
 	memcpy(ri->i_name, fname->disk_name.name, fname->disk_name.len);
+	if (IS_ENCRYPTED(dir)) {
+		file_set_enc_name(inode);
+		/*
+		 * Roll-forward recovery doesn't have encryption keys available,
+		 * so it can't compute the dirhash for encrypted+casefolded
+		 * filenames.  Append it to i_name if possible.  Else, disable
+		 * roll-forward recovery of the dentry (i.e., make fsync'ing the
+		 * file force a checkpoint) by setting LOST_PINO.
+		 */
+		if (IS_CASEFOLDED(dir)) {
+			if (fname->disk_name.len + sizeof(f2fs_hash_t) <=
+			    F2FS_NAME_LEN)
+				put_unaligned(fname->hash, (f2fs_hash_t *)
+					&ri->i_name[fname->disk_name.len]);
+			else
+				file_lost_pino(inode);
+		}
+	}
 	set_page_dirty(ipage);
 }
 
@@ -541,11 +602,7 @@ struct page *f2fs_init_inode_metadata(struct inode *inode, struct inode *dir,
 			return page;
 	}
 
-	if (fname) {
-		init_dent_inode(fname, page);
-		if (IS_ENCRYPTED(dir))
-			file_set_enc_name(inode);
-	}
+	init_dent_inode(dir, inode, fname, page);
 
 	/*
 	 * This file should be checkpointed during fsync.
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 62b4f31d30e2..878308736761 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -533,9 +533,11 @@ struct f2fs_filename {
 #ifdef CONFIG_UNICODE
 	/*
 	 * For casefolded directories: the casefolded name, but it's left NULL
-	 * if the original name is not valid Unicode or if the filesystem is
-	 * doing an internal operation where usr_fname is also NULL.  In these
-	 * cases we fall back to treating the name as an opaque byte sequence.
+	 * if the original name is not valid Unicode, if the directory is both
+	 * casefolded and encrypted and its encryption key is unavailable, or if
+	 * the filesystem is doing an internal operation where usr_fname is also
+	 * NULL.  In all these cases we fall back to treating the name as an
+	 * opaque byte sequence.
 	 */
 	struct fscrypt_str cf_name;
 #endif
diff --git a/fs/f2fs/hash.c b/fs/f2fs/hash.c
index de841aaf3c43..e3beac546c63 100644
--- a/fs/f2fs/hash.c
+++ b/fs/f2fs/hash.c
@@ -111,7 +111,9 @@ void f2fs_hash_filename(const struct inode *dir, struct f2fs_filename *fname)
 		 * If the casefolded name is provided, hash it instead of the
 		 * on-disk name.  If the casefolded name is *not* provided, that
 		 * should only be because the name wasn't valid Unicode, so fall
-		 * back to treating the name as an opaque byte sequence.
+		 * back to treating the name as an opaque byte sequence.  Note
+		 * that to handle encrypted directories, the fallback must use
+		 * usr_fname (plaintext) rather than disk_name (ciphertext).
 		 */
 		WARN_ON_ONCE(!fname->usr_fname->name);
 		if (fname->cf_name.name) {
@@ -121,6 +123,13 @@ void f2fs_hash_filename(const struct inode *dir, struct f2fs_filename *fname)
 			name = fname->usr_fname->name;
 			len = fname->usr_fname->len;
 		}
+		if (IS_ENCRYPTED(dir)) {
+			struct qstr tmp = QSTR_INIT(name, len);
+
+			fname->hash =
+				cpu_to_le32(fscrypt_fname_siphash(dir, &tmp));
+			return;
+		}
 	}
 #endif
 	fname->hash = cpu_to_le32(TEA_hash_name(name, len));
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 70384e31788d..92e9852d316a 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -332,6 +332,10 @@ struct f2fs_dir_entry *f2fs_find_in_inline_dir(struct inode *dir,
 	make_dentry_ptr_inline(dir, &d, inline_dentry);
 	de = f2fs_find_target_dentry(&d, fname, NULL);
 	unlock_page(ipage);
+	if (IS_ERR(de)) {
+		*res_page = ERR_CAST(de);
+		de = NULL;
+	}
 	if (de)
 		*res_page = ipage;
 	else
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 4f12ade6410a..0947d36af1a8 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2012 Samsung Electronics Co., Ltd.
  *             http://www.samsung.com/
  */
+#include <asm/unaligned.h>
 #include <linux/fs.h>
 #include <linux/f2fs_fs.h>
 #include "f2fs.h"
@@ -128,7 +129,16 @@ static int init_recovered_filename(const struct inode *dir,
 	}
 
 	/* Compute the hash of the filename */
-	if (IS_CASEFOLDED(dir)) {
+	if (IS_ENCRYPTED(dir) && IS_CASEFOLDED(dir)) {
+		/*
+		 * In this case the hash isn't computable without the key, so it
+		 * was saved on-disk.
+		 */
+		if (fname->disk_name.len + sizeof(f2fs_hash_t) > F2FS_NAME_LEN)
+			return -EINVAL;
+		fname->hash = get_unaligned((f2fs_hash_t *)
+				&raw_inode->i_name[fname->disk_name.len]);
+	} else if (IS_CASEFOLDED(dir)) {
 		err = f2fs_init_casefolded_name(dir, fname);
 		if (err)
 			return err;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index f51d52591c99..42293b7ceaf2 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3399,12 +3399,6 @@ static int f2fs_setup_casefold(struct f2fs_sb_info *sbi)
 		struct unicode_map *encoding;
 		__u16 encoding_flags;
 
-		if (f2fs_sb_has_encrypt(sbi)) {
-			f2fs_err(sbi,
-				"Can't mount with encoding and encryption");
-			return -EINVAL;
-		}
-
 		if (f2fs_sb_read_encoding(sbi->raw_super, &encoding_info,
 					  &encoding_flags)) {
 			f2fs_err(sbi,
-- 
2.29.2.299.gdc1121823c-goog

[PATCH v2 3/3] f2fs: Handle casefolding with Encryption

[Daniel Rosenberg]

Expand f2fs's casefolding support to include encrypted directories.  To
index casefolded+encrypted directories, we use the SipHash of the
casefolded name, keyed by a key derived from the directory's fscrypt
master key.  This ensures that the dirhash doesn't leak information
about the plaintext filenames.

Encryption keys are unavailable during roll-forward recovery, so we
can't compute the dirhash when recovering a new dentry in an encrypted +
casefolded directory.  To avoid having to force a checkpoint when a new
file is fsync'ed, store the dirhash on-disk appended to i_name.

This patch incorporates work by Eric Biggers <ebiggers@google.com>
and Jaegeuk Kim <jaegeuk@kernel.org>.

Co-developed-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Daniel Rosenberg <drosen@google.com>
---
 fs/f2fs/dir.c      | 89 +++++++++++++++++++++++++++++++++++++---------
 fs/f2fs/f2fs.h     |  8 +++--
 fs/f2fs/hash.c     | 11 +++++-
 fs/f2fs/inline.c   |  4 +++
 fs/f2fs/recovery.c | 12 ++++++-
 fs/f2fs/super.c    |  6 ----
 6 files changed, 103 insertions(+), 27 deletions(-)

diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 71fdf5076461..0adc6bcfb5c0 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2012 Samsung Electronics Co., Ltd.
  *             http://www.samsung.com/
  */
+#include <asm/unaligned.h>
 #include <linux/fs.h>
 #include <linux/f2fs_fs.h>
 #include <linux/sched/signal.h>
@@ -195,26 +196,53 @@ static struct f2fs_dir_entry *find_in_block(struct inode *dir,
 {
 	struct f2fs_dentry_block *dentry_blk;
 	struct f2fs_dentry_ptr d;
+	struct f2fs_dir_entry *res;
 
 	dentry_blk = (struct f2fs_dentry_block *)page_address(dentry_page);
 
 	make_dentry_ptr_block(dir, &d, dentry_blk);
-	return f2fs_find_target_dentry(&d, fname, max_slots);
+	res = f2fs_find_target_dentry(&d, fname, max_slots);
+	if (IS_ERR(res)) {
+		dentry_page = ERR_CAST(res);
+		res = NULL;
+	}
+	return res;
 }
 
 #ifdef CONFIG_UNICODE
 /*
  * Test whether a case-insensitive directory entry matches the filename
  * being searched for.
+ *
+ * Returns 1 for a match, 0 for no match, and -errno on an error.
  */
-static bool f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
+static int f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
 			       const u8 *de_name, u32 de_name_len)
 {
 	const struct super_block *sb = dir->i_sb;
 	const struct unicode_map *um = sb->s_encoding;
+	struct fscrypt_str decrypted_name = FSTR_INIT(NULL, de_name_len);
 	struct qstr entry = QSTR_INIT(de_name, de_name_len);
 	int res;
 
+	if (IS_ENCRYPTED(dir)) {
+		const struct fscrypt_str encrypted_name =
+			FSTR_INIT((u8 *)de_name, de_name_len);
+
+		if (WARN_ON_ONCE(!fscrypt_has_encryption_key(dir)))
+			return -EINVAL;
+
+		decrypted_name.name = kmalloc(de_name_len, GFP_KERNEL);
+		if (!decrypted_name.name)
+			return -ENOMEM;
+		res = fscrypt_fname_disk_to_usr(dir, 0, 0, &encrypted_name,
+						&decrypted_name);
+		if (res < 0)
+			goto out;
+		entry.name = decrypted_name.name;
+		entry.len = decrypted_name.len;
+	}
+
 	res = utf8_strncasecmp_folded(um, name, &entry);
 	if (res < 0) {
 		/*
@@ -222,14 +250,20 @@ static bool f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
 		 * fall back to treating them as opaque byte sequences.
 		 */
 		if (sb_has_strict_encoding(sb) || name->len != entry.len)
-			return false;
-		return !memcmp(name->name, entry.name, name->len);
+			res = 0;
+		else
+			res = memcmp(name->name, entry.name, name->len) == 0;
+	} else {
+		/* utf8_strncasecmp_folded returns 0 on match */
+		res = (res == 0);
 	}
-	return res == 0;
+out:
+	kfree(decrypted_name.name);
+	return res;
 }
 #endif /* CONFIG_UNICODE */
 
-static inline bool f2fs_match_name(const struct inode *dir,
+static inline int f2fs_match_name(const struct inode *dir,
 				   const struct f2fs_filename *fname,
 				   const u8 *de_name, u32 de_name_len)
 {
@@ -256,6 +290,7 @@ struct f2fs_dir_entry *f2fs_find_target_dentry(const struct f2fs_dentry_ptr *d,
 	struct f2fs_dir_entry *de;
 	unsigned long bit_pos = 0;
 	int max_len = 0;
+	int res = 0;
 
 	if (max_slots)
 		*max_slots = 0;
@@ -273,10 +308,14 @@ struct f2fs_dir_entry *f2fs_find_target_dentry(const struct f2fs_dentry_ptr *d,
 			continue;
 		}
 
-		if (de->hash_code == fname->hash &&
-		    f2fs_match_name(d->inode, fname, d->filename[bit_pos],
-				    le16_to_cpu(de->name_len)))
-			goto found;
+		if (de->hash_code == fname->hash) {
+			res = f2fs_match_name(d->inode, fname, d->filename[bit_pos],
+				    le16_to_cpu(de->name_len));
+			if (res < 0)
+				return ERR_PTR(res);
+			else if (res)
+				goto found;
+		}
 
 		if (max_slots && max_len > *max_slots)
 			*max_slots = max_len;
@@ -448,17 +487,39 @@ void f2fs_set_link(struct inode *dir, struct f2fs_dir_entry *de,
 	f2fs_put_page(page, 1);
 }
 
-static void init_dent_inode(const struct f2fs_filename *fname,
+static void init_dent_inode(struct inode *dir, struct inode *inode,
+			    const struct f2fs_filename *fname,
 			    struct page *ipage)
 {
 	struct f2fs_inode *ri;
 
+	if (!fname) /* tmpfile case? */
+		return;
+
 	f2fs_wait_on_page_writeback(ipage, NODE, true, true);
 
 	/* copy name info. to this inode page */
 	ri = F2FS_INODE(ipage);
 	ri->i_namelen = cpu_to_le32(fname->disk_name.len);
 	memcpy(ri->i_name, fname->disk_name.name, fname->disk_name.len);
+	if (IS_ENCRYPTED(dir)) {
+		file_set_enc_name(inode);
+		/*
+		 * Roll-forward recovery doesn't have encryption keys available,
+		 * so it can't compute the dirhash for encrypted+casefolded
+		 * filenames.  Append it to i_name if possible.  Else, disable
+		 * roll-forward recovery of the dentry (i.e., make fsync'ing the
+		 * file force a checkpoint) by setting LOST_PINO.
+		 */
+		if (IS_CASEFOLDED(dir)) {
+			if (fname->disk_name.len + sizeof(f2fs_hash_t) <=
+			    F2FS_NAME_LEN)
+				put_unaligned(fname->hash, (f2fs_hash_t *)
+					&ri->i_name[fname->disk_name.len]);
+			else
+				file_lost_pino(inode);
+		}
+	}
 	set_page_dirty(ipage);
 }
 
@@ -541,11 +602,7 @@ struct page *f2fs_init_inode_metadata(struct inode *inode, struct inode *dir,
 			return page;
 	}
 
-	if (fname) {
-		init_dent_inode(fname, page);
-		if (IS_ENCRYPTED(dir))
-			file_set_enc_name(inode);
-	}
+	init_dent_inode(dir, inode, fname, page);
 
 	/*
 	 * This file should be checkpointed during fsync.
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 62b4f31d30e2..878308736761 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -533,9 +533,11 @@ struct f2fs_filename {
 #ifdef CONFIG_UNICODE
 	/*
 	 * For casefolded directories: the casefolded name, but it's left NULL
-	 * if the original name is not valid Unicode or if the filesystem is
-	 * doing an internal operation where usr_fname is also NULL.  In these
-	 * cases we fall back to treating the name as an opaque byte sequence.
+	 * if the original name is not valid Unicode, if the directory is both
+	 * casefolded and encrypted and its encryption key is unavailable, or if
+	 * the filesystem is doing an internal operation where usr_fname is also
+	 * NULL.  In all these cases we fall back to treating the name as an
+	 * opaque byte sequence.
 	 */
 	struct fscrypt_str cf_name;
 #endif
diff --git a/fs/f2fs/hash.c b/fs/f2fs/hash.c
index de841aaf3c43..e3beac546c63 100644
--- a/fs/f2fs/hash.c
+++ b/fs/f2fs/hash.c
@@ -111,7 +111,9 @@ void f2fs_hash_filename(const struct inode *dir, struct f2fs_filename *fname)
 		 * If the casefolded name is provided, hash it instead of the
 		 * on-disk name.  If the casefolded name is *not* provided, that
 		 * should only be because the name wasn't valid Unicode, so fall
-		 * back to treating the name as an opaque byte sequence.
+		 * back to treating the name as an opaque byte sequence.  Note
+		 * that to handle encrypted directories, the fallback must use
+		 * usr_fname (plaintext) rather than disk_name (ciphertext).
 		 */
 		WARN_ON_ONCE(!fname->usr_fname->name);
 		if (fname->cf_name.name) {
@@ -121,6 +123,13 @@ void f2fs_hash_filename(const struct inode *dir, struct f2fs_filename *fname)
 			name = fname->usr_fname->name;
 			len = fname->usr_fname->len;
 		}
+		if (IS_ENCRYPTED(dir)) {
+			struct qstr tmp = QSTR_INIT(name, len);
+
+			fname->hash =
+				cpu_to_le32(fscrypt_fname_siphash(dir, &tmp));
+			return;
+		}
 	}
 #endif
 	fname->hash = cpu_to_le32(TEA_hash_name(name, len));
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 70384e31788d..92e9852d316a 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -332,6 +332,10 @@ struct f2fs_dir_entry *f2fs_find_in_inline_dir(struct inode *dir,
 	make_dentry_ptr_inline(dir, &d, inline_dentry);
 	de = f2fs_find_target_dentry(&d, fname, NULL);
 	unlock_page(ipage);
+	if (IS_ERR(de)) {
+		*res_page = ERR_CAST(de);
+		de = NULL;
+	}
 	if (de)
 		*res_page = ipage;
 	else
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 4f12ade6410a..0947d36af1a8 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2012 Samsung Electronics Co., Ltd.
  *             http://www.samsung.com/
  */
+#include <asm/unaligned.h>
 #include <linux/fs.h>
 #include <linux/f2fs_fs.h>
 #include "f2fs.h"
@@ -128,7 +129,16 @@ static int init_recovered_filename(const struct inode *dir,
 	}
 
 	/* Compute the hash of the filename */
-	if (IS_CASEFOLDED(dir)) {
+	if (IS_ENCRYPTED(dir) && IS_CASEFOLDED(dir)) {
+		/*
+		 * In this case the hash isn't computable without the key, so it
+		 * was saved on-disk.
+		 */
+		if (fname->disk_name.len + sizeof(f2fs_hash_t) > F2FS_NAME_LEN)
+			return -EINVAL;
+		fname->hash = get_unaligned((f2fs_hash_t *)
+				&raw_inode->i_name[fname->disk_name.len]);
+	} else if (IS_CASEFOLDED(dir)) {
 		err = f2fs_init_casefolded_name(dir, fname);
 		if (err)
 			return err;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index f51d52591c99..42293b7ceaf2 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3399,12 +3399,6 @@ static int f2fs_setup_casefold(struct f2fs_sb_info *sbi)
 		struct unicode_map *encoding;
 		__u16 encoding_flags;
 
-		if (f2fs_sb_has_encrypt(sbi)) {
-			f2fs_err(sbi,
-				"Can't mount with encoding and encryption");
-			return -EINVAL;
-		}
-
 		if (f2fs_sb_read_encoding(sbi->raw_super, &encoding_info,
 					  &encoding_flags)) {
 			f2fs_err(sbi,
-- 
2.29.2.299.gdc1121823c-goog


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/