[U-Boot] [PATCH 1/1] fs: fat: correct file name normalization

[AKASHI Takahiro]

Heinrich,

On Sun, May 12, 2019 at 09:59:18AM +0200, Heinrich Schuchardt wrote:
> File names may not contain control characters (< 0x20).
> Simplify the coding.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  fs/fat/fat_write.c | 48 +++++++++++++++++++---------------------------
>  1 file changed, 20 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> index 852f874e58..2a74199236 100644
> --- a/fs/fat/fat_write.c
> +++ b/fs/fat/fat_write.c
> @@ -1009,40 +1009,32 @@ again:
>  	return 0;
>  }
> 
> +/**
> + * normalize_longname() - check long file name and convert to lower case
> + *
> + * We assume here that the FAT file system is using an 8bit code page.
> + * Linux typically uses CP437, EDK2 assumes CP1250.
> + *
> + * @l_filename:	preallocated buffer receiving the normalized name
> + * @filename:	filename to normalize
> + * Return:	0 on success, -1 on failure
> + */
>  static int normalize_longname(char *l_filename, const char *filename)
>  {
> -	const char *p, legal[] = "!#$%&\'()-.@^`_{}~";
> -	unsigned char c;
> -	int name_len;
> -
> -	/* Check that the filename is valid */
> -	for (p = filename; p < filename + strlen(filename); p++) {
> -		c = *p;
> -
> -		if (('0' <= c) && (c <= '9'))
> -			continue;
> -		if (('A' <= c) && (c <= 'Z'))
> -			continue;
> -		if (('a' <= c) && (c <= 'z'))
> -			continue;
> -		if (strchr(legal, c))
> -			continue;
> -		/* extended code */
> -		if ((0x80 <= c) && (c <= 0xff))
> -			continue;
> +	const char *p, illegal[] = "<>:\"/\\|?*";

With your patch, 8 characters, " +,;-[]" and \177 (= DEL), are
added as legal characters. DEL should be excluded.
Other than that, the rule seems to be fine for long file names.

> +	if (strlen(filename) >= VFAT_MAXLEN_BYTES)
>  		return -1;
> -	}
> 
> -	/* Normalize it */
> -	name_len = strlen(filename);
> -	if (name_len >= VFAT_MAXLEN_BYTES)
> -		/* should return an error? */
> -		name_len = VFAT_MAXLEN_BYTES - 1;
> +	for (p = filename; *p; ++p) {
> +		if ((unsigned char)*p < 0x20)
> +			return -1;
> +		if (strchr(illegal, *p))
> +			return -1;
> +	}
> 
> -	memcpy(l_filename, filename, name_len);
> -	l_filename[name_len] = 0; /* terminate the string */
> -	downcase(l_filename, INT_MAX);
> +	strcpy(l_filename, filename);
> +	downcase(l_filename, VFAT_MAXLEN_BYTES);

Strictly speaking,
it is not necessary to 'downcase' a file name here as a long file
name may contain lower-case characters.
On the other hand, a short file name, which is to be set in
set_name(), should be checked against more restricted character set.

In addition, 
l_filename, instead of filename, should be passed to fill_dir_slot()
at file_fat_write_at() as fill_dir_slot() expects a long file name.
l_dirname in fat_mkdir() as well.

-Takahiro Akashi

>  	return 0;
>  }
> --
> 2.20.1
>