[PATCH v2 0/3] fs: fat: avoid invalid filenames
The FAT32 File System Specification requires leading and trailing spaces as well as trailing periods of long names to be ignored.
Up to now U-Boot could create files with invalid file names violating the restriction above.
With the patches this is avoided.
v2: Change the separation of duties between split_filename() and normalize_longname(): split_filename() may return '.' and '..'.
Heinrich Schuchardt (3): fs: fat: usage basename in file_fat_write_at, fat_mkdir fs: fat: must not write directory '.' and '..' fs: fat: remove trailing periods from long name
fs/fat/fat_write.c | 70 +++++++++++++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 26 deletions(-)
-- 2.30.0
This patch involves no functional change. It is just about code readability.
Both in file_fat_write_at() and fat_mkdir() the incoming file or directory path are split into two parts: the parent directory and the base name.
In file_fat_write_at() the value of the variable basename is assigned to the filename parameter and afterwards the variable filename is used instead of basename. It is more readable to use the variable basename and leave filename unchanged.
In fat_mkdir() the base name variable is called directory. This is confusing. Call it basename like in file_fat_write_at(). This allows to rename parameter new_directory to directory in the implementation of fat_mkdir() to match the function declaration.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Simon Glass sjg@chromium.org --- v2: no change --- fs/fat/fat_write.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index b43a27b205..a9b9fa5d68 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -1299,9 +1299,8 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer, goto exit; }
- filename = basename; - if (normalize_longname(l_filename, filename)) { - printf("FAT: illegal filename (%s)\n", filename); + if (normalize_longname(l_filename, basename)) { + printf("FAT: illegal filename (%s)\n", basename); ret = -EINVAL; goto exit; } @@ -1365,7 +1364,7 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer, }
/* Check if long name is needed */ - ndent = set_name(itr, filename, shortname); + ndent = set_name(itr, basename, shortname); if (ndent < 0) { ret = ndent; goto exit; @@ -1375,7 +1374,7 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer, goto exit; if (ndent > 1) { /* Set long name entries */ - ret = fill_dir_slot(itr, filename, shortname); + ret = fill_dir_slot(itr, basename, shortname); if (ret) goto exit; } @@ -1611,31 +1610,31 @@ exit: return ret; }
-int fat_mkdir(const char *new_dirname) +int fat_mkdir(const char *dirname) { dir_entry *retdent; fsdata datablock = { .fatbuf = NULL, }; fsdata *mydata = &datablock; fat_itr *itr = NULL; - char *dirname_copy, *parent, *dirname; + char *dirname_copy, *parent, *basename; char l_dirname[VFAT_MAXLEN_BYTES]; int ret = -1; loff_t actwrite; unsigned int bytesperclust; dir_entry *dotdent = NULL;
- dirname_copy = strdup(new_dirname); + dirname_copy = strdup(dirname); if (!dirname_copy) goto exit;
- split_filename(dirname_copy, &parent, &dirname); - if (!strlen(dirname)) { + split_filename(dirname_copy, &parent, &basename); + if (!strlen(basename)) { ret = -EINVAL; goto exit; }
- if (normalize_longname(l_dirname, dirname)) { - printf("FAT: illegal filename (%s)\n", dirname); + if (normalize_longname(l_dirname, basename)) { + printf("FAT: illegal filename (%s)\n", basename); ret = -EINVAL; goto exit; } @@ -1678,7 +1677,7 @@ int fat_mkdir(const char *new_dirname) }
/* Check if long name is needed */ - ndent = set_name(itr, dirname, shortname); + ndent = set_name(itr, basename, shortname); if (ndent < 0) { ret = ndent; goto exit; @@ -1688,7 +1687,7 @@ int fat_mkdir(const char *new_dirname) goto exit; if (ndent > 1) { /* Set long name entries */ - ret = fill_dir_slot(itr, dirname, shortname); + ret = fill_dir_slot(itr, basename, shortname); if (ret) goto exit; } -- 2.30.0
Directories or files called '.' or '..' cannot be created or written to in any directory. Move the test to normalize_longname() to check this early.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2: check for file length 0, simplify check for invalid file names --- fs/fat/fat_write.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index a9b9fa5d68..8945649977 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -1259,8 +1259,10 @@ again: static int normalize_longname(char *l_filename, const char *filename) { const char *p, illegal[] = "<>:"/\|?*"; + size_t len;
- if (strlen(filename) >= VFAT_MAXLEN_BYTES) + len = strlen(filename); + if (!len || len >= VFAT_MAXLEN_BYTES || filename[len - 1] == '.') return -1;
for (p = filename; *p; ++p) { @@ -1348,15 +1350,6 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer, char shortname[SHORT_NAME_SIZE]; int ndent;
- if (itr->is_root) { - /* root dir cannot have "." or ".." */ - if (!strcmp(l_filename, ".") || - !strcmp(l_filename, "..")) { - ret = -EINVAL; - goto exit; - } - } - if (pos) { /* No hole allowed */ ret = -EINVAL; -- 2.30.0
On Thu, 4 Feb 2021 at 00:40, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Directories or files called '.' or '..' cannot be created or written to in any directory. Move the test to normalize_longname() to check this early.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v2: check for file length 0, simplify check for invalid file names
fs/fat/fat_write.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
The FAT32 File System Specification [1] requires leading and trailing spaces as well as trailing periods of long names to be ignored.
[1] Microsoft Extensible Firmware Initiative FAT32 File System Specification Version 1.03, December 6, 2000 Microsoft Corporation https://www.win.tue.nl/~aeb/linux/fs/fat/fatgen103.pdf
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2: special treatment of '.' and '..' --- fs/fat/fat_write.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index 8945649977..8ff2f6def0 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -1237,12 +1237,38 @@ again: }
*last_slash_cont = '\0'; - *basename = last_slash_cont + 1; + filename = last_slash_cont + 1; } else { *dirname = "/"; /* root by default */ - *basename = filename; }
+ /* + * The FAT32 File System Specification v1.03 requires leading and + * trailing spaces as well as trailing periods to be ignored. + */ + for (; *filename == ' '; ++filename) + ; + + /* Keep special entries '.' and '..' */ + if (filename[0] == '.' && + (!filename[1] || (filename[1] == '.' && !filename[2]))) + goto done; + + /* Remove trailing periods and spaces */ + for (p = filename + strlen(filename) - 1; p >= filename; --p) { + switch (*p) { + case ' ': + case '.': + *p = 0; + break; + default: + goto done; + } + } + +done: + *basename = filename; + return 0; }
-- 2.30.0
On Thu, 4 Feb 2021 at 00:40, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
The FAT32 File System Specification [1] requires leading and trailing spaces as well as trailing periods of long names to be ignored.
[1] Microsoft Extensible Firmware Initiative FAT32 File System Specification Version 1.03, December 6, 2000 Microsoft Corporation https://www.win.tue.nl/~aeb/linux/fs/fat/fatgen103.pdf
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v2: special treatment of '.' and '..'
fs/fat/fat_write.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
participants (2)
-
Heinrich Schuchardt -
Simon Glass