linux-kernel - RE: [PATCH 2/2] exfat: unify name extraction

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 9 Aug 2020 02:19:10 +0900
From:   "Sungjong Seo" <sj1557.seo@...sung.com>
To:     "'Tetsuhiro Kohada'" <kohada.t2@...il.com>
Cc:     <kohada.tetsuhiro@...mitsubishielectric.co.jp>,
        <mori.takahiro@...mitsubishielectric.co.jp>,
        <motai.hirotaka@...mitsubishielectric.co.jp>,
        "'Namjae Jeon'" <namjae.jeon@...sung.com>,
        <linux-fsdevel@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 2/2] exfat: unify name extraction

> Name extraction in exfat_find_dir_entry() also doesn't care NameLength, so
> the name may be incorrect.
> Replace the name extraction in exfat_find_dir_entry() with using
> exfat_entry_set_cache and exfat_get_uniname_from_name_entries(),
> like exfat_readdir().
> Replace the name extraction with using exfat_entry_set_cache and
> exfat_get_uniname_from_name_entries(), like exfat_readdir().
> And, remove unused functions/parameters.
> 
> ** This patch depends on:
>   '[PATCH v3] exfat: integrates dir-entry getting and validation'.
> 
> Signed-off-by: Tetsuhiro Kohada <kohada.t2@...il.com>
> ---
>  fs/exfat/dir.c      | 161 ++++++++++----------------------------------
>  fs/exfat/exfat_fs.h |   2 +-
>  fs/exfat/namei.c    |   4 +-
>  3 files changed, 38 insertions(+), 129 deletions(-)
> 
> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index
> 545bb73b95e9..c9715c7a55a1 100644
> --- a/fs/exfat/dir.c
> +++ b/fs/exfat/dir.c
> @@ -10,24 +10,6 @@
>  #include "exfat_raw.h"
>  #include "exfat_fs.h"
[snip]
> @@ -963,80 +942,38 @@ int exfat_find_dir_entry(struct super_block *sb,
> struct exfat_inode_info *ei,
>  			num_empty = 0;
>  			candi_empty.eidx = EXFAT_HINT_NONE;
> 
[snip]
> 
> -			if (entry_type &
> -					(TYPE_CRITICAL_SEC |
TYPE_BENIGN_SEC)) {
> -				if (step == DIRENT_STEP_SECD) {
> -					if (++order == num_ext)
> -						goto found;
> -					continue;
> -				}
> +			exfat_get_uniname_from_name_entries(es, &uni_name);

It is needed to check a return value.

> +			exfat_free_dentry_set(es, false);
> +
> +			if (!exfat_uniname_ncmp(sb,
> +						p_uniname->name,
> +						uni_name.name,
> +						name_len)) {
> +				/* set the last used position as hint */
> +				hint_stat->clu = clu.dir;
> +				hint_stat->eidx = dentry;

eidx and clu of hint_stat should have one for the next entry we'll start
looking for.
Did you intentionally change the concept?

> +				return dentry;
>  			}
> -			step = DIRENT_STEP_FILE;
>  		}
> 
>  		if (clu.flags == ALLOC_NO_FAT_CHAIN) { @@ -1069,32 +1006,6
> @@ int exfat_find_dir_entry(struct super_block *sb, struct
> exfat_inode_info *ei,
>  	hint_stat->clu = p_dir->dir;
>  	hint_stat->eidx = 0;
>  	return -ENOENT;
> -
> -found:
> -	/* next dentry we'll find is out of this cluster */
> -	if (!((dentry + 1) & (dentries_per_clu - 1))) {
> -		int ret = 0;
> -
> -		if (clu.flags == ALLOC_NO_FAT_CHAIN) {
> -			if (--clu.size > 0)
> -				clu.dir++;
> -			else
> -				clu.dir = EXFAT_EOF_CLUSTER;
> -		} else {
> -			ret = exfat_get_next_cluster(sb, &clu.dir);
> -		}
> -
> -		if (ret || clu.dir == EXFAT_EOF_CLUSTER) {
> -			/* just initialized hint_stat */
> -			hint_stat->clu = p_dir->dir;
> -			hint_stat->eidx = 0;
> -			return (dentry - num_ext);
> -		}
> -	}
> -
> -	hint_stat->clu = clu.dir;
> -	hint_stat->eidx = dentry + 1;
> -	return dentry - num_ext;
>  }
> 
>  int exfat_count_ext_entries(struct super_block *sb, struct exfat_chain
> *p_dir, diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h index
> b88b7abc25bd..62a4768a4f6e 100644
> --- a/fs/exfat/exfat_fs.h
> +++ b/fs/exfat/exfat_fs.h
> @@ -456,7 +456,7 @@ void exfat_update_dir_chksum_with_entry_set(struct
> exfat_entry_set_cache *es);  int exfat_calc_num_entries(struct
> exfat_uni_name *p_uniname);  int exfat_find_dir_entry(struct super_block
> *sb, struct exfat_inode_info *ei,
>  		struct exfat_chain *p_dir, struct exfat_uni_name *p_uniname,
> -		int num_entries, unsigned int type);
> +		int num_entries);
>  int exfat_alloc_new_dir(struct inode *inode, struct exfat_chain *clu);
> int exfat_find_location(struct super_block *sb, struct exfat_chain *p_dir,
>  		int entry, sector_t *sector, int *offset); diff --git
> a/fs/exfat/namei.c b/fs/exfat/namei.c index a65d60ef93f4..c59d523547ca
> 100644
> --- a/fs/exfat/namei.c
> +++ b/fs/exfat/namei.c
> @@ -625,9 +625,7 @@ static int exfat_find(struct inode *dir, struct qstr
> *qname,
>  	}
> 
>  	/* search the file name for directories */
> -	dentry = exfat_find_dir_entry(sb, ei, &cdir, &uni_name,
> -			num_entries, TYPE_ALL);
> -
> +	dentry = exfat_find_dir_entry(sb, ei, &cdir, &uni_name,
> num_entries);
>  	if ((dentry < 0) && (dentry != -EEXIST))
>  		return dentry; /* -error value */
> 
> --
> 2.25.1