Re: [PATCH v4 3/3] f2fs: Support case-insensitive file name lookups

From: Jaegeuk Kim
Date: Thu Aug 01 2019 - 00:00:28 EST


On 08/01, Chao Yu wrote:
> Hi Nathan,
>
> Thanks for the report! :)
>
> On 2019/8/1 1:57, Nathan Chancellor wrote:
> > Hi all,
> >
> > <snip>
> >
> >> diff --git a/fs/f2fs/hash.c b/fs/f2fs/hash.c
> >> index cc82f142f811f..99e79934f5088 100644
> >> --- a/fs/f2fs/hash.c
> >> +++ b/fs/f2fs/hash.c
> >> @@ -14,6 +14,7 @@
> >> #include <linux/f2fs_fs.h>
> >> #include <linux/cryptohash.h>
> >> #include <linux/pagemap.h>
> >> +#include <linux/unicode.h>
> >>
> >> #include "f2fs.h"
> >>
> >> @@ -67,7 +68,7 @@ static void str2hashbuf(const unsigned char *msg, size_t len,
> >> *buf++ = pad;
> >> }
> >>
> >> -f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info,
> >> +static f2fs_hash_t __f2fs_dentry_hash(const struct qstr *name_info,
> >> struct fscrypt_name *fname)
> >> {
> >> __u32 hash;
> >> @@ -103,3 +104,35 @@ f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info,
> >> f2fs_hash = cpu_to_le32(hash & ~F2FS_HASH_COL_BIT);
> >> return f2fs_hash;
> >> }
> >> +
> >> +f2fs_hash_t f2fs_dentry_hash(const struct inode *dir,
> >> + const struct qstr *name_info, struct fscrypt_name *fname)
> >> +{
> >> +#ifdef CONFIG_UNICODE
> >> + struct f2fs_sb_info *sbi = F2FS_SB(dir->i_sb);
> >> + const struct unicode_map *um = sbi->s_encoding;
> >> + int r, dlen;
> >> + unsigned char *buff;
> >> + struct qstr *folded;
> >> +
> >> + if (name_info->len && IS_CASEFOLDED(dir)) {
> >> + buff = f2fs_kzalloc(sbi, sizeof(char) * PATH_MAX, GFP_KERNEL);
> >> + if (!buff)
> >> + return -ENOMEM;
> >> +
> >> + dlen = utf8_casefold(um, name_info, buff, PATH_MAX);
> >> + if (dlen < 0) {
> >> + kvfree(buff);
> >> + goto opaque_seq;
> >> + }
> >> + folded->name = buff;
> >> + folded->len = dlen;
> >> + r = __f2fs_dentry_hash(folded, fname);
> >> +
> >> + kvfree(buff);
> >> + return r;
> >> + }
> >> +opaque_seq:
> >> +#endif
> >> + return __f2fs_dentry_hash(name_info, fname);
> >> +}
> >
> > Clang now warns:
> >
> > fs/f2fs/hash.c:128:3: warning: variable 'folded' is uninitialized when used here [-Wuninitialized]
> > folded->name = buff;
> > ^~~~~~
> > fs/f2fs/hash.c:116:21: note: initialize the variable 'folded' to silence this warning
> > struct qstr *folded;
> > ^
> > = NULL
> > 1 warning generated.
> >
> > I assume that it wants to be initialized with f2fs_kzalloc as well but
> > I am not familiar with this code and what it expects to do.
> >
> > Please look into this when you get a chance!
>
> That should be a bug, it needs to define a struct qstr type variable rather than
> a pointer there.
>
> Jaegeuk, could you fix this in you branch?

Yeah, let me apply this.

--- a/fs/f2fs/hash.c
+++ b/fs/f2fs/hash.c
@@ -113,25 +113,27 @@ f2fs_hash_t f2fs_dentry_hash(const struct inode *dir,
const struct unicode_map *um = sbi->s_encoding;
int r, dlen;
unsigned char *buff;
- struct qstr *folded;
+ struct qstr folded;

- if (name_info->len && IS_CASEFOLDED(dir)) {
- buff = f2fs_kzalloc(sbi, sizeof(char) * PATH_MAX, GFP_KERNEL);
- if (!buff)
- return -ENOMEM;
+ if (!name_info->len || !IS_CASEFOLDED(dir))
+ goto opaque_seq;

- dlen = utf8_casefold(um, name_info, buff, PATH_MAX);
- if (dlen < 0) {
- kvfree(buff);
- goto opaque_seq;
- }
- folded->name = buff;
- folded->len = dlen;
- r = __f2fs_dentry_hash(folded, fname);
+ buff = f2fs_kzalloc(sbi, sizeof(char) * PATH_MAX, GFP_KERNEL);
+ if (!buff)
+ return -ENOMEM;

+ dlen = utf8_casefold(um, name_info, buff, PATH_MAX);
+ if (dlen < 0) {
kvfree(buff);
- return r;
+ goto opaque_seq;
}
+ folded.name = buff;
+ folded.len = dlen;
+ r = __f2fs_dentry_hash(&folded, fname);
+
+ kvfree(buff);
+ return r;
+
opaque_seq:
#endif
return __f2fs_dentry_hash(name_info, fname);