[PATCH v4 1/5] exfat: integrates dir-entry getting and validation

[PATCH v4 1/5] exfat: integrates dir-entry getting and validation

[Tetsuhiro Kohada]

Add validation for num, bh and type on getting dir-entry.
Renamed exfat_get_dentry_cached() to exfat_get_validated_dentry() due to
a change in functionality.

Integrate type-validation with simplified.
This will also recognize a dir-entry set that contains 'benign secondary'
dir-entries.

Pre-Validated 'file' and 'stream-ext' dir-entries are provided via
ES_FILE/ES_STREAM macros.

And, rename TYPE_EXTEND to TYPE_NAME.

Suggested-by: Sungjong Seo <sj1557.seo@samsung.com>
Suggested-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
---
Changes in v2
 - Change verification order
 - Verification loop start with index 2
Changes in v3
 - Fix indent 
 - Fix comment of exfat_get_dentry_set()
 - Add de_file/de_stream in exfat_entry_set_cache
 - Add srtuct tag name for each dir-entry type in exfat_dentry
 - Add description about de_file/de_stream to commit-log
Changes in v4
 - Replace de_file/de_stream with ep_file/ep_stream in exfat_entry_set_cache
 - Remove srtuct tag names added in v3
 - Add ES_INDEX_XXX macro definitions
 - Add ES_FILE/ES_STREAM macro definitions
 - Replace some EXFAT_FIRST_CLUSTER with ES_INDEX_NAME
 - Modify commit-log

 fs/exfat/dir.c       | 155 ++++++++++++++++++-------------------------
 fs/exfat/exfat_fs.h  |  19 ++++--
 fs/exfat/exfat_raw.h |  14 ++--
 fs/exfat/file.c      |  25 +++----
 fs/exfat/inode.c     |  49 ++++++--------
 fs/exfat/namei.c     |  36 +++++-----
 6 files changed, 132 insertions(+), 166 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index 573659bfbc55..bb3c20bac422 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -33,6 +33,7 @@ static void exfat_get_uniname_from_ext_entry(struct super_block *sb,
 {
 	int i;
 	struct exfat_entry_set_cache *es;
+	struct exfat_dentry *ep;
 
 	es = exfat_get_dentry_set(sb, p_dir, entry, ES_ALL_ENTRIES);
 	if (!es)
@@ -44,13 +45,9 @@ 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.
 	 */
-	for (i = 2; i < es->num_entries; i++) {
-		struct exfat_dentry *ep = exfat_get_dentry_cached(es, i);
-
-		/* end of name entry */
-		if (exfat_get_entry_type(ep) != TYPE_EXTEND)
-			break;
 
+	i = ES_INDEX_NAME;
+	while ((ep = exfat_get_validated_dentry(es, i++, TYPE_NAME))) {
 		exfat_extract_uni_name(ep, uniname);
 		uniname += EXFAT_FILE_NAME_LEN;
 	}
@@ -372,7 +369,7 @@ unsigned int exfat_get_entry_type(struct exfat_dentry *ep)
 		if (ep->type == EXFAT_STREAM)
 			return TYPE_STREAM;
 		if (ep->type == EXFAT_NAME)
-			return TYPE_EXTEND;
+			return TYPE_NAME;
 		if (ep->type == EXFAT_ACL)
 			return TYPE_ACL;
 		return TYPE_CRITICAL_SEC;
@@ -388,7 +385,7 @@ static void exfat_set_entry_type(struct exfat_dentry *ep, unsigned int type)
 		ep->type &= EXFAT_DELETE;
 	} else if (type == TYPE_STREAM) {
 		ep->type = EXFAT_STREAM;
-	} else if (type == TYPE_EXTEND) {
+	} else if (type == TYPE_NAME) {
 		ep->type = EXFAT_NAME;
 	} else if (type == TYPE_BITMAP) {
 		ep->type = EXFAT_BITMAP;
@@ -421,7 +418,7 @@ static void exfat_init_name_entry(struct exfat_dentry *ep,
 {
 	int i;
 
-	exfat_set_entry_type(ep, TYPE_EXTEND);
+	exfat_set_entry_type(ep, TYPE_NAME);
 	ep->dentry.name.flags = 0x0;
 
 	for (i = 0; i < EXFAT_FILE_NAME_LEN; i++) {
@@ -550,7 +547,7 @@ int exfat_init_ext_entry(struct inode *inode, struct exfat_chain *p_dir,
 	exfat_update_bh(bh, sync);
 	brelse(bh);
 
-	for (i = EXFAT_FIRST_CLUSTER; i < num_entries; i++) {
+	for (i = ES_INDEX_NAME; i < num_entries; i++) {
 		ep = exfat_get_dentry(sb, p_dir, entry + i, &bh, &sector);
 		if (!ep)
 			return -EIO;
@@ -590,17 +587,16 @@ int exfat_remove_entries(struct inode *inode, struct exfat_chain *p_dir,
 void exfat_update_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es)
 {
 	int chksum_type = CS_DIR_ENTRY, i;
-	unsigned short chksum = 0;
+	u16 chksum = 0;
 	struct exfat_dentry *ep;
 
 	for (i = 0; i < es->num_entries; i++) {
-		ep = exfat_get_dentry_cached(es, i);
+		ep = exfat_get_validated_dentry(es, i, TYPE_ALL);
 		chksum = exfat_calc_chksum16(ep, DENTRY_SIZE, chksum,
 					     chksum_type);
 		chksum_type = CS_DEFAULT;
 	}
-	ep = exfat_get_dentry_cached(es, 0);
-	ep->dentry.file.checksum = cpu_to_le16(chksum);
+	ES_FILE(es).checksum = cpu_to_le16(chksum);
 	es->modified = true;
 }
 
@@ -741,92 +737,63 @@ struct exfat_dentry *exfat_get_dentry(struct super_block *sb,
 	return (struct exfat_dentry *)((*bh)->b_data + off);
 }
 
-enum exfat_validate_dentry_mode {
-	ES_MODE_STARTED,
-	ES_MODE_GET_FILE_ENTRY,
-	ES_MODE_GET_STRM_ENTRY,
-	ES_MODE_GET_NAME_ENTRY,
-	ES_MODE_GET_CRITICAL_SEC_ENTRY,
-};
-
-static bool exfat_validate_entry(unsigned int type,
-		enum exfat_validate_dentry_mode *mode)
-{
-	if (type == TYPE_UNUSED || type == TYPE_DELETED)
-		return false;
-
-	switch (*mode) {
-	case ES_MODE_STARTED:
-		if  (type != TYPE_FILE && type != TYPE_DIR)
-			return false;
-		*mode = ES_MODE_GET_FILE_ENTRY;
-		return true;
-	case ES_MODE_GET_FILE_ENTRY:
-		if (type != TYPE_STREAM)
-			return false;
-		*mode = ES_MODE_GET_STRM_ENTRY;
-		return true;
-	case ES_MODE_GET_STRM_ENTRY:
-		if (type != TYPE_EXTEND)
-			return false;
-		*mode = ES_MODE_GET_NAME_ENTRY;
-		return true;
-	case ES_MODE_GET_NAME_ENTRY:
-		if (type == TYPE_STREAM)
-			return false;
-		if (type != TYPE_EXTEND) {
-			if (!(type & TYPE_CRITICAL_SEC))
-				return false;
-			*mode = ES_MODE_GET_CRITICAL_SEC_ENTRY;
-		}
-		return true;
-	case ES_MODE_GET_CRITICAL_SEC_ENTRY:
-		if (type == TYPE_EXTEND || type == TYPE_STREAM)
-			return false;
-		if ((type & TYPE_CRITICAL_SEC) != TYPE_CRITICAL_SEC)
-			return false;
-		return true;
-	default:
-		WARN_ON_ONCE(1);
-		return false;
-	}
-}
-
-struct exfat_dentry *exfat_get_dentry_cached(
-	struct exfat_entry_set_cache *es, int num)
+struct exfat_dentry *exfat_get_validated_dentry(struct exfat_entry_set_cache *es,
+		int num, unsigned int type)
 {
 	int off = es->start_off + num * DENTRY_SIZE;
-	struct buffer_head *bh = es->bh[EXFAT_B_TO_BLK(off, es->sb)];
-	char *p = bh->b_data + EXFAT_BLK_OFFSET(off, es->sb);
+	struct buffer_head *bh;
+	struct exfat_dentry *ep;
+
+	if (num >= es->num_entries)
+		return NULL;
+
+	bh = es->bh[EXFAT_B_TO_BLK(off, es->sb)];
+	if (!bh)
+		return NULL;
 
-	return (struct exfat_dentry *)p;
+	ep = (struct exfat_dentry *)
+		(bh->b_data + EXFAT_BLK_OFFSET(off, es->sb));
+
+	switch (type) {
+	case TYPE_ALL: /* accept any */
+		break;
+	case TYPE_FILE:
+		if (ep->type != EXFAT_FILE)
+			return NULL;
+		break;
+	case TYPE_SECONDARY:
+		if (!(type & exfat_get_entry_type(ep)))
+			return NULL;
+		break;
+	default:
+		if (type != exfat_get_entry_type(ep))
+			return NULL;
+	}
+	return ep;
 }
 
 /*
  * Returns a set of dentries for a file or dir.
  *
- * Note It provides a direct pointer to bh->data via exfat_get_dentry_cached().
+ * Note It provides a direct pointer to bh->data via exfat_get_validated_dentry().
  * User should call exfat_get_dentry_set() after setting 'modified' to apply
  * changes made in this entry set to the real device.
  *
  * in:
  *   sb+p_dir+entry: indicates a file/dir
- *   type:  specifies how many dentries should be included.
+ *   max_entries:  specifies how many dentries should be included.
  * return:
  *   pointer of entry set on success,
  *   NULL on failure.
  */
 struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
-		struct exfat_chain *p_dir, int entry, unsigned int type)
+		struct exfat_chain *p_dir, int entry, int max_entries)
 {
 	int ret, i, num_bh;
-	unsigned int off, byte_offset, clu = 0;
+	unsigned int byte_offset, clu = 0;
 	sector_t sec;
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
 	struct exfat_entry_set_cache *es;
-	struct exfat_dentry *ep;
-	int num_entries;
-	enum exfat_validate_dentry_mode mode = ES_MODE_STARTED;
 	struct buffer_head *bh;
 
 	if (p_dir->dir == DIR_DELETED) {
@@ -844,13 +811,13 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
 		return NULL;
 	es->sb = sb;
 	es->modified = false;
+	es->num_entries = 1;
 
 	/* byte offset in cluster */
 	byte_offset = EXFAT_CLU_OFFSET(byte_offset, sbi);
 
 	/* byte offset in sector */
-	off = EXFAT_BLK_OFFSET(byte_offset, sb);
-	es->start_off = off;
+	es->start_off = EXFAT_BLK_OFFSET(byte_offset, sb);
 
 	/* sector offset in cluster */
 	sec = EXFAT_B_TO_BLK(byte_offset, sb);
@@ -861,15 +828,12 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
 		goto free_es;
 	es->bh[es->num_bh++] = bh;
 
-	ep = exfat_get_dentry_cached(es, 0);
-	if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode))
+	es->ep_file = exfat_get_validated_dentry(es, ES_INDEX_FILE, TYPE_FILE);
+	if (!es->ep_file)
 		goto free_es;
+	es->num_entries = min(ES_FILE(es).num_ext + 1, max_entries);
 
-	num_entries = type == ES_ALL_ENTRIES ?
-		ep->dentry.file.num_ext + 1 : type;
-	es->num_entries = num_entries;
-
-	num_bh = EXFAT_B_TO_BLK_ROUND_UP(off + num_entries * DENTRY_SIZE, sb);
+	num_bh = EXFAT_B_TO_BLK_ROUND_UP(es->start_off  + es->num_entries * DENTRY_SIZE, sb);
 	for (i = 1; i < num_bh; i++) {
 		/* get the next sector */
 		if (exfat_is_last_sector_in_cluster(sbi, sec)) {
@@ -889,10 +853,17 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
 	}
 
 	/* validiate cached dentries */
-	for (i = 1; i < num_entries; i++) {
-		ep = exfat_get_dentry_cached(es, i);
-		if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode))
-			goto free_es;
+	es->ep_stream = exfat_get_validated_dentry(es, ES_INDEX_STREAM, TYPE_STREAM);
+	if (!es->ep_stream)
+		goto free_es;
+
+	if (max_entries == ES_ALL_ENTRIES) {
+		for (i = 0; i < ES_FILE(es).num_ext; i++)
+			if (!exfat_get_validated_dentry(es, ES_INDEX_STREAM + i, TYPE_SECONDARY))
+				goto free_es;
+		for (i = 0; i * EXFAT_FILE_NAME_LEN < ES_STREAM(es).name_len; i++)
+			if (!exfat_get_validated_dentry(es, ES_INDEX_NAME + i, TYPE_NAME))
+				goto free_es;
 	}
 	return es;
 
@@ -1028,7 +999,7 @@ int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,
 			}
 
 			brelse(bh);
-			if (entry_type == TYPE_EXTEND) {
+			if (entry_type == TYPE_NAME) {
 				unsigned short entry_uniname[16], unichar;
 
 				if (step != DIRENT_STEP_NAME) {
@@ -1144,7 +1115,7 @@ int exfat_count_ext_entries(struct super_block *sb, struct exfat_chain *p_dir,
 
 		type = exfat_get_entry_type(ext_ep);
 		brelse(bh);
-		if (type == TYPE_EXTEND || type == TYPE_STREAM)
+		if (type == TYPE_NAME || type == TYPE_STREAM)
 			count++;
 		else
 			break;
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index 44dc04520175..e46f3e0c16b7 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -40,7 +40,7 @@ enum {
  * Type Definitions
  */
 #define ES_2_ENTRIES		2
-#define ES_ALL_ENTRIES		0
+#define ES_ALL_ENTRIES		256
 
 #define DIR_DELETED		0xFFFF0321
 
@@ -56,7 +56,7 @@ enum {
 #define TYPE_FILE		0x011F
 #define TYPE_CRITICAL_SEC	0x0200
 #define TYPE_STREAM		0x0201
-#define TYPE_EXTEND		0x0202
+#define TYPE_NAME		0x0202
 #define TYPE_ACL		0x0203
 #define TYPE_BENIGN_PRI		0x0400
 #define TYPE_GUID		0x0401
@@ -65,6 +65,9 @@ enum {
 #define TYPE_BENIGN_SEC		0x0800
 #define TYPE_ALL		0x0FFF
 
+#define TYPE_PRIMARY		(TYPE_CRITICAL_PRI | TYPE_BENIGN_PRI)
+#define TYPE_SECONDARY		(TYPE_CRITICAL_SEC | TYPE_BENIGN_SEC)
+
 #define MAX_CHARSET_SIZE	6 /* max size of multi-byte character */
 #define MAX_NAME_LENGTH		255 /* max len of file name excluding NULL */
 #define MAX_VFSNAME_BUF_SIZE	((MAX_NAME_LENGTH + 1) * MAX_CHARSET_SIZE)
@@ -171,9 +174,13 @@ struct exfat_entry_set_cache {
 	unsigned int start_off;
 	int num_bh;
 	struct buffer_head *bh[DIR_CACHE_SIZE];
-	unsigned int num_entries;
+	int num_entries;
+	struct exfat_dentry *ep_file, *ep_stream;
 };
 
+#define ES_FILE(es)	((es)->ep_file->dentry.file)
+#define ES_STREAM(es)	((es)->ep_stream->dentry.stream)
+
 struct exfat_dir_entry {
 	struct exfat_chain dir;
 	int entry;
@@ -458,10 +465,10 @@ int exfat_find_location(struct super_block *sb, struct exfat_chain *p_dir,
 struct exfat_dentry *exfat_get_dentry(struct super_block *sb,
 		struct exfat_chain *p_dir, int entry, struct buffer_head **bh,
 		sector_t *sector);
-struct exfat_dentry *exfat_get_dentry_cached(struct exfat_entry_set_cache *es,
-		int num);
+struct exfat_dentry *exfat_get_validated_dentry(struct exfat_entry_set_cache *es,
+		int num, unsigned int type);
 struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
-		struct exfat_chain *p_dir, int entry, unsigned int type);
+		struct exfat_chain *p_dir, int entry, int max_entries);
 int exfat_free_dentry_set(struct exfat_entry_set_cache *es, int sync);
 int exfat_count_dir_entries(struct super_block *sb, struct exfat_chain *p_dir);
 
diff --git a/fs/exfat/exfat_raw.h b/fs/exfat/exfat_raw.h
index 6aec6288e1f2..7e556379ac4b 100644
--- a/fs/exfat/exfat_raw.h
+++ b/fs/exfat/exfat_raw.h
@@ -77,6 +77,10 @@
 
 #define EXFAT_FILE_NAME_LEN		15
 
+#define	ES_INDEX_FILE		0
+#define	ES_INDEX_STREAM		1
+#define	ES_INDEX_NAME		2
+
 /* EXFAT: Main and Backup Boot Sector (512 bytes) */
 struct boot_sector {
 	__u8	jmp_boot[BOOTSEC_JUMP_BOOT_LEN];
@@ -105,7 +109,7 @@ struct boot_sector {
 struct exfat_dentry {
 	__u8 type;
 	union {
-		struct {
+		struct exfat_de_file {
 			__u8 num_ext;
 			__le16 checksum;
 			__le16 attr;
@@ -123,7 +127,7 @@ struct exfat_dentry {
 			__u8 access_tz;
 			__u8 reserved2[7];
 		} __packed file; /* file directory entry */
-		struct {
+		struct exfat_de_stream {
 			__u8 flags;
 			__u8 reserved1;
 			__u8 name_len;
@@ -134,17 +138,17 @@ struct exfat_dentry {
 			__le32 start_clu;
 			__le64 size;
 		} __packed stream; /* stream extension directory entry */
-		struct {
+		struct exfat_de_name {
 			__u8 flags;
 			__le16 unicode_0_14[EXFAT_FILE_NAME_LEN];
 		} __packed name; /* file name directory entry */
-		struct {
+		struct exfat_de_bitmap {
 			__u8 flags;
 			__u8 reserved[18];
 			__le32 start_clu;
 			__le64 size;
 		} __packed bitmap; /* allocation bitmap directory entry */
-		struct {
+		struct exfat_de_upcase {
 			__u8 reserved1[3];
 			__le32 checksum;
 			__u8 reserved2[12];
diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index 4831a39632a1..504ffcaffacc 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -152,7 +152,6 @@ int __exfat_truncate(struct inode *inode, loff_t new_size)
 	/* update the directory entry */
 	if (!evict) {
 		struct timespec64 ts;
-		struct exfat_dentry *ep, *ep2;
 		struct exfat_entry_set_cache *es;
 		int err;
 
@@ -160,32 +159,30 @@ int __exfat_truncate(struct inode *inode, loff_t new_size)
 				ES_ALL_ENTRIES);
 		if (!es)
 			return -EIO;
-		ep = exfat_get_dentry_cached(es, 0);
-		ep2 = exfat_get_dentry_cached(es, 1);
 
 		ts = current_time(inode);
 		exfat_set_entry_time(sbi, &ts,
-				&ep->dentry.file.modify_tz,
-				&ep->dentry.file.modify_time,
-				&ep->dentry.file.modify_date,
-				&ep->dentry.file.modify_time_cs);
-		ep->dentry.file.attr = cpu_to_le16(ei->attr);
+				&ES_FILE(es).modify_tz,
+				&ES_FILE(es).modify_time,
+				&ES_FILE(es).modify_date,
+				&ES_FILE(es).modify_time_cs);
+		ES_FILE(es).attr = cpu_to_le16(ei->attr);
 
 		/* File size should be zero if there is no cluster allocated */
 		if (ei->start_clu == EXFAT_EOF_CLUSTER) {
-			ep2->dentry.stream.valid_size = 0;
-			ep2->dentry.stream.size = 0;
+			ES_STREAM(es).valid_size = 0;
+			ES_STREAM(es).size = 0;
 		} else {
-			ep2->dentry.stream.valid_size = cpu_to_le64(new_size);
-			ep2->dentry.stream.size = ep2->dentry.stream.valid_size;
+			ES_STREAM(es).valid_size = cpu_to_le64(new_size);
+			ES_STREAM(es).size = ES_STREAM(es).valid_size;
 		}
 
 		if (new_size == 0) {
 			/* Any directory can not be truncated to zero */
 			WARN_ON(ei->type != TYPE_FILE);
 
-			ep2->dentry.stream.flags = ALLOC_FAT_CHAIN;
-			ep2->dentry.stream.start_clu = EXFAT_FREE_CLUSTER;
+			ES_STREAM(es).flags = ALLOC_FAT_CHAIN;
+			ES_STREAM(es).start_clu = EXFAT_FREE_CLUSTER;
 		}
 
 		exfat_update_dir_chksum_with_entry_set(es);
diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c
index 7f90204adef5..466f0bf07f75 100644
--- a/fs/exfat/inode.c
+++ b/fs/exfat/inode.c
@@ -20,7 +20,6 @@
 static int __exfat_write_inode(struct inode *inode, int sync)
 {
 	unsigned long long on_disk_size;
-	struct exfat_dentry *ep, *ep2;
 	struct exfat_entry_set_cache *es = NULL;
 	struct super_block *sb = inode->i_sb;
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
@@ -45,26 +44,24 @@ static int __exfat_write_inode(struct inode *inode, int sync)
 	es = exfat_get_dentry_set(sb, &(ei->dir), ei->entry, ES_ALL_ENTRIES);
 	if (!es)
 		return -EIO;
-	ep = exfat_get_dentry_cached(es, 0);
-	ep2 = exfat_get_dentry_cached(es, 1);
 
-	ep->dentry.file.attr = cpu_to_le16(exfat_make_attr(inode));
+	ES_FILE(es).attr = cpu_to_le16(exfat_make_attr(inode));
 
 	/* set FILE_INFO structure using the acquired struct exfat_dentry */
 	exfat_set_entry_time(sbi, &ei->i_crtime,
-			&ep->dentry.file.create_tz,
-			&ep->dentry.file.create_time,
-			&ep->dentry.file.create_date,
-			&ep->dentry.file.create_time_cs);
+			&ES_FILE(es).create_tz,
+			&ES_FILE(es).create_time,
+			&ES_FILE(es).create_date,
+			&ES_FILE(es).create_time_cs);
 	exfat_set_entry_time(sbi, &inode->i_mtime,
-			&ep->dentry.file.modify_tz,
-			&ep->dentry.file.modify_time,
-			&ep->dentry.file.modify_date,
-			&ep->dentry.file.modify_time_cs);
+			&ES_FILE(es).modify_tz,
+			&ES_FILE(es).modify_time,
+			&ES_FILE(es).modify_date,
+			&ES_FILE(es).modify_time_cs);
 	exfat_set_entry_time(sbi, &inode->i_atime,
-			&ep->dentry.file.access_tz,
-			&ep->dentry.file.access_time,
-			&ep->dentry.file.access_date,
+			&ES_FILE(es).access_tz,
+			&ES_FILE(es).access_time,
+			&ES_FILE(es).access_date,
 			NULL);
 
 	/* File size should be zero if there is no cluster allocated */
@@ -73,8 +70,8 @@ static int __exfat_write_inode(struct inode *inode, int sync)
 	if (ei->start_clu == EXFAT_EOF_CLUSTER)
 		on_disk_size = 0;
 
-	ep2->dentry.stream.valid_size = cpu_to_le64(on_disk_size);
-	ep2->dentry.stream.size = ep2->dentry.stream.valid_size;
+	ES_STREAM(es).valid_size = cpu_to_le64(on_disk_size);
+	ES_STREAM(es).size = ES_STREAM(es).valid_size;
 
 	exfat_update_dir_chksum_with_entry_set(es);
 	return exfat_free_dentry_set(es, sync);
@@ -219,7 +216,6 @@ static int exfat_map_cluster(struct inode *inode, unsigned int clu_offset,
 		*clu = new_clu.dir;
 
 		if (ei->dir.dir != DIR_DELETED && modified) {
-			struct exfat_dentry *ep;
 			struct exfat_entry_set_cache *es;
 			int err;
 
@@ -227,17 +223,12 @@ static int exfat_map_cluster(struct inode *inode, unsigned int clu_offset,
 				ES_ALL_ENTRIES);
 			if (!es)
 				return -EIO;
-			/* get stream entry */
-			ep = exfat_get_dentry_cached(es, 1);
-
-			/* update directory entry */
-			ep->dentry.stream.flags = ei->flags;
-			ep->dentry.stream.start_clu =
-				cpu_to_le32(ei->start_clu);
-			ep->dentry.stream.valid_size =
-				cpu_to_le64(i_size_read(inode));
-			ep->dentry.stream.size =
-				ep->dentry.stream.valid_size;
+
+			/* update stream directory entry */
+			ES_STREAM(es).flags = ei->flags;
+			ES_STREAM(es).start_clu = cpu_to_le32(ei->start_clu);
+			ES_STREAM(es).valid_size = cpu_to_le64(i_size_read(inode));
+			ES_STREAM(es).size = ES_STREAM(es).valid_size;
 
 			exfat_update_dir_chksum_with_entry_set(es);
 			err = exfat_free_dentry_set(es, inode_needs_sync(inode));
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index 2aff6605fecc..6a034f504d83 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -658,25 +658,21 @@ static int exfat_find(struct inode *dir, struct qstr *qname,
 
 		info->num_subdirs = count;
 	} else {
-		struct exfat_dentry *ep, *ep2;
 		struct exfat_entry_set_cache *es;
 
 		es = exfat_get_dentry_set(sb, &cdir, dentry, ES_2_ENTRIES);
 		if (!es)
 			return -EIO;
-		ep = exfat_get_dentry_cached(es, 0);
-		ep2 = exfat_get_dentry_cached(es, 1);
 
-		info->type = exfat_get_entry_type(ep);
-		info->attr = le16_to_cpu(ep->dentry.file.attr);
-		info->size = le64_to_cpu(ep2->dentry.stream.valid_size);
+		info->attr = le16_to_cpu(ES_FILE(es).attr);
+		info->type = (info->attr & ATTR_SUBDIR) ? TYPE_DIR : TYPE_FILE;
+		info->size = le64_to_cpu(ES_STREAM(es).valid_size);
 		if ((info->type == TYPE_FILE) && (info->size == 0)) {
 			info->flags = ALLOC_NO_FAT_CHAIN;
 			info->start_clu = EXFAT_EOF_CLUSTER;
 		} else {
-			info->flags = ep2->dentry.stream.flags;
-			info->start_clu =
-				le32_to_cpu(ep2->dentry.stream.start_clu);
+			info->flags = ES_STREAM(es).flags;
+			info->start_clu = le32_to_cpu(ES_STREAM(es).start_clu);
 		}
 
 		if (ei->start_clu == EXFAT_FREE_CLUSTER) {
@@ -688,19 +684,19 @@ static int exfat_find(struct inode *dir, struct qstr *qname,
 		}
 
 		exfat_get_entry_time(sbi, &info->crtime,
-				ep->dentry.file.create_tz,
-				ep->dentry.file.create_time,
-				ep->dentry.file.create_date,
-				ep->dentry.file.create_time_cs);
+				ES_FILE(es).create_tz,
+				ES_FILE(es).create_time,
+				ES_FILE(es).create_date,
+				ES_FILE(es).create_time_cs);
 		exfat_get_entry_time(sbi, &info->mtime,
-				ep->dentry.file.modify_tz,
-				ep->dentry.file.modify_time,
-				ep->dentry.file.modify_date,
-				ep->dentry.file.modify_time_cs);
+				ES_FILE(es).modify_tz,
+				ES_FILE(es).modify_time,
+				ES_FILE(es).modify_date,
+				ES_FILE(es).modify_time_cs);
 		exfat_get_entry_time(sbi, &info->atime,
-				ep->dentry.file.access_tz,
-				ep->dentry.file.access_time,
-				ep->dentry.file.access_date,
+				ES_FILE(es).access_tz,
+				ES_FILE(es).access_time,
+				ES_FILE(es).access_date,
 				0);
 		exfat_free_dentry_set(es, false);
 
-- 
2.25.1

[PATCH v4 2/5] exfat: add NameLength check when extracting name

[Tetsuhiro Kohada]

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-entry, 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 of exfat_entry_set_cache.

Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
---
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
Changes in v3:
 - Nothing
Changes in v4:
 - Into patch series '[PATCH v4] exfat: integrates dir-entry getting and validation'

 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 bb3c20bac422..99d9e6d119d6 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_STREAM(es).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 = ES_INDEX_NAME;
-	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 = ES_INDEX_NAME; 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_FILE(es).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_FILE(es).create_tz,
+					ES_FILE(es).create_time,
+					ES_FILE(es).create_date,
+					ES_FILE(es).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_FILE(es).modify_tz,
+					ES_FILE(es).modify_time,
+					ES_FILE(es).modify_date,
+					ES_FILE(es).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_FILE(es).access_tz,
+					ES_FILE(es).access_time,
+					ES_FILE(es).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_STREAM(es).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

[PATCH v4 3/5] exfat: unify name extraction

[Tetsuhiro Kohada]

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.

Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
---
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
Changes in v3:
 - Nothing
Changes in v4:
 - Into patch series '[PATCH v4] exfat: integrates dir-entry getting and validation'

 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 99d9e6d119d6..cd37091844fa 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_FILE(es).num_ext;
+			name_hash = le16_to_cpu(ES_STREAM(es).name_hash);
+			name_len = ES_STREAM(es).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 e46f3e0c16b7..7057e64b405d 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -458,7 +458,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 6a034f504d83..d2b9044d0b31 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

[PATCH v4 4/5] exfat: add dir-entry set checksum validation

[Tetsuhiro Kohada]

Add checksum validation for dir-entry set when getting it.
exfat_calc_entry_set_chksum_with() also validates entry-type.

Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
---
Changes in v2
 - Add error log if checksum mismatch
Changes in v3:
 - Nothing
Changes in v4:
 - Into patch series '[PATCH v4] exfat: integrates dir-entry getting and validation'

 fs/exfat/dir.c | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index cd37091844fa..d4beea796708 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -565,18 +565,26 @@ int exfat_remove_entries(struct inode *inode, struct exfat_chain *p_dir,
 	return 0;
 }
 
-void exfat_update_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es)
+static int exfat_calc_entry_set_chksum(struct exfat_entry_set_cache *es, u16 *chksum)
 {
-	int chksum_type = CS_DIR_ENTRY, i;
-	u16 chksum = 0;
 	struct exfat_dentry *ep;
+	int i;
 
-	for (i = 0; i < es->num_entries; i++) {
-		ep = exfat_get_validated_dentry(es, i, TYPE_ALL);
-		chksum = exfat_calc_chksum16(ep, DENTRY_SIZE, chksum,
-					     chksum_type);
-		chksum_type = CS_DEFAULT;
+	*chksum = exfat_calc_chksum16(es->ep_file, DENTRY_SIZE, 0, CS_DIR_ENTRY);
+	for (i = 0; i < ES_FILE(es).num_ext; i++) {
+		ep = exfat_get_validated_dentry(es, 1 + i, TYPE_SECONDARY);
+		if (!ep)
+			return -EIO;
+		*chksum = exfat_calc_chksum16(ep, DENTRY_SIZE, *chksum, CS_DEFAULT);
 	}
+	return 0;
+}
+
+void exfat_update_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es)
+{
+	u16 chksum;
+
+	exfat_calc_entry_set_chksum(es, &chksum);
 	ES_FILE(es).checksum = cpu_to_le16(chksum);
 	es->modified = true;
 }
@@ -776,6 +784,7 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
 	struct exfat_entry_set_cache *es;
 	struct buffer_head *bh;
+	u16 chksum;
 
 	if (p_dir->dir == DIR_DELETED) {
 		exfat_err(sb, "access to deleted dentry");
@@ -839,9 +848,12 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
 		goto free_es;
 
 	if (max_entries == ES_ALL_ENTRIES) {
-		for (i = 0; i < ES_FILE(es).num_ext; i++)
-			if (!exfat_get_validated_dentry(es, ES_INDEX_STREAM + i, TYPE_SECONDARY))
-				goto free_es;
+		if (exfat_calc_entry_set_chksum(es, &chksum) ||
+		   chksum != le16_to_cpu(ES_FILE(es).checksum)) {
+			exfat_err(sb, "invalid entry-set checksum (entry : 0x%08x, set-checksum : 0x%04x, checksum : 0x%04x)",
+				  entry, le16_to_cpu(ES_FILE(es).checksum), chksum);
+			goto free_es;
+		}
 		for (i = 0; i * EXFAT_FILE_NAME_LEN < ES_STREAM(es).name_len; i++)
 			if (!exfat_get_validated_dentry(es, ES_INDEX_NAME + i, TYPE_NAME))
 				goto free_es;
-- 
2.25.1

[PATCH v4 5/5] exfat: write only modified part of dir-entry set

[Tetsuhiro Kohada]

Currently exfat_free_dentry_set() writes all of dir-entry set.
Change it to write only the modified part of dir-entry set.
And, Integrate exfat_free_dentry_set() and
exfat_update_dir_chksum_with_entry_set() as exfat_put_dentry_set().

Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
---
Changes in v2
 - Based on v2 'name-length' patches
Changes in v3:
 - Nothing
Changes in v4:
 - Into patch series '[PATCH v4] exfat: integrates dir-entry getting and validation'

 fs/exfat/dir.c      | 31 +++++++++++++++----------------
 fs/exfat/exfat_fs.h |  4 +---
 fs/exfat/file.c     |  3 +--
 fs/exfat/inode.c    |  6 ++----
 fs/exfat/namei.c    |  4 ++--
 5 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index d4beea796708..78539d91d3c5 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -129,7 +129,7 @@ static int exfat_readdir(struct inode *inode, struct exfat_dir_entry *dir_entry)
 			dir_entry->size = le64_to_cpu(ES_STREAM(es).valid_size);
 
 			err = exfat_get_uniname_from_name_entries(es, &uni_name);
-			exfat_free_dentry_set(es, false);
+			exfat_put_dentry_set(es, 0, false);
 			if (err)
 				return err;
 
@@ -580,21 +580,21 @@ static int exfat_calc_entry_set_chksum(struct exfat_entry_set_cache *es, u16 *ch
 	return 0;
 }
 
-void exfat_update_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es)
-{
-	u16 chksum;
-
-	exfat_calc_entry_set_chksum(es, &chksum);
-	ES_FILE(es).checksum = cpu_to_le16(chksum);
-	es->modified = true;
-}
-
-int exfat_free_dentry_set(struct exfat_entry_set_cache *es, int sync)
+int exfat_put_dentry_set(struct exfat_entry_set_cache *es, int modified, int sync)
 {
 	int i, err = 0;
 
-	if (es->modified)
-		err = exfat_update_bhs(es->bh, es->num_bh, sync);
+	if (modified) {
+		int off = es->start_off + (modified - 1) * DENTRY_SIZE;
+		int modified_bh = min(EXFAT_B_TO_BLK(off, es->sb) + 1, es->num_bh);
+		u16 chksum;
+
+		err = exfat_calc_entry_set_chksum(es, &chksum);
+		if (!err) {
+			ES_FILE(es).checksum = cpu_to_le16(chksum);
+			err = exfat_update_bhs(es->bh, modified_bh, sync);
+		}
+	}
 
 	for (i = 0; i < es->num_bh; i++)
 		if (err)
@@ -800,7 +800,6 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
 	if (!es)
 		return NULL;
 	es->sb = sb;
-	es->modified = false;
 	es->num_entries = 1;
 
 	/* byte offset in cluster */
@@ -861,7 +860,7 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
 	return es;
 
 free_es:
-	exfat_free_dentry_set(es, false);
+	exfat_put_dentry_set(es, 0, false);
 	return NULL;
 }
 
@@ -973,7 +972,7 @@ int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,
 				!exfat_get_uniname_from_name_entries(es, &uni_name) &&
 				!exfat_uniname_ncmp(sb, p_uniname->name, uni_name.name, name_len);
 
-			exfat_free_dentry_set(es, false);
+			exfat_put_dentry_set(es, 0, false);
 
 			if (found) {
 				/* set the last used position as hint */
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index 7057e64b405d..3354512629ac 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -170,7 +170,6 @@ struct exfat_hint {
 
 struct exfat_entry_set_cache {
 	struct super_block *sb;
-	bool modified;
 	unsigned int start_off;
 	int num_bh;
 	struct buffer_head *bh[DIR_CACHE_SIZE];
@@ -454,7 +453,6 @@ int exfat_remove_entries(struct inode *inode, struct exfat_chain *p_dir,
 		int entry, int order, int num_entries);
 int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
 		int entry);
-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,
@@ -469,7 +467,7 @@ struct exfat_dentry *exfat_get_validated_dentry(struct exfat_entry_set_cache *es
 		int num, unsigned int type);
 struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
 		struct exfat_chain *p_dir, int entry, int max_entries);
-int exfat_free_dentry_set(struct exfat_entry_set_cache *es, int sync);
+int exfat_put_dentry_set(struct exfat_entry_set_cache *es, int modified, int sync);
 int exfat_count_dir_entries(struct super_block *sb, struct exfat_chain *p_dir);
 
 /* inode.c */
diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index 504ffcaffacc..d2b727effb69 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -185,8 +185,7 @@ int __exfat_truncate(struct inode *inode, loff_t new_size)
 			ES_STREAM(es).start_clu = EXFAT_FREE_CLUSTER;
 		}
 
-		exfat_update_dir_chksum_with_entry_set(es);
-		err = exfat_free_dentry_set(es, inode_needs_sync(inode));
+		err = exfat_put_dentry_set(es, 2, inode_needs_sync(inode));
 		if (err)
 			return err;
 	}
diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c
index 466f0bf07f75..d587c7e02b35 100644
--- a/fs/exfat/inode.c
+++ b/fs/exfat/inode.c
@@ -73,8 +73,7 @@ static int __exfat_write_inode(struct inode *inode, int sync)
 	ES_STREAM(es).valid_size = cpu_to_le64(on_disk_size);
 	ES_STREAM(es).size = ES_STREAM(es).valid_size;
 
-	exfat_update_dir_chksum_with_entry_set(es);
-	return exfat_free_dentry_set(es, sync);
+	return exfat_put_dentry_set(es, 2, sync);
 }
 
 int exfat_write_inode(struct inode *inode, struct writeback_control *wbc)
@@ -230,8 +229,7 @@ static int exfat_map_cluster(struct inode *inode, unsigned int clu_offset,
 			ES_STREAM(es).valid_size = cpu_to_le64(i_size_read(inode));
 			ES_STREAM(es).size = ES_STREAM(es).valid_size;
 
-			exfat_update_dir_chksum_with_entry_set(es);
-			err = exfat_free_dentry_set(es, inode_needs_sync(inode));
+			err = exfat_put_dentry_set(es, 2, inode_needs_sync(inode));
 			if (err)
 				return err;
 		} /* end of if != DIR_DELETED */
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index d2b9044d0b31..283650825115 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -677,7 +677,7 @@ static int exfat_find(struct inode *dir, struct qstr *qname,
 			exfat_fs_error(sb,
 				"non-zero size file starts with zero cluster (size : %llu, p_dir : %u, entry : 0x%08x)",
 				i_size_read(dir), ei->dir.dir, ei->entry);
-			exfat_free_dentry_set(es, false);
+			exfat_put_dentry_set(es, 0, false);
 			return -EIO;
 		}
 
@@ -696,7 +696,7 @@ static int exfat_find(struct inode *dir, struct qstr *qname,
 				ES_FILE(es).access_time,
 				ES_FILE(es).access_date,
 				0);
-		exfat_free_dentry_set(es, false);
+		exfat_put_dentry_set(es, 0, false);
 
 		if (info->type == TYPE_DIR) {
 			exfat_chain_set(&cdir, info->start_clu,
-- 
2.25.1

RE: [PATCH v4 1/5] exfat: integrates dir-entry getting and validation

[Namjae Jeon]

> +	i = ES_INDEX_NAME;
> +	while ((ep = exfat_get_validated_dentry(es, i++, TYPE_NAME))) {
Please find the way to access name entries like ep_file, ep_stream
without calling exfat_get_validated_dentry().
>  		exfat_extract_uni_name(ep, uniname);
>  		uniname += EXFAT_FILE_NAME_LEN;
>  	}
> @@ -372,7 +369,7 @@ unsigned int exfat_get_entry_type(struct exfat_dentry *ep)
>  		if (ep->type == EXFAT_STREAM)
>  			return TYPE_STREAM;
>  		if (ep->type == EXFAT_NAME)
> -			return TYPE_EXTEND;
> +			return TYPE_NAME;
>  		if (ep->type == EXFAT_ACL)
>  			return TYPE_ACL;
>  		return TYPE_CRITICAL_SEC;
> @@ -388,7 +385,7 @@ static void exfat_set_entry_type(struct exfat_dentry *ep, unsigned int type)
>  		ep->type &= EXFAT_DELETE;
>  	} else if (type == TYPE_STREAM) {
>  		ep->type = EXFAT_STREAM;
> -	} else if (type == TYPE_EXTEND) {
> +	} else if (type == TYPE_NAME) {
>  		ep->type = EXFAT_NAME;
>  	} else if (type == TYPE_BITMAP) {
>  		ep->type = EXFAT_BITMAP;
> @@ -421,7 +418,7 @@ static void exfat_init_name_entry(struct exfat_dentry *ep,  {
>  	int i;
> 
> -	exfat_set_entry_type(ep, TYPE_EXTEND);
> +	exfat_set_entry_type(ep, TYPE_NAME);
>  	ep->dentry.name.flags = 0x0;
> 
>  	for (i = 0; i < EXFAT_FILE_NAME_LEN; i++) { @@ -550,7 +547,7 @@ int exfat_init_ext_entry(struct
> inode *inode, struct exfat_chain *p_dir,
>  	exfat_update_bh(bh, sync);
>  	brelse(bh);
> 
> -	for (i = EXFAT_FIRST_CLUSTER; i < num_entries; i++) {
> +	for (i = ES_INDEX_NAME; i < num_entries; i++) {
>  		ep = exfat_get_dentry(sb, p_dir, entry + i, &bh, &sector);
>  		if (!ep)
>  			return -EIO;
> @@ -590,17 +587,16 @@ int exfat_remove_entries(struct inode *inode, struct exfat_chain *p_dir,  void
> exfat_update_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es)  {
>  	int chksum_type = CS_DIR_ENTRY, i;
> -	unsigned short chksum = 0;
> +	u16 chksum = 0;
>  	struct exfat_dentry *ep;
> 
>  	for (i = 0; i < es->num_entries; i++) {
> -		ep = exfat_get_dentry_cached(es, i);
> +		ep = exfat_get_validated_dentry(es, i, TYPE_ALL);
Ditto, You do not need to repeatedly call exfat_get_validated_dentry() for the entries
which got from exfat_get_dentry_set().
>  		chksum = exfat_calc_chksum16(ep, DENTRY_SIZE, chksum,
>  					     chksum_type);
>  		chksum_type = CS_DEFAULT;
>  	}
> -	ep = exfat_get_dentry_cached(es, 0);
> -	ep->dentry.file.checksum = cpu_to_le16(chksum);
> +	ES_FILE(es).checksum = cpu_to_le16(chksum);
>  	es->modified = true;
>  }
> 
>  struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
> -		struct exfat_chain *p_dir, int entry, unsigned int type)
> +		struct exfat_chain *p_dir, int entry, int max_entries)
>  {
>  	int ret, i, num_bh;
> -	unsigned int off, byte_offset, clu = 0;
> +	unsigned int byte_offset, clu = 0;
>  	sector_t sec;
>  	struct exfat_sb_info *sbi = EXFAT_SB(sb);
>  	struct exfat_entry_set_cache *es;
> -	struct exfat_dentry *ep;
> -	int num_entries;
> -	enum exfat_validate_dentry_mode mode = ES_MODE_STARTED;
>  	struct buffer_head *bh;
> 
>  	if (p_dir->dir == DIR_DELETED) {
> @@ -844,13 +811,13 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
>  		return NULL;
>  	es->sb = sb;
>  	es->modified = false;
> +	es->num_entries = 1;
> 
>  	/* byte offset in cluster */
>  	byte_offset = EXFAT_CLU_OFFSET(byte_offset, sbi);
> 
>  	/* byte offset in sector */
> -	off = EXFAT_BLK_OFFSET(byte_offset, sb);
> -	es->start_off = off;
> +	es->start_off = EXFAT_BLK_OFFSET(byte_offset, sb);
> 
>  	/* sector offset in cluster */
>  	sec = EXFAT_B_TO_BLK(byte_offset, sb); @@ -861,15 +828,12 @@ struct exfat_entry_set_cache
> *exfat_get_dentry_set(struct super_block *sb,
>  		goto free_es;
>  	es->bh[es->num_bh++] = bh;
> 
> -	ep = exfat_get_dentry_cached(es, 0);
> -	if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode))
> +	es->ep_file = exfat_get_validated_dentry(es, ES_INDEX_FILE, TYPE_FILE);
> +	if (!es->ep_file)
>  		goto free_es;
> +	es->num_entries = min(ES_FILE(es).num_ext + 1, max_entries);
> 
> -	num_entries = type == ES_ALL_ENTRIES ?
> -		ep->dentry.file.num_ext + 1 : type;
> -	es->num_entries = num_entries;
> -
> -	num_bh = EXFAT_B_TO_BLK_ROUND_UP(off + num_entries * DENTRY_SIZE, sb);
> +	num_bh = EXFAT_B_TO_BLK_ROUND_UP(es->start_off  + es->num_entries *
> +DENTRY_SIZE, sb);
>  	for (i = 1; i < num_bh; i++) {
>  		/* get the next sector */
>  		if (exfat_is_last_sector_in_cluster(sbi, sec)) { @@ -889,10 +853,17 @@ struct
> exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
>  	}
> 
>  	/* validiate cached dentries */
> -	for (i = 1; i < num_entries; i++) {
> -		ep = exfat_get_dentry_cached(es, i);
> -		if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode))
> -			goto free_es;
> +	es->ep_stream = exfat_get_validated_dentry(es, ES_INDEX_STREAM, TYPE_STREAM);
> +	if (!es->ep_stream)
> +		goto free_es;
> +
> +	if (max_entries == ES_ALL_ENTRIES) {
> +		for (i = 0; i < ES_FILE(es).num_ext; i++)
> +			if (!exfat_get_validated_dentry(es, ES_INDEX_STREAM + i, TYPE_SECONDARY))
> +				goto free_es;
> +		for (i = 0; i * EXFAT_FILE_NAME_LEN < ES_STREAM(es).name_len; i++)
> +			if (!exfat_get_validated_dentry(es, ES_INDEX_NAME + i, TYPE_NAME))
> +				goto free_es;
Why do you unnecessarily check entries with two loops?
Please refer to the patch I sent.

>  	}
>  	return es;

> 
> diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h index 44dc04520175..e46f3e0c16b7 100644
> --- a/fs/exfat/exfat_fs.h
> +++ b/fs/exfat/exfat_fs.h
> @@ -40,7 +40,7 @@ enum {
>   * Type Definitions
>   */
>  #define ES_2_ENTRIES		2
> -#define ES_ALL_ENTRIES		0
> +#define ES_ALL_ENTRIES		256
> 
>  #define DIR_DELETED		0xFFFF0321
> 
> @@ -56,7 +56,7 @@ enum {
>  #define TYPE_FILE		0x011F
>  #define TYPE_CRITICAL_SEC	0x0200
>  #define TYPE_STREAM		0x0201
> -#define TYPE_EXTEND		0x0202
> +#define TYPE_NAME		0x0202
>  #define TYPE_ACL		0x0203
>  #define TYPE_BENIGN_PRI		0x0400
>  #define TYPE_GUID		0x0401
> @@ -65,6 +65,9 @@ enum {
>  #define TYPE_BENIGN_SEC		0x0800
>  #define TYPE_ALL		0x0FFF
> 
> +#define TYPE_PRIMARY		(TYPE_CRITICAL_PRI | TYPE_BENIGN_PRI)
Where is this in use? If it is not used, Do not need to add it.
> +#define TYPE_SECONDARY		(TYPE_CRITICAL_SEC | TYPE_BENIGN_SEC)
> +
>  #define MAX_CHARSET_SIZE	6 /* max size of multi-byte character */
>  #define MAX_NAME_LENGTH		255 /* max len of file name excluding NULL */
>  #define MAX_VFSNAME_BUF_SIZE	((MAX_NAME_LENGTH + 1) * MAX_CHARSET_SIZE)

Re: [PATCH v4 1/5] exfat: integrates dir-entry getting and validation

[Tetsuhiro Kohada]

Thank you for your quick review.

On 2020/08/27 12:19, Namjae Jeon wrote:
>> +	i = ES_INDEX_NAME;
>> +	while ((ep = exfat_get_validated_dentry(es, i++, TYPE_NAME))) {
> Please find the way to access name entries like ep_file, ep_stream
> without calling exfat_get_validated_dentry().

Hmm, it's a hard order.
I can't separate length/type validation and extraction.
Sorry, I have no good idea.


>> @@ -590,17 +587,16 @@ int exfat_remove_entries(struct inode *inode, struct exfat_chain *p_dir,  void
>> exfat_update_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es)  {
>>   	int chksum_type = CS_DIR_ENTRY, i;
>> -	unsigned short chksum = 0;
>> +	u16 chksum = 0;
>>   	struct exfat_dentry *ep;
>>
>>   	for (i = 0; i < es->num_entries; i++) {
>> -		ep = exfat_get_dentry_cached(es, i);
>> +		ep = exfat_get_validated_dentry(es, i, TYPE_ALL);
> Ditto, You do not need to repeatedly call exfat_get_validated_dentry() for the entries
> which got from exfat_get_dentry_set().

Even if I could do that, it would be very difficult to implement a checksum patch.
It is also difficult to use for rename, move, delete.
(these also have no verification of neme-length and set-checksum)


>>   	/* validiate cached dentries */
>> -	for (i = 1; i < num_entries; i++) {
>> -		ep = exfat_get_dentry_cached(es, i);
>> -		if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode))
>> -			goto free_es;
>> +	es->ep_stream = exfat_get_validated_dentry(es, ES_INDEX_STREAM, TYPE_STREAM);
>> +	if (!es->ep_stream)
>> +		goto free_es;
>> +
>> +	if (max_entries == ES_ALL_ENTRIES) {
>> +		for (i = 0; i < ES_FILE(es).num_ext; i++)
>> +			if (!exfat_get_validated_dentry(es, ES_INDEX_STREAM + i, TYPE_SECONDARY))
>> +				goto free_es;
>> +		for (i = 0; i * EXFAT_FILE_NAME_LEN < ES_STREAM(es).name_len; i++)
>> +			if (!exfat_get_validated_dentry(es, ES_INDEX_NAME + i, TYPE_NAME))
>> +				goto free_es;
> Why do you unnecessarily check entries with two loops?
> Please refer to the patch I sent.

This order is possible.
However, TYPE_SECONDARY loop will be back as checksum loop.

In the next patch, I can fix the 'TYPE_SECONDARY loop' order.
do you need it?


BR
---
Tetsuhiro Kohada <kohada.t2@gmail.com>