2020-08-19 06:40:11

by Tetsuhiro Kohada

[permalink] [raw]
Subject: [PATCH v2 1/2] exfat: add NameLength check when extracting name

The current implementation doesn't care NameLength when extracting
the name from Name dir-entries, so the name may be incorrect.
(Without null-termination, Insufficient Name dir-entries, etc)
Add a NameLength check when extracting the name from Name dir-entries
to extract correct name.
And, change to get the information of file/stream-ext dir-entries
via the member variable in exfat_entry_set_cache.

** This patch depends on:
'[PATCH v3] exfat: integrates dir-entry getting and validation'.

Suggested-by: Sungjong Seo <[email protected]>
Signed-off-by: Tetsuhiro Kohada <[email protected]>
---
Changes in v2
- Add error check when extracting name
- Change error from EIO to EINVAL when the name length is invalid
- Correct the spelling in commit messages

fs/exfat/dir.c | 87 +++++++++++++++++++++++++-------------------------
1 file changed, 43 insertions(+), 44 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index 91cdbede0fd1..08ebfcdd66a0 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -28,16 +28,15 @@ static int exfat_extract_uni_name(struct exfat_dentry *ep,

}

-static void exfat_get_uniname_from_ext_entry(struct super_block *sb,
- struct exfat_chain *p_dir, int entry, unsigned short *uniname)
+static int exfat_get_uniname_from_name_entries(struct exfat_entry_set_cache *es,
+ struct exfat_uni_name *uniname)
{
- int i;
- struct exfat_entry_set_cache *es;
+ int n, l, i;
struct exfat_dentry *ep;

- es = exfat_get_dentry_set(sb, p_dir, entry, ES_ALL_ENTRIES);
- if (!es)
- return;
+ uniname->name_len = es->de_stream->name_len;
+ if (uniname->name_len == 0)
+ return -EINVAL;

/*
* First entry : file entry
@@ -45,24 +44,26 @@ static void exfat_get_uniname_from_ext_entry(struct super_block *sb,
* Third entry : first file-name entry
* So, the index of first file-name dentry should start from 2.
*/
-
- i = 2;
- while ((ep = exfat_get_validated_dentry(es, i++, TYPE_NAME))) {
- exfat_extract_uni_name(ep, uniname);
- uniname += EXFAT_FILE_NAME_LEN;
+ for (l = 0, n = 2; l < uniname->name_len; n++) {
+ ep = exfat_get_validated_dentry(es, n, TYPE_NAME);
+ if (!ep)
+ return -EIO;
+ for (i = 0; l < uniname->name_len && i < EXFAT_FILE_NAME_LEN; i++, l++)
+ uniname->name[l] = le16_to_cpu(ep->dentry.name.unicode_0_14[i]);
}
-
- exfat_free_dentry_set(es, false);
+ uniname->name[l] = 0;
+ return 0;
}

/* read a directory entry from the opened directory */
static int exfat_readdir(struct inode *inode, struct exfat_dir_entry *dir_entry)
{
- int i, dentries_per_clu, dentries_per_clu_bits = 0;
+ int i, dentries_per_clu, dentries_per_clu_bits = 0, err;
unsigned int type, clu_offset;
sector_t sector;
struct exfat_chain dir, clu;
struct exfat_uni_name uni_name;
+ struct exfat_entry_set_cache *es;
struct exfat_dentry *ep;
struct super_block *sb = inode->i_sb;
struct exfat_sb_info *sbi = EXFAT_SB(sb);
@@ -114,47 +115,45 @@ static int exfat_readdir(struct inode *inode, struct exfat_dir_entry *dir_entry)
return -EIO;

type = exfat_get_entry_type(ep);
- if (type == TYPE_UNUSED) {
- brelse(bh);
+ brelse(bh);
+
+ if (type == TYPE_UNUSED)
break;
- }

- if (type != TYPE_FILE && type != TYPE_DIR) {
- brelse(bh);
+ if (type != TYPE_FILE && type != TYPE_DIR)
continue;
- }

- dir_entry->attr = le16_to_cpu(ep->dentry.file.attr);
+ es = exfat_get_dentry_set(sb, &dir, dentry, ES_ALL_ENTRIES);
+ if (!es)
+ return -EIO;
+
+ dir_entry->attr = le16_to_cpu(es->de_file->attr);
exfat_get_entry_time(sbi, &dir_entry->crtime,
- ep->dentry.file.create_tz,
- ep->dentry.file.create_time,
- ep->dentry.file.create_date,
- ep->dentry.file.create_time_cs);
+ es->de_file->create_tz,
+ es->de_file->create_time,
+ es->de_file->create_date,
+ es->de_file->create_time_cs);
exfat_get_entry_time(sbi, &dir_entry->mtime,
- ep->dentry.file.modify_tz,
- ep->dentry.file.modify_time,
- ep->dentry.file.modify_date,
- ep->dentry.file.modify_time_cs);
+ es->de_file->modify_tz,
+ es->de_file->modify_time,
+ es->de_file->modify_date,
+ es->de_file->modify_time_cs);
exfat_get_entry_time(sbi, &dir_entry->atime,
- ep->dentry.file.access_tz,
- ep->dentry.file.access_time,
- ep->dentry.file.access_date,
+ es->de_file->access_tz,
+ es->de_file->access_time,
+ es->de_file->access_date,
0);

- *uni_name.name = 0x0;
- exfat_get_uniname_from_ext_entry(sb, &dir, dentry,
- uni_name.name);
+ dir_entry->size = le64_to_cpu(es->de_stream->valid_size);
+
+ err = exfat_get_uniname_from_name_entries(es, &uni_name);
+ exfat_free_dentry_set(es, false);
+ if (err)
+ return err;
+
exfat_utf16_to_nls(sb, &uni_name,
dir_entry->namebuf.lfn,
dir_entry->namebuf.lfnbuf_len);
- brelse(bh);
-
- ep = exfat_get_dentry(sb, &clu, i + 1, &bh, NULL);
- if (!ep)
- return -EIO;
- dir_entry->size =
- le64_to_cpu(ep->dentry.stream.valid_size);
- brelse(bh);

ei->hint_bmap.off = dentry >> dentries_per_clu_bits;
ei->hint_bmap.clu = clu.dir;
--
2.25.1


2020-08-19 06:40:22

by Tetsuhiro Kohada

[permalink] [raw]
Subject: [PATCH v2 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().
And, remove unused functions/parameters.

** This patch depends on:
'[PATCH v3] exfat: integrates dir-entry getting and validation'.

Signed-off-by: Tetsuhiro Kohada <[email protected]>
---
Changes in v2
- Add error check when extracting name
- Remove temporary exfat_get_dentry_set() with ES_2_ENTRIES
- Remove duplicate parts in commit message

fs/exfat/dir.c | 156 +++++++++-----------------------------------
fs/exfat/exfat_fs.h | 2 +-
fs/exfat/namei.c | 4 +-
3 files changed, 32 insertions(+), 130 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index 08ebfcdd66a0..0b42544e6340 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -10,24 +10,6 @@
#include "exfat_raw.h"
#include "exfat_fs.h"

-static int exfat_extract_uni_name(struct exfat_dentry *ep,
- unsigned short *uniname)
-{
- int i, len = 0;
-
- for (i = 0; i < EXFAT_FILE_NAME_LEN; i++) {
- *uniname = le16_to_cpu(ep->dentry.name.unicode_0_14[i]);
- if (*uniname == 0x0)
- return len;
- uniname++;
- len++;
- }
-
- *uniname = 0x0;
- return len;
-
-}
-
static int exfat_get_uniname_from_name_entries(struct exfat_entry_set_cache *es,
struct exfat_uni_name *uniname)
{
@@ -871,13 +853,6 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
return NULL;
}

-enum {
- DIRENT_STEP_FILE,
- DIRENT_STEP_STRM,
- DIRENT_STEP_NAME,
- DIRENT_STEP_SECD,
-};
-
/*
* return values:
* >= 0 : return dir entiry position with the name in dir
@@ -887,13 +862,12 @@ enum {
*/
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 i, rewind = 0, dentry = 0, end_eidx = 0, num_ext = 0, len;
- int order, step, name_len = 0;
+ int i, rewind = 0, dentry = 0, end_eidx = 0, num_ext = 0;
+ int name_len = 0;
int dentries_per_clu, num_empty = 0;
unsigned int entry_type;
- unsigned short *uniname = NULL;
struct exfat_chain clu;
struct exfat_hint *hint_stat = &ei->hint_stat;
struct exfat_hint_femp candi_empty;
@@ -911,27 +885,34 @@ int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,

candi_empty.eidx = EXFAT_HINT_NONE;
rewind:
- order = 0;
- step = DIRENT_STEP_FILE;
while (clu.dir != EXFAT_EOF_CLUSTER) {
i = dentry & (dentries_per_clu - 1);
for (; i < dentries_per_clu; i++, dentry++) {
struct exfat_dentry *ep;
struct buffer_head *bh;
+ struct exfat_entry_set_cache *es;
+ struct exfat_uni_name uni_name;
+ u16 name_hash;
+ bool found;

if (rewind && dentry == end_eidx)
goto not_found;

+ /* skip secondary dir-entries in previous dir-entry set */
+ if (num_ext) {
+ num_ext--;
+ continue;
+ }
+
ep = exfat_get_dentry(sb, &clu, i, &bh, NULL);
if (!ep)
return -EIO;

entry_type = exfat_get_entry_type(ep);
+ brelse(bh);

if (entry_type == TYPE_UNUSED ||
entry_type == TYPE_DELETED) {
- step = DIRENT_STEP_FILE;
-
num_empty++;
if (candi_empty.eidx == EXFAT_HINT_NONE &&
num_empty == 1) {
@@ -956,7 +937,6 @@ int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,
}
}

- brelse(bh);
if (entry_type == TYPE_UNUSED)
goto not_found;
continue;
@@ -965,80 +945,30 @@ int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,
num_empty = 0;
candi_empty.eidx = EXFAT_HINT_NONE;

- if (entry_type == TYPE_FILE || entry_type == TYPE_DIR) {
- step = DIRENT_STEP_FILE;
- if (type == TYPE_ALL || type == entry_type) {
- num_ext = ep->dentry.file.num_ext;
- step = DIRENT_STEP_STRM;
- }
- brelse(bh);
+ if (entry_type != TYPE_FILE && entry_type != TYPE_DIR)
continue;
- }

- if (entry_type == TYPE_STREAM) {
- u16 name_hash;
-
- if (step != DIRENT_STEP_STRM) {
- step = DIRENT_STEP_FILE;
- brelse(bh);
- continue;
- }
- step = DIRENT_STEP_FILE;
- name_hash = le16_to_cpu(
- ep->dentry.stream.name_hash);
- if (p_uniname->name_hash == name_hash &&
- p_uniname->name_len ==
- ep->dentry.stream.name_len) {
- step = DIRENT_STEP_NAME;
- order = 1;
- name_len = 0;
- }
- brelse(bh);
+ es = exfat_get_dentry_set(sb, &ei->dir, dentry, ES_ALL_ENTRIES);
+ if (!es)
continue;
- }

- brelse(bh);
- if (entry_type == TYPE_NAME) {
- unsigned short entry_uniname[16], unichar;
+ num_ext = es->de_file->num_ext;
+ name_hash = le16_to_cpu(es->de_stream->name_hash);
+ name_len = es->de_stream->name_len;

- if (step != DIRENT_STEP_NAME) {
- step = DIRENT_STEP_FILE;
- continue;
- }
-
- if (++order == 2)
- uniname = p_uniname->name;
- else
- uniname += EXFAT_FILE_NAME_LEN;
-
- len = exfat_extract_uni_name(ep, entry_uniname);
- name_len += len;
-
- unichar = *(uniname+len);
- *(uniname+len) = 0x0;
-
- if (exfat_uniname_ncmp(sb, uniname,
- entry_uniname, len)) {
- step = DIRENT_STEP_FILE;
- } else if (p_uniname->name_len == name_len) {
- if (order == num_ext)
- goto found;
- step = DIRENT_STEP_SECD;
- }
+ found = p_uniname->name_hash == name_hash &&
+ p_uniname->name_len == name_len &&
+ !exfat_get_uniname_from_name_entries(es, &uni_name) &&
+ !exfat_uniname_ncmp(sb, p_uniname->name, uni_name.name, name_len);

- *(uniname+len) = unichar;
- continue;
- }
+ exfat_free_dentry_set(es, false);

- if (entry_type &
- (TYPE_CRITICAL_SEC | TYPE_BENIGN_SEC)) {
- if (step == DIRENT_STEP_SECD) {
- if (++order == num_ext)
- goto found;
- continue;
- }
+ if (found) {
+ /* set the last used position as hint */
+ hint_stat->clu = clu.dir;
+ hint_stat->eidx = dentry;
+ return dentry;
}
- step = DIRENT_STEP_FILE;
}

if (clu.flags == ALLOC_NO_FAT_CHAIN) {
@@ -1071,32 +1001,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 19440e7fa439..489881bf9bde 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 aece627e4e83..c190e56be3a5 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