[PATCH 00/18] fs: fat: fix long name support

[PATCH 00/18] fs: fat: fix long name support

[Heinrich Schuchardt]

Since long name support was added to U-Boot's implementation of the
FAT file system corruption of the directories was observed when running
the UEFI Self Certifification Test.

The following problems are addressed by this series.

* short names were not unique
* long names directory entries were not deleted when a file was deleted
* directory entries released by file deletion were not reused

The first four patches are just resent to indicate the import sequence.

The following problems need to be addressed in further patches:

* When extending a file only available FAT entries after the last cluster
  of the file are considered. Available FAT entries before are ignored.
* The case of no FAT entry found is not treated correctly.
* The free FAT count is not updated on FAT32.
* Some FAT related tests are not run on Gitlab CI.
* Add tests to detect directory corruption.

Heinrich Schuchardt (18):
  fs: fat: avoid NULL dereference when root dir is full
  fs: fat: directory entries starting with 0x05
  fs: fat: use ATTR_ARCH instead of anonymous 0x20
  fs: fat: correct first cluster for '..'
  fs: fat: export fat_next_cluster()
  fs: fat: create correct short names
  fs: fat: pass shortname to fill_dir_slot
  fs: fat: call set_name() only once
  fs: fat: generate unique short names
  fs: fat: dentry iterator for fill_dir_slot()
  fs: fat: set start cluster for root directory
  fs: fat: flush new directory cluster
  fs: fat: fat_find_empty_dentries()
  fs: fat: reuse deleted directory entries
  fs: fat: search file should not allocate cluster
  fs: fat: use constant DELETED_FLAG
  fs: fat: first dentry of long name in FAT iterator
  fs: fat: deletion of long file names

 fs/fat/fat.c       | 133 ++++++++----
 fs/fat/fat_write.c | 532 +++++++++++++++++++++++++++++++++------------
 include/fat.h      |   7 +-
 lib/Kconfig        |   2 +-
 4 files changed, 494 insertions(+), 180 deletions(-)

--
2.29.2

[PATCH 01/18] fs: fat: avoid NULL dereference when root dir is full

[Heinrich Schuchardt]

When trying to create a file in the full root directory of a FAT32
filesystem a NULL dereference can be observed.

When the root directory of a FAT16 filesystem is full fill_dir_slot() must
return -1 to signal that a new directory entry could not be allocated.

Fixes: cd2d727fff7e ("fs: fat: allocate a new cluster for root directory of fat32")
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index a2682b5f46..fc932df953 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -260,9 +260,8 @@ fill_dir_slot(fat_itr *itr, const char *l_name)
 			flush_dir(itr);

 		/* allocate a cluster for more entries */
-		if (!fat_itr_next(itr))
-			if (!itr->dent &&
-			    (!itr->is_root || itr->fsdata->fatsize == 32) &&
+		if (!fat_itr_next(itr) && !itr->dent)
+			if ((itr->is_root && itr->fsdata->fatsize != 32) ||
 			    new_dir_table(itr))
 				return -1;
 	}
--
2.29.2

[PATCH 02/18] fs: fat: directory entries starting with 0x05

[Heinrich Schuchardt]

0x05 is used as replacement letter for 0xe5 at the first position of short
file names. We must not skip over directory entries starting with 0x05.

Cf. Microsoft FAT Specification, August 30 2005

Fixes: 39606d462c97 ("fs: fat: handle deleted directory entries correctly")
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 fs/fat/fat.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 28aa5aaa9f..fb6ba89466 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -926,8 +926,7 @@ static int fat_itr_next(fat_itr *itr)
 		if (!dent)
 			return 0;

-		if (dent->name[0] == DELETED_FLAG ||
-		    dent->name[0] == aRING)
+		if (dent->name[0] == DELETED_FLAG)
 			continue;

 		if (dent->attr & ATTR_VOLUME) {
--
2.29.2

[PATCH 03/18] fs: fat: use ATTR_ARCH instead of anonymous 0x20

[Heinrich Schuchardt]

Using constants instead of anonymous numbers increases code readability.

Fixes: 704df6aa0a28 ("fs: fat: refactor write interface for a file offset")
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index fc932df953..7afc8388b2 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -1191,7 +1191,8 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
 		}

 		/* Set short name entry */
-		fill_dentry(itr->fsdata, itr->dent, filename, 0, size, 0x20);
+		fill_dentry(itr->fsdata, itr->dent, filename, 0, size,
+			    ATTR_ARCH);

 		retdent = itr->dent;
 	}
--
2.29.2

[PATCH 04/18] fs: fat: correct first cluster for '..'

[Heinrich Schuchardt]

The FAT specification [1] requires that for a '..' directory entry pointing
to the root directory the fields DIR_FstClusHi and DIR_FstClusLo are 0.

[1] Microsoft FAT Specification, Microsoft Corporation, August 30 2005

Fixes: 31a18d570d96 ("fs: fat: support mkdir")
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 fs/fat/fat_write.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 7afc8388b2..4da342f9c9 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -1468,7 +1468,11 @@ int fat_mkdir(const char *new_dirname)
 	memcpy(dotdent[1].name, "..      ", 8);
 	memcpy(dotdent[1].ext, "   ", 3);
 	dotdent[1].attr = ATTR_DIR | ATTR_ARCH;
-	set_start_cluster(mydata, &dotdent[1], itr->start_clust);
+
+	if (itr->is_root)
+		set_start_cluster(mydata, &dotdent[1], 0);
+	else
+		set_start_cluster(mydata, &dotdent[1], itr->start_clust);

 	ret = set_contents(mydata, retdent, 0, (__u8 *)dotdent,
 			   bytesperclust, &actwrite);
--
2.29.2

[PATCH 05/18] fs: fat: export fat_next_cluster()

[Heinrich Schuchardt]

Rename function next_cluster() to fat_next_cluster() and export it.

When creating a new directory entries we should reuse deleted entries.
This requires re-scanning the directory.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat.c  | 106 +++++++++++++++++++++++++++++++++++---------------
 include/fat.h |   7 +++-
 2 files changed, 80 insertions(+), 33 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index fb6ba89466..674236d68a 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -621,7 +621,7 @@ static int get_fs_info(fsdata *mydata)
 		/*
 		 * The root directory is not cluster-aligned and may be on a
 		 * "negative" cluster, this will be handled specially in
-		 * next_cluster().
+		 * fat_next_cluster().
 		 */
 		mydata->root_cluster = 0;
 	}
@@ -647,44 +647,76 @@ static int get_fs_info(fsdata *mydata)
 	return 0;
 }

-
-/*
- * Directory iterator, to simplify filesystem traversal
+/**
+ * struct fat_itr - directory iterator, to simplify filesystem traversal
  *
  * Implements an iterator pattern to traverse directory tables,
  * transparently handling directory tables split across multiple
  * clusters, and the difference between FAT12/FAT16 root directory
  * (contiguous) and subdirectories + FAT32 root (chained).
  *
- * Rough usage:
+ * Rough usage
  *
- *   for (fat_itr_root(&itr, fsdata); fat_itr_next(&itr); ) {
- *      // to traverse down to a subdirectory pointed to by
- *      // current iterator position:
- *      fat_itr_child(&itr, &itr);
- *   }
+ * .. code-block:: c
  *
- * For more complete example, see fat_itr_resolve()
+ *     for (fat_itr_root(&itr, fsdata); fat_itr_next(&itr); ) {
+ *         // to traverse down to a subdirectory pointed to by
+ *         // current iterator position:
+ *         fat_itr_child(&itr, &itr);
+ *     }
+ *
+ * For a more complete example, see fat_itr_resolve().
  */
-
-typedef struct {
-	fsdata    *fsdata;        /* filesystem parameters */
-	unsigned   start_clust;   /* first cluster */
-	unsigned   clust;         /* current cluster */
-	unsigned   next_clust;    /* next cluster if remaining == 0 */
-	int        last_cluster;  /* set once we've read last cluster */
-	int        is_root;       /* is iterator at root directory */
-	int        remaining;     /* remaining dent's in current cluster */
-
-	/* current iterator position values: */
-	dir_entry *dent;          /* current directory entry */
-	char       l_name[VFAT_MAXLEN_BYTES];    /* long (vfat) name */
-	char       s_name[14];    /* short 8.3 name */
-	char      *name;          /* l_name if there is one, else s_name */
-
-	/* storage for current cluster in memory: */
-	u8         block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
-} fat_itr;
+struct fat_itr {
+	/**
+	 * @fsdata:		filesystem parameters
+	 */
+	fsdata *fsdata;
+	/**
+	 * @start_clust:	first cluster
+	 */
+	unsigned int start_clust;
+	/**
+	 * @clust:		current cluster
+	 */
+	unsigned int clust;
+	/**
+	 * @next_clust:		next cluster if remaining == 0
+	 */
+	unsigned int next_clust;
+	/**
+	 * @last_cluster:	set if last cluster of directory reached
+	 */
+	int last_cluster;
+	/**
+	 * @is_root:		is iterator at root directory
+	 */
+	int is_root;
+	/**
+	 * @remaining:		remaining directory entries in current cluster
+	 */
+	int remaining;
+	/**
+	 * @dent:		current directory entry
+	 */
+	dir_entry *dent;
+	/**
+	 * @l_name:		long name of current directory entry
+	 */
+	char l_name[VFAT_MAXLEN_BYTES];
+	/**
+	 * @s_name:		short 8.3 name of current directory entry
+	 */
+	char s_name[14];
+	/**
+	 * @name:		l_name if there is one, else s_name
+	 */
+	char *name;
+	/**
+	 * @block:		buffer for current cluster
+	 */
+	u8 block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
+};

 static int fat_itr_isdir(fat_itr *itr);

@@ -753,7 +785,17 @@ static void fat_itr_child(fat_itr *itr, fat_itr *parent)
 	itr->last_cluster = 0;
 }

-static void *next_cluster(fat_itr *itr, unsigned *nbytes)
+/**
+ * fat_next_cluster() - load next FAT cluster
+ *
+ * The function is used when iterating through directories. It loads the
+ * next cluster with directory entries
+ *
+ * @itr:	directory iterator
+ * @nbytes:	number of bytes read, 0 on error
+ * Return:	first directory entry, NULL on error
+ */
+void *fat_next_cluster(fat_itr *itr, unsigned int *nbytes)
 {
 	fsdata *mydata = itr->fsdata;  /* for silly macros */
 	int ret;
@@ -825,7 +867,7 @@ static dir_entry *next_dent(fat_itr *itr)
 {
 	if (itr->remaining == 0) {
 		unsigned nbytes;
-		struct dir_entry *dent = next_cluster(itr, &nbytes);
+		struct dir_entry *dent = fat_next_cluster(itr, &nbytes);

 		/* have we reached the last cluster? */
 		if (!dent) {
diff --git a/include/fat.h b/include/fat.h
index 02742f92a5..3c29a4484d 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -9,8 +9,9 @@
 #ifndef _FAT_H_
 #define _FAT_H_

-#include <asm/byteorder.h>
 #include <fs.h>
+#include <asm/byteorder.h>
+#include <asm/cache.h>

 struct disk_partition;

@@ -179,6 +180,9 @@ typedef struct {
 	int	fats;		/* Number of FATs */
 } fsdata;

+struct fat_itr;
+typedef struct fat_itr fat_itr;
+
 static inline u32 clust_to_sect(fsdata *fsdata, u32 clust)
 {
 	return fsdata->data_begin + clust * fsdata->clust_size;
@@ -208,4 +212,5 @@ void fat_closedir(struct fs_dir_stream *dirs);
 int fat_unlink(const char *filename);
 int fat_mkdir(const char *dirname);
 void fat_close(void);
+void *fat_next_cluster(fat_itr *itr, unsigned int *nbytes);
 #endif /* _FAT_H_ */
--
2.29.2

[PATCH 06/18] fs: fat: create correct short names

[Heinrich Schuchardt]

The current function set_name() used to create short names has the
following deficiencies resolved by this patch:

* Long names (e.g. FOO.TXT) are stored even if a short name is enough.
* Short names with spaces are created, e.g. "A     ~1.TXT".
* Short names with illegal characters are created, e.g. "FOO++BAR".
* Debug output does not not consider that the short file name has no
  concluding '\0'.

The solution for the following bug is split of into a separate patch:

* Short file names must be unique.

This patch only provides the loop over possible short file names.

Fixes: c30a15e590c ("FAT: Add FAT write feature")
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 215 +++++++++++++++++++++++++++++----------------
 lib/Kconfig        |   2 +-
 2 files changed, 140 insertions(+), 77 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 4da342f9c9..058b566629 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -8,25 +8,140 @@
 #include <common.h>
 #include <command.h>
 #include <config.h>
+#include <div64.h>
 #include <fat.h>
 #include <log.h>
 #include <malloc.h>
-#include <asm/byteorder.h>
 #include <part.h>
+#include <rand.h>
+#include <asm/byteorder.h>
 #include <asm/cache.h>
 #include <linux/ctype.h>
-#include <div64.h>
 #include <linux/math64.h>
 #include "fat.c"

-static void uppercase(char *str, int len)
+/* Characters that may only be used in long file names */
+static const char LONG_ONLY_CHARS[] = "+,;=[]";
+
+/**
+ * str2fat() - convert string to valid FAT name characters
+ *
+ * Stop when reaching end of @src or a period.
+ * Ignore spaces.
+ * Replace characters that may only be used in long names by underscores.
+ * Convert lower case characters to upper case.
+ *
+ * To avoid assumptions about the code page we do not use characters
+ * above 0x7f for the short name.
+ *
+ * @dest:	destination buffer
+ * @src:	source buffer
+ * @length:	size of destination buffer
+ * Return:	number of bytes in destination buffer
+ */
+static int str2fat(char *dest, char *src, int length)
+{
+	int i;
+
+	for (i = 0; i < length; ++src) {
+		char c = *src;
+
+		if (!c || c == '.')
+			break;
+		if (c == ' ')
+			continue;
+		if (strchr(LONG_ONLY_CHARS, c) || c > 0x7f)
+			c = '_';
+		else if (c >= 'a' && c <= 'z')
+			c &= 0xdf;
+		dest[i] = c;
+		++i;
+	}
+	return i;
+}
+
+/**
+ * set_name() - set short name in directory entry
+ *
+ * The function determines if the @filename is a valid short name.
+ * In this case no long name is needed.
+ *
+ * If a long name is needed, a short name is constructed.
+ *
+ * @dirent:	directory entry
+ * @filename:	long file name
+ * Return:	number of directory entries needed, negative on error
+ */
+static int set_name(dir_entry *dirent, const char *filename)
 {
+	char *period;
+	char *pos;
+	int period_location;
+	char buf[13];
 	int i;

-	for (i = 0; i < len; i++) {
-		*str = toupper(*str);
-		str++;
+	if (!filename)
+		return -EIO;
+
+	/* Initialize buffers */
+	memset(dirent->name, ' ', sizeof(dirent->name));
+	memset(dirent->ext, ' ', sizeof(dirent->ext));
+
+	/* Convert filename to upper case short name */
+	period = strrchr(filename, '.');
+	pos = (char *)filename;
+	if (*pos == '.') {
+		pos = period + 1;
+		period = 0;
+	}
+	if (period)
+		str2fat(dirent->ext, period + 1, sizeof(dirent->ext));
+	period_location = str2fat(dirent->name, pos, sizeof(dirent->name));
+	if (period_location < 0)
+		return period_location;
+	if (*dirent->name == ' ')
+		*dirent->name = '_';
+	/* 0xe5 signals a deleted directory entry. Replace it by 0x05. */
+	if (*dirent->name == 0xe5)
+		*dirent->name = 0x05;
+
+	/* If filename and short name are the same, quit. */
+	sprintf(buf, "%.*s.%.3s", period_location, dirent->name, dirent->ext);
+	if (!strcmp(buf, filename))
+		return 1;
+
+	/* Construct an indexed short name */
+	for (i = 1; i < 0x200000; ++i) {
+		int suffix_len;
+		int suffix_start;
+		int j;
+
+		/* To speed up the search use random numbers */
+		if (i < 10) {
+			j = i;
+		} else {
+			j = 30 - fls(i);
+			j = 10 + (rand() >> j);
+		}
+		sprintf(buf, "~%d", j);
+		suffix_len = strlen(buf);
+		suffix_start = 8 - suffix_len;
+		if (suffix_start > period_location)
+			suffix_start = period_location;
+		memcpy(dirent->name + suffix_start, buf, suffix_len);
+		if (*dirent->ext != ' ')
+			sprintf(buf, "%.*s.%.3s", suffix_start + suffix_len,
+				dirent->name, dirent->ext);
+		else
+			sprintf(buf, "%.*s", suffix_start + suffix_len,
+				dirent->name);
+		debug("short name: %s\n", buf);
+		/* TODO: Check that the short name does not exist yet. */
+
+		/* Each long name directory entry takes 13 characters. */
+		return (strlen(filename) + 25) / 13;
 	}
+	return -EIO;
 }

 static int total_sector;
@@ -50,67 +165,6 @@ static int disk_write(__u32 block, __u32 nr_blocks, void *buf)
 	return ret;
 }

-/**
- * set_name() - set short name in directory entry
- *
- * @dirent:	directory entry
- * @filename:	long file name
- */
-static void set_name(dir_entry *dirent, const char *filename)
-{
-	char s_name[VFAT_MAXLEN_BYTES];
-	char *period;
-	int period_location, len, i, ext_num;
-
-	if (filename == NULL)
-		return;
-
-	len = strlen(filename);
-	if (len == 0)
-		return;
-
-	strncpy(s_name, filename, VFAT_MAXLEN_BYTES - 1);
-	s_name[VFAT_MAXLEN_BYTES - 1] = '\0';
-	uppercase(s_name, len);
-
-	period = strchr(s_name, '.');
-	if (period == NULL) {
-		period_location = len;
-		ext_num = 0;
-	} else {
-		period_location = period - s_name;
-		ext_num = len - period_location - 1;
-	}
-
-	/* Pad spaces when the length of file name is shorter than eight */
-	if (period_location < 8) {
-		memcpy(dirent->name, s_name, period_location);
-		for (i = period_location; i < 8; i++)
-			dirent->name[i] = ' ';
-	} else if (period_location == 8) {
-		memcpy(dirent->name, s_name, period_location);
-	} else {
-		memcpy(dirent->name, s_name, 6);
-		/*
-		 * TODO: Translating two long names with the same first six
-		 *       characters to the same short name is utterly wrong.
-		 *       Short names must be unique.
-		 */
-		dirent->name[6] = '~';
-		dirent->name[7] = '1';
-	}
-
-	if (ext_num < 3) {
-		memcpy(dirent->ext, s_name + period_location + 1, ext_num);
-		for (i = ext_num; i < 3; i++)
-			dirent->ext[i] = ' ';
-	} else
-		memcpy(dirent->ext, s_name + period_location + 1, 3);
-
-	debug("name : %s\n", dirent->name);
-	debug("ext : %s\n", dirent->ext);
-}
-
 /*
  * Write fat buffer into block device
  */
@@ -1181,13 +1235,15 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,

 		memset(itr->dent, 0, sizeof(*itr->dent));

-		/* Calculate checksum for short name */
-		set_name(itr->dent, filename);
-
-		/* Set long name entries */
-		if (fill_dir_slot(itr, filename)) {
-			ret = -EIO;
+		/* Check if long name is needed */
+		ret = set_name(itr->dent, filename);
+		if (ret < 0)
 			goto exit;
+		if (ret > 1) {
+			/* Set long name entries */
+			ret = fill_dir_slot(itr, filename);
+			if (ret)
+				goto exit;
 		}

 		/* Set short name entry */
@@ -1441,9 +1497,16 @@ int fat_mkdir(const char *new_dirname)

 		memset(itr->dent, 0, sizeof(*itr->dent));

-		/* Set short name to set alias checksum field in dir_slot */
-		set_name(itr->dent, dirname);
-		fill_dir_slot(itr, dirname);
+		/* Check if long name is needed */
+		ret = set_name(itr->dent, dirname);
+		if (ret < 0)
+			goto exit;
+		if (ret > 1) {
+			/* Set long name entries */
+			ret = fill_dir_slot(itr, dirname);
+			if (ret)
+				goto exit;
+		}

 		/* Set attribute as archive for regular file */
 		fill_dentry(itr->fsdata, itr->dent, dirname, 0, 0,
diff --git a/lib/Kconfig b/lib/Kconfig
index 7673d2e4e0..06eb8d07dc 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -168,7 +168,7 @@ config REGEX
 choice
 	prompt "Pseudo-random library support type"
 	depends on NET_RANDOM_ETHADDR || RANDOM_UUID || CMD_UUID || \
-		   RNG_SANDBOX || UT_LIB && AES
+		   RNG_SANDBOX || UT_LIB && AES || FAT_WRITE
 	default LIB_RAND
 	help
 	  Select the library to provide pseudo-random number generator
--
2.29.2

[PATCH 07/18] fs: fat: pass shortname to fill_dir_slot

[Heinrich Schuchardt]

Currently we pass the short name via the directory iterator.
Pass it explicitly as a parameter.

This removes the requirement to set the short name in the iterator before
writing the long name.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 058b566629..7e8886791c 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -278,12 +278,16 @@ name11_12:
 static int new_dir_table(fat_itr *itr);
 static int flush_dir(fat_itr *itr);

-/*
- * Fill dir_slot entries with appropriate name, id, and attr
- * 'itr' will point to a next entry
+/**
+ * fill_dir_slot() - fill directory entries for long name
+ *
+ * @itr:	directory iterator
+ * @l_name:	long name
+ * @shortname:	short name
+ * Return:	0 for success, -errno otherwise
  */
 static int
-fill_dir_slot(fat_itr *itr, const char *l_name)
+fill_dir_slot(fat_itr *itr, const char *l_name, const char *shortname)
 {
 	__u8 temp_dir_slot_buffer[MAX_LFN_SLOT * sizeof(dir_slot)];
 	dir_slot *slotptr = (dir_slot *)temp_dir_slot_buffer;
@@ -291,7 +295,7 @@ fill_dir_slot(fat_itr *itr, const char *l_name)
 	int idx = 0, ret;

 	/* Get short file name checksum value */
-	checksum = mkcksum(itr->dent->name, itr->dent->ext);
+	checksum = mkcksum(shortname, shortname + 8);

 	do {
 		memset(slotptr, 0x00, sizeof(dir_slot));
@@ -317,7 +321,7 @@ fill_dir_slot(fat_itr *itr, const char *l_name)
 		if (!fat_itr_next(itr) && !itr->dent)
 			if ((itr->is_root && itr->fsdata->fatsize != 32) ||
 			    new_dir_table(itr))
-				return -1;
+				return -EIO;
 	}

 	return 0;
@@ -1241,7 +1245,7 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
 			goto exit;
 		if (ret > 1) {
 			/* Set long name entries */
-			ret = fill_dir_slot(itr, filename);
+			ret = fill_dir_slot(itr, filename, itr->dent->name);
 			if (ret)
 				goto exit;
 		}
@@ -1503,7 +1507,7 @@ int fat_mkdir(const char *new_dirname)
 			goto exit;
 		if (ret > 1) {
 			/* Set long name entries */
-			ret = fill_dir_slot(itr, dirname);
+			ret = fill_dir_slot(itr, dirname, itr->dent->name);
 			if (ret)
 				goto exit;
 		}
--
2.29.2

[PATCH 08/18] fs: fat: call set_name() only once

[Heinrich Schuchardt]

In set_name() we select the short name. Once this is correctly implemented
this will be a performance intensive operation because we need to check
that the name does not exist yet. So set_name should only be called once.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 87 ++++++++++++++++++++++++++++------------------
 1 file changed, 54 insertions(+), 33 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 7e8886791c..716b6a6627 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -23,6 +23,9 @@
 /* Characters that may only be used in long file names */
 static const char LONG_ONLY_CHARS[] = "+,;=[]";

+/* Combined size of the name and ext fields in the directory entry */
+#define SHORT_NAME_SIZE 11
+
 /**
  * str2fat() - convert string to valid FAT name characters
  *
@@ -68,24 +71,28 @@ static int str2fat(char *dest, char *src, int length)
  *
  * If a long name is needed, a short name is constructed.
  *
- * @dirent:	directory entry
  * @filename:	long file name
+ * @shortname:	buffer of 11 bytes to receive chosen short name and extension
  * Return:	number of directory entries needed, negative on error
  */
-static int set_name(dir_entry *dirent, const char *filename)
+static int set_name(const char *filename, char *shortname)
 {
 	char *period;
 	char *pos;
 	int period_location;
 	char buf[13];
 	int i;
+	int ret;
+	struct {
+		char name[8];
+		char ext[3];
+	} dirent;

 	if (!filename)
 		return -EIO;

-	/* Initialize buffers */
-	memset(dirent->name, ' ', sizeof(dirent->name));
-	memset(dirent->ext, ' ', sizeof(dirent->ext));
+	/* Initialize buffer */
+	memset(&dirent, ' ', sizeof(dirent));

 	/* Convert filename to upper case short name */
 	period = strrchr(filename, '.');
@@ -95,20 +102,22 @@ static int set_name(dir_entry *dirent, const char *filename)
 		period = 0;
 	}
 	if (period)
-		str2fat(dirent->ext, period + 1, sizeof(dirent->ext));
-	period_location = str2fat(dirent->name, pos, sizeof(dirent->name));
+		str2fat(dirent.ext, period + 1, sizeof(dirent.ext));
+	period_location = str2fat(dirent.name, pos, sizeof(dirent.name));
 	if (period_location < 0)
 		return period_location;
-	if (*dirent->name == ' ')
-		*dirent->name = '_';
+	if (*dirent.name == ' ')
+		*dirent.name = '_';
 	/* 0xe5 signals a deleted directory entry. Replace it by 0x05. */
-	if (*dirent->name == 0xe5)
-		*dirent->name = 0x05;
+	if (*dirent.name == 0xe5)
+		*dirent.name = 0x05;

 	/* If filename and short name are the same, quit. */
-	sprintf(buf, "%.*s.%.3s", period_location, dirent->name, dirent->ext);
-	if (!strcmp(buf, filename))
-		return 1;
+	sprintf(buf, "%.*s.%.3s", period_location, dirent.name, dirent.ext);
+	if (!strcmp(buf, filename)) {
+		ret = 1;
+		goto out;
+	}

 	/* Construct an indexed short name */
 	for (i = 1; i < 0x200000; ++i) {
@@ -128,20 +137,24 @@ static int set_name(dir_entry *dirent, const char *filename)
 		suffix_start = 8 - suffix_len;
 		if (suffix_start > period_location)
 			suffix_start = period_location;
-		memcpy(dirent->name + suffix_start, buf, suffix_len);
-		if (*dirent->ext != ' ')
+		memcpy(dirent.name + suffix_start, buf, suffix_len);
+		if (*dirent.ext != ' ')
 			sprintf(buf, "%.*s.%.3s", suffix_start + suffix_len,
-				dirent->name, dirent->ext);
+				dirent.name, dirent.ext);
 		else
 			sprintf(buf, "%.*s", suffix_start + suffix_len,
-				dirent->name);
+				dirent.name);
 		debug("short name: %s\n", buf);
 		/* TODO: Check that the short name does not exist yet. */

 		/* Each long name directory entry takes 13 characters. */
-		return (strlen(filename) + 25) / 13;
+		ret = (strlen(filename) + 25) / 13;
+		goto out;
 	}
 	return -EIO;
+out:
+	memcpy(shortname, dirent.name, SHORT_NAME_SIZE);
+	return ret;
 }

 static int total_sector;
@@ -1019,18 +1032,27 @@ getit:
 	return 0;
 }

-/*
- * Fill dir_entry
+/**
+ * fill_dentry() - fill directory entry with shortname
+ *
+ * @mydata:		private filesystem parameters
+ * @dentptr:		directory entry
+ * @shortname:		chosen short name
+ * @start_cluster:	first cluster of file
+ * @size:		file size
+ * @attr:		file attributes
  */
 static void fill_dentry(fsdata *mydata, dir_entry *dentptr,
-	const char *filename, __u32 start_cluster, __u32 size, __u8 attr)
+	const char *shortname, __u32 start_cluster, __u32 size, __u8 attr)
 {
+	memset(dentptr, 0, sizeof(*dentptr));
+
 	set_start_cluster(mydata, dentptr, start_cluster);
 	dentptr->size = cpu_to_le32(size);

 	dentptr->attr = attr;

-	set_name(dentptr, filename);
+	memcpy(dentptr->name, shortname, SHORT_NAME_SIZE);
 }

 /*
@@ -1215,6 +1237,7 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
 		retdent->size = cpu_to_le32(pos + size);
 	} else {
 		/* Create a new file */
+		char shortname[SHORT_NAME_SIZE];

 		if (itr->is_root) {
 			/* root dir cannot have "." or ".." */
@@ -1237,21 +1260,19 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
 			goto exit;
 		}

-		memset(itr->dent, 0, sizeof(*itr->dent));
-
 		/* Check if long name is needed */
-		ret = set_name(itr->dent, filename);
+		ret = set_name(filename, shortname);
 		if (ret < 0)
 			goto exit;
 		if (ret > 1) {
 			/* Set long name entries */
-			ret = fill_dir_slot(itr, filename, itr->dent->name);
+			ret = fill_dir_slot(itr, filename, shortname);
 			if (ret)
 				goto exit;
 		}

 		/* Set short name entry */
-		fill_dentry(itr->fsdata, itr->dent, filename, 0, size,
+		fill_dentry(itr->fsdata, itr->dent, shortname, 0, size,
 			    ATTR_ARCH);

 		retdent = itr->dent;
@@ -1484,6 +1505,8 @@ int fat_mkdir(const char *new_dirname)
 		ret = -EEXIST;
 		goto exit;
 	} else {
+		char shortname[SHORT_NAME_SIZE];
+
 		if (itr->is_root) {
 			/* root dir cannot have "." or ".." */
 			if (!strcmp(l_dirname, ".") ||
@@ -1499,21 +1522,19 @@ int fat_mkdir(const char *new_dirname)
 			goto exit;
 		}

-		memset(itr->dent, 0, sizeof(*itr->dent));
-
 		/* Check if long name is needed */
-		ret = set_name(itr->dent, dirname);
+		ret = set_name(dirname, shortname);
 		if (ret < 0)
 			goto exit;
 		if (ret > 1) {
 			/* Set long name entries */
-			ret = fill_dir_slot(itr, dirname, itr->dent->name);
+			ret = fill_dir_slot(itr, dirname, shortname);
 			if (ret)
 				goto exit;
 		}

 		/* Set attribute as archive for regular file */
-		fill_dentry(itr->fsdata, itr->dent, dirname, 0, 0,
+		fill_dentry(itr->fsdata, itr->dent, shortname, 0, 0,
 			    ATTR_DIR | ATTR_ARCH);

 		retdent = itr->dent;
--
2.29.2

[PATCH 09/18] fs: fat: generate unique short names

[Heinrich Schuchardt]

File names must be unique within their directory. So before assigning a
short name we must check that it is unique.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 41 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 716b6a6627..59cffef34e 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -20,6 +20,8 @@
 #include <linux/math64.h>
 #include "fat.c"

+static dir_entry *find_directory_entry(fat_itr *itr, char *filename);
+
 /* Characters that may only be used in long file names */
 static const char LONG_ONLY_CHARS[] = "+,;=[]";

@@ -63,6 +65,27 @@ static int str2fat(char *dest, char *src, int length)
 	return i;
 }

+/**
+ * fat_move_to_cluster() - position to first directory entry in cluster
+ *
+ * @itr:	directory iterator
+ * @cluster	cluster
+ * Return:	0 for success, -EIO on error
+ */
+static int fat_move_to_cluster(fat_itr *itr, unsigned int cluster)
+{
+	unsigned int nbytes;
+
+	/* position to the start of the directory */
+	itr->next_clust = cluster;
+	itr->last_cluster = 0;
+	if (!fat_next_cluster(itr, &nbytes))
+		return -EIO;
+	itr->dent = (dir_entry *)itr->block;
+	itr->remaining = nbytes / sizeof(dir_entry) - 1;
+	return 0;
+}
+
 /**
  * set_name() - set short name in directory entry
  *
@@ -71,11 +94,12 @@ static int str2fat(char *dest, char *src, int length)
  *
  * If a long name is needed, a short name is constructed.
  *
+ * @itr:	directory iterator
  * @filename:	long file name
  * @shortname:	buffer of 11 bytes to receive chosen short name and extension
  * Return:	number of directory entries needed, negative on error
  */
-static int set_name(const char *filename, char *shortname)
+static int set_name(fat_itr *itr, const char *filename, char *shortname)
 {
 	char *period;
 	char *pos;
@@ -144,9 +168,16 @@ static int set_name(const char *filename, char *shortname)
 		else
 			sprintf(buf, "%.*s", suffix_start + suffix_len,
 				dirent.name);
-		debug("short name: %s\n", buf);
-		/* TODO: Check that the short name does not exist yet. */
+		debug("generated short name: %s\n", buf);
+
+		/* Check that the short name does not exist yet. */
+		ret = fat_move_to_cluster(itr, itr->start_clust);
+		if (ret)
+			return ret;
+		if (find_directory_entry(itr, buf))
+			continue;

+		debug("chosen short name: %s\n", buf);
 		/* Each long name directory entry takes 13 characters. */
 		ret = (strlen(filename) + 25) / 13;
 		goto out;
@@ -1261,7 +1292,7 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
 		}

 		/* Check if long name is needed */
-		ret = set_name(filename, shortname);
+		ret = set_name(itr, filename, shortname);
 		if (ret < 0)
 			goto exit;
 		if (ret > 1) {
@@ -1523,7 +1554,7 @@ int fat_mkdir(const char *new_dirname)
 		}

 		/* Check if long name is needed */
-		ret = set_name(dirname, shortname);
+		ret = set_name(itr, dirname, shortname);
 		if (ret < 0)
 			goto exit;
 		if (ret > 1) {
--
2.29.2

[PATCH 10/18] fs: fat: dentry iterator for fill_dir_slot()

[Heinrich Schuchardt]

For reusing deleted directory entries we have to adjust the function called
to step to the next directory entry.

This patch alone is not enough to actually reuse deleted directory entries
as the fill_dir_slot() is still called with first never used directory
entry.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 59cffef34e..0746d73f8d 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -362,7 +362,7 @@ fill_dir_slot(fat_itr *itr, const char *l_name, const char *shortname)
 			flush_dir(itr);

 		/* allocate a cluster for more entries */
-		if (!fat_itr_next(itr) && !itr->dent)
+		if (!next_dent(itr) && !itr->dent)
 			if ((itr->is_root && itr->fsdata->fatsize != 32) ||
 			    new_dir_table(itr))
 				return -EIO;
--
2.29.2

[PATCH 11/18] fs: fat: set start cluster for root directory

[Heinrich Schuchardt]

When iterating over a child directory we set itr->start_clust.
Do the same when over the root directory.

When looking for deleted directory entries or existing short names we will
have to iterate over directories a second and third time. With this patch
we do not need any special logic for the root directory.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 674236d68a..5a418cfbb7 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -734,7 +734,7 @@ static int fat_itr_root(fat_itr *itr, fsdata *fsdata)
 		return -ENXIO;

 	itr->fsdata = fsdata;
-	itr->start_clust = 0;
+	itr->start_clust = fsdata->root_cluster;
 	itr->clust = fsdata->root_cluster;
 	itr->next_clust = fsdata->root_cluster;
 	itr->dent = NULL;
@@ -778,6 +778,7 @@ static void fat_itr_child(fat_itr *itr, fat_itr *parent)
 	} else {
 		itr->clust = parent->fsdata->root_cluster;
 		itr->next_clust = parent->fsdata->root_cluster;
+		itr->start_clust = parent->fsdata->root_cluster;
 		itr->is_root = 1;
 	}
 	itr->dent = NULL;
@@ -1067,6 +1068,7 @@ static int fat_itr_resolve(fat_itr *itr, const char *path, unsigned type)
 			/* point back to itself */
 			itr->clust = itr->fsdata->root_cluster;
 			itr->next_clust = itr->fsdata->root_cluster;
+			itr->start_clust = itr->fsdata->root_cluster;
 			itr->dent = NULL;
 			itr->remaining = 0;
 			itr->last_cluster = 0;
--
2.29.2

[PATCH 12/18] fs: fat: flush new directory cluster

[Heinrich Schuchardt]

When handling long file names directory entries may be split over multiple
clusters. We must make sure that new clusters are zero filled on disk.

When allocating a new cluster for a directory flush it.

The flushing should be executed before updating the FAT. This way if
flushing fails, we still have a valid directory structure.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 0746d73f8d..941f8789ab 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -738,17 +738,32 @@ static int find_empty_cluster(fsdata *mydata)
 	return entry;
 }

-/*
- * Allocate a cluster for additional directory entries
+/**
+ * new_dir_table() - allocate a cluster for additional directory entries
+ *
+ * @itr:	directory iterator
+ * Return:	0 on success, -EIO otherwise
  */
 static int new_dir_table(fat_itr *itr)
 {
 	fsdata *mydata = itr->fsdata;
 	int dir_newclust = 0;
+	int dir_oldclust = itr->clust;
 	unsigned int bytesperclust = mydata->clust_size * mydata->sect_size;

 	dir_newclust = find_empty_cluster(mydata);
-	set_fatent_value(mydata, itr->clust, dir_newclust);
+
+	/*
+	 * Flush before updating FAT to ensure valid directory structure
+	 * in case of failure.
+	 */
+	itr->clust = dir_newclust;
+	itr->next_clust = dir_newclust;
+	memset(itr->block, 0x00, bytesperclust);
+	if (flush_dir(itr))
+		return -EIO;
+
+	set_fatent_value(mydata, dir_oldclust, dir_newclust);
 	if (mydata->fatsize == 32)
 		set_fatent_value(mydata, dir_newclust, 0xffffff8);
 	else if (mydata->fatsize == 16)
@@ -756,13 +771,8 @@ static int new_dir_table(fat_itr *itr)
 	else if (mydata->fatsize == 12)
 		set_fatent_value(mydata, dir_newclust, 0xff8);

-	itr->clust = dir_newclust;
-	itr->next_clust = dir_newclust;
-
 	if (flush_dirty_fat_buffer(mydata) < 0)
-		return -1;
-
-	memset(itr->block, 0x00, bytesperclust);
+		return -EIO;

 	itr->dent = (dir_entry *)itr->block;
 	itr->last_cluster = 1;
--
2.29.2

[PATCH 13/18] fs: fat: fat_find_empty_dentries()

[Heinrich Schuchardt]

Provide a function to find a series of empty directory entries.

The current directory is scanned for deleted entries.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 62 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 941f8789ab..d560b94b60 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -21,6 +21,7 @@
 #include "fat.c"

 static dir_entry *find_directory_entry(fat_itr *itr, char *filename);
+static int new_dir_table(fat_itr *itr);

 /* Characters that may only be used in long file names */
 static const char LONG_ONLY_CHARS[] = "+,;=[]";
@@ -250,6 +251,66 @@ static int flush_dirty_fat_buffer(fsdata *mydata)
 	return 0;
 }

+/**
+ * fat_find_empty_dentries() - find a sequence of available directory entries
+ *
+ * @itr:	directory iterator
+ * @count:	number of directory entries to find
+ * Return:	0 on success or negative error number
+ */
+static int __maybe_unused fat_find_empty_dentries(fat_itr *itr, int count)
+{
+	unsigned int cluster;
+	dir_entry *dent;
+	int remaining;
+	unsigned int n = 0;
+	int ret;
+
+	ret = fat_move_to_cluster(itr, itr->start_clust);
+	if (ret)
+		return ret;
+
+	for (;;) {
+		if (!itr->dent) {
+			log_debug("Not enough directory entries available\n");
+			return -ENOSPC;
+		}
+		switch (itr->dent->name[0]) {
+		case 0x00:
+		case DELETED_FLAG:
+			if (!n) {
+				/* Remember first deleted directory entry */
+				cluster = itr->clust;
+				dent = itr->dent;
+				remaining = itr->remaining;
+			}
+			++n;
+			if (n == count)
+				goto out;
+			break;
+		default:
+			n = 0;
+			break;
+		}
+
+		next_dent(itr);
+		if (!itr->dent &&
+		    (!itr->is_root || itr->fsdata->fatsize == 32) &&
+		    new_dir_table(itr))
+			return -ENOSPC;
+	}
+out:
+	/* Position back to first directory entry */
+	if (itr->clust != cluster) {
+		ret = fat_move_to_cluster(itr, cluster);
+		if (ret)
+			return ret;
+	}
+	itr->dent = dent;
+	itr->remaining = remaining;
+	return 0;
+}
+
 /*
  * Set the file name information from 'name' into 'slotptr',
  */
@@ -319,7 +380,6 @@ name11_12:
 	return 1;
 }

-static int new_dir_table(fat_itr *itr);
 static int flush_dir(fat_itr *itr);

 /**
--
2.29.2

[PATCH 14/18] fs: fat: reuse deleted directory entries

[Heinrich Schuchardt]

When creating new directory entries try to reuse entries marked as deleted.

In fill_dir_slot() do not allocate new clusters as this has already been
done in fat_find_empty_dentries().

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index d560b94b60..a029ef8ed6 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -258,7 +258,7 @@ static int flush_dirty_fat_buffer(fsdata *mydata)
  * @count:	number of directory entries to find
  * Return:	0 on success or negative error number
  */
-static int __maybe_unused fat_find_empty_dentries(fat_itr *itr, int count)
+static int fat_find_empty_dentries(fat_itr *itr, int count)
 {
 	unsigned int cluster;
 	dir_entry *dent;
@@ -421,11 +421,9 @@ fill_dir_slot(fat_itr *itr, const char *l_name, const char *shortname)
 		if (itr->remaining == 0)
 			flush_dir(itr);

-		/* allocate a cluster for more entries */
-		if (!next_dent(itr) && !itr->dent)
-			if ((itr->is_root && itr->fsdata->fatsize != 32) ||
-			    new_dir_table(itr))
-				return -EIO;
+		next_dent(itr);
+		if (!itr->dent)
+			return -EIO;
 	}

 	return 0;
@@ -1339,6 +1337,7 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
 	} else {
 		/* Create a new file */
 		char shortname[SHORT_NAME_SIZE];
+		int ndent;

 		if (itr->is_root) {
 			/* root dir cannot have "." or ".." */
@@ -1362,10 +1361,15 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
 		}

 		/* Check if long name is needed */
-		ret = set_name(itr, filename, shortname);
-		if (ret < 0)
+		ndent = set_name(itr, filename, shortname);
+		if (ndent < 0) {
+			ret = ndent;
+			goto exit;
+		}
+		ret = fat_find_empty_dentries(itr, ndent);
+		if (ret)
 			goto exit;
-		if (ret > 1) {
+		if (ndent > 1) {
 			/* Set long name entries */
 			ret = fill_dir_slot(itr, filename, shortname);
 			if (ret)
@@ -1607,6 +1611,7 @@ int fat_mkdir(const char *new_dirname)
 		goto exit;
 	} else {
 		char shortname[SHORT_NAME_SIZE];
+		int ndent;

 		if (itr->is_root) {
 			/* root dir cannot have "." or ".." */
@@ -1624,10 +1629,15 @@ int fat_mkdir(const char *new_dirname)
 		}

 		/* Check if long name is needed */
-		ret = set_name(itr, dirname, shortname);
-		if (ret < 0)
+		ndent = set_name(itr, dirname, shortname);
+		if (ndent < 0) {
+			ret = ndent;
+			goto exit;
+		}
+		ret = fat_find_empty_dentries(itr, ndent);
+		if (ret)
 			goto exit;
-		if (ret > 1) {
+		if (ndent > 1) {
 			/* Set long name entries */
 			ret = fill_dir_slot(itr, dirname, shortname);
 			if (ret)
--
2.29.2

[PATCH 15/18] fs: fat: search file should not allocate cluster

[Heinrich Schuchardt]

Searching for a file is not a write operation. So it should not lead to the
allocation of a new cluster to the directory.

If we reuse deleted entries, we might not even use the new cluster and due
to not flushing it the directory could be corrupted.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 29 ++++++-----------------------
 1 file changed, 6 insertions(+), 23 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index a029ef8ed6..56ea28553b 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -1154,10 +1154,12 @@ static void fill_dentry(fsdata *mydata, dir_entry *dentptr,
 	memcpy(dentptr->name, shortname, SHORT_NAME_SIZE);
 }

-/*
- * Find a directory entry based on filename or start cluster number
- * If the directory entry is not found,
- * the new position for writing a directory entry will be returned
+/**
+ * find_directory_entry() - find a directory entry by filename
+ *
+ * @itr:	directory iterator
+ * @filename:	name of file to find
+ * Return:	directory entry or NULL
  */
 static dir_entry *find_directory_entry(fat_itr *itr, char *filename)
 {
@@ -1180,13 +1182,6 @@ static dir_entry *find_directory_entry(fat_itr *itr, char *filename)
 			return itr->dent;
 	}

-	/* allocate a cluster for more entries */
-	if (!itr->dent &&
-	    (!itr->is_root || itr->fsdata->fatsize == 32) &&
-	    new_dir_table(itr))
-		/* indicate that allocating dent failed */
-		itr->dent = NULL;
-
 	return NULL;
 }

@@ -1348,12 +1343,6 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
 			}
 		}

-		if (!itr->dent) {
-			printf("Error: allocating new dir entry\n");
-			ret = -EIO;
-			goto exit;
-		}
-
 		if (pos) {
 			/* No hole allowed */
 			ret = -EINVAL;
@@ -1622,12 +1611,6 @@ int fat_mkdir(const char *new_dirname)
 			}
 		}

-		if (!itr->dent) {
-			printf("Error: allocating new dir entry\n");
-			ret = -EIO;
-			goto exit;
-		}
-
 		/* Check if long name is needed */
 		ndent = set_name(itr, dirname, shortname);
 		if (ndent < 0) {
--
2.29.2

[PATCH 16/18] fs: fat: use constant DELETED_FLAG

[Heinrich Schuchardt]

When deleting a directory entry 0xe5 is written to name[0].

We have a constant for this value and should use it consistently.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 56ea28553b..c403d7d5c6 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -1464,7 +1464,7 @@ static int delete_dentry(fat_itr *itr)
 	 *  - find and mark the "new" first invalid entry as name[0]=0x00
 	 */
 	memset(dentptr, 0, sizeof(*dentptr));
-	dentptr->name[0] = 0xe5;
+	dentptr->name[0] = DELETED_FLAG;

 	if (flush_dir(itr)) {
 		printf("error: writing directory entry\n");
--
2.29.2

[PATCH 17/18] fs: fat: first dentry of long name in FAT iterator

[Heinrich Schuchardt]

A long name is split over multiple directory entries. When deleting a file
with a long name we need the first directory entry to be able to delete the
whole chain.

Add the necessary fields to the FAT iterator:

* cluster of first directory entry
* address of first directory entry
* remaining entries in cluster

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 5a418cfbb7..47344bb57e 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -700,6 +700,18 @@ struct fat_itr {
 	 * @dent:		current directory entry
 	 */
 	dir_entry *dent;
+	/**
+	 * @dent_rem:		remaining entries after long name start
+	 */
+	int dent_rem;
+	/**
+	 * @dent_clust:		cluster of long name start
+	 */
+	unsigned int dent_clust;
+	/**
+	 * @dent_start:		first directory entry for long name
+	 */
+	dir_entry *dent_start;
 	/**
 	 * @l_name:		long name of current directory entry
 	 */
@@ -966,9 +978,13 @@ static int fat_itr_next(fat_itr *itr)

 	while (1) {
 		dent = next_dent(itr);
-		if (!dent)
+		if (!dent) {
+			itr->dent_start = NULL;
 			return 0;
-
+		}
+		itr->dent_rem = itr->remaining;
+		itr->dent_start = itr->dent;
+		itr->dent_clust = itr->clust;
 		if (dent->name[0] == DELETED_FLAG)
 			continue;

--
2.29.2

[PATCH 18/18] fs: fat: deletion of long file names

[Heinrich Schuchardt]

Long file names are stored in multiple directory entries. When deleting a
file we must delete all of them.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 88 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 76 insertions(+), 12 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index c403d7d5c6..20a54a2418 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -1445,27 +1445,91 @@ exit:
 	return count;
 }

-static int delete_dentry(fat_itr *itr)
+/**
+ * delete_single_dentry() - delete a single directory entry
+ *
+ * @itr:	directory iterator
+ * Return:	0 for success
+ */
+static int delete_single_dentry(fat_itr *itr)
+{
+	struct dir_entry *dent = itr->dent;
+
+	memset(dent, 0, sizeof(*dent));
+	dent->name[0] = DELETED_FLAG;
+
+	if (!itr->remaining) {
+		if (flush_dir(itr)) {
+			printf("error: writing directory entry\n");
+			return -EIO;
+		}
+	}
+	return 0;
+}
+
+/**
+ * delete_long_name() - delete long name directory entries
+ *
+ * @itr:	directory iterator
+ * Return:	0 for success
+ */
+static int delete_long_name(fat_itr *itr)
+{
+	struct dir_entry *dent = itr->dent;
+	int seqn = itr->dent->name[0] & ~LAST_LONG_ENTRY_MASK;
+
+	while (seqn--) {
+		int ret;
+
+		ret = delete_single_dentry(itr);
+		if (ret)
+			return ret;
+		dent = next_dent(itr);
+		if (!dent)
+			return -EIO;
+	}
+	return 0;
+}
+
+/**
+ * delete_dentry_long() - remove directory entry
+ *
+ * @itr:	directory iterator
+ * Return:	0 for success
+ */
+static int delete_dentry_long(fat_itr *itr)
 {
 	fsdata *mydata = itr->fsdata;
-	dir_entry *dentptr = itr->dent;
+	dir_entry *dent = itr->dent;

 	/* free cluster blocks */
-	clear_fatent(mydata, START(dentptr));
+	clear_fatent(mydata, START(dent));
 	if (flush_dirty_fat_buffer(mydata) < 0) {
 		printf("Error: flush fat buffer\n");
 		return -EIO;
 	}
+	/* Position to first directory entry for long name */
+	if (itr->clust != itr->dent_clust) {
+		int ret;

-	/*
-	 * update a directory entry
-	 * TODO:
-	 *  - long file name support
-	 *  - find and mark the "new" first invalid entry as name[0]=0x00
-	 */
-	memset(dentptr, 0, sizeof(*dentptr));
-	dentptr->name[0] = DELETED_FLAG;
+		ret = fat_move_to_cluster(itr, itr->dent_clust);
+		if (ret)
+			return ret;
+	}
+	itr->dent = itr->dent_start;
+	itr->remaining = itr->dent_rem;
+	dent = itr->dent_start;
+	/* Delete long name */
+	if ((dent->attr & ATTR_VFAT) == ATTR_VFAT &&
+	    (dent->name[0] & LAST_LONG_ENTRY_MASK)) {
+		int ret;

+		ret = delete_long_name(itr);
+		if (ret)
+			return ret;
+	}
+	/* Delete short name */
+	delete_single_dentry(itr);
 	if (flush_dir(itr)) {
 		printf("error: writing directory entry\n");
 		return -EIO;
@@ -1535,7 +1599,7 @@ int fat_unlink(const char *filename)
 		}
 	}

-	ret = delete_dentry(itr);
+	ret = delete_dentry_long(itr);

 exit:
 	free(fsdata.fatbuf);
--
2.29.2

[PATCH 00/18] fs: fat: fix long name support

[Marek Szyprowski]

Hi

On 29.11.2020 03:01, Heinrich Schuchardt wrote:
> Since long name support was added to U-Boot's implementation of the
> FAT file system corruption of the directories was observed when running
> the UEFI Self Certifification Test.
>
> The following problems are addressed by this series.
>
> * short names were not unique
> * long names directory entries were not deleted when a file was deleted
> * directory entries released by file deletion were not reused
>
> The first four patches are just resent to indicate the import sequence.

I've noticed some of those FAT issues over a year ago, but I never had 
enough time to start fixing it. Thanks for doing it! Feel free to add:

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

> The following problems need to be addressed in further patches:
>
> * When extending a file only available FAT entries after the last cluster
>    of the file are considered. Available FAT entries before are ignored.
> * The case of no FAT entry found is not treated correctly.
> * The free FAT count is not updated on FAT32.
> * Some FAT related tests are not run on Gitlab CI.
> * Add tests to detect directory corruption.
>
> Heinrich Schuchardt (18):
>    fs: fat: avoid NULL dereference when root dir is full
>    fs: fat: directory entries starting with 0x05
>    fs: fat: use ATTR_ARCH instead of anonymous 0x20
>    fs: fat: correct first cluster for '..'
>    fs: fat: export fat_next_cluster()
>    fs: fat: create correct short names
>    fs: fat: pass shortname to fill_dir_slot
>    fs: fat: call set_name() only once
>    fs: fat: generate unique short names
>    fs: fat: dentry iterator for fill_dir_slot()
>    fs: fat: set start cluster for root directory
>    fs: fat: flush new directory cluster
>    fs: fat: fat_find_empty_dentries()
>    fs: fat: reuse deleted directory entries
>    fs: fat: search file should not allocate cluster
>    fs: fat: use constant DELETED_FLAG
>    fs: fat: first dentry of long name in FAT iterator
>    fs: fat: deletion of long file names
>
>   fs/fat/fat.c       | 133 ++++++++----
>   fs/fat/fat_write.c | 532 +++++++++++++++++++++++++++++++++------------
>   include/fat.h      |   7 +-
>   lib/Kconfig        |   2 +-
>   4 files changed, 494 insertions(+), 180 deletions(-)
>
> --
> 2.29.2
>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland