[U-Boot] [PATCH 1/2] fs: fat: treat invalid FAT clusters as errors

[AKASHI Takahiro]

On Thu, Sep 12, 2019 at 07:19:29PM +0200, Heinrich Schuchardt wrote:
> When hitting an invalid FAT cluster while reading a file always print an
> error message and return an error code.

I don't know what the intention of original author was here.

In general, a cluster's FAT entry points to a next cluster
which composes part of a file and hence a chain of clusters.
The last cluster's FAT is marked with EOC(End of Cluster),
which has a dedicated value, for fat16, one of 0xfff8:0xffff.

As you can see, those values are also detected by CHECK_CLUST() macro.
I'm afraid that, at least initially, this macro might have worked
for detecting a file end correctly (so returned 0?).

Another issue is that CHECK_CLUST() checks against >0xfff0 for fat16
while 0xfff0:0xfff6 are still valid for cluster numbers.
So it will potentially detect a wrong "invalid" FAT entry for
a quite large file.

Thanks,
-Takahiro Akashi

> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  fs/fat/fat.c | 30 ++++++++++++++++++++----------
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index 73a89fc9e3..b4e8083734 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -301,10 +301,20 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size)
>  	return 0;
>  }
> 
> -/*
> +/**
> + * get_contents() - read from file
> + *
>   * Read at most 'maxsize' bytes from 'pos' in the file associated with 'dentptr'
> - * into 'buffer'.
> - * Update the number of bytes read in *gotsize or return -1 on fatal errors.
> + * into 'buffer'. Update the number of bytes read in *gotsize or return -1 on
> + * fatal errors.
> + *
> + * @mydata:	file system description
> + * @dentprt:	directory entry pointer
> + * @pos:	position from where to read
> + * @buffer:	buffer into which to read
> + * @maxsize:	maximum number of bytes to read
> + * @gotsize:	number of bytes actually read
> + * Return:	-1 on error, otherwise 0
>   */
>  static int get_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos,
>  			__u8 *buffer, loff_t maxsize, loff_t *gotsize)
> @@ -335,8 +345,8 @@ static int get_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos,
>  		curclust = get_fatent(mydata, curclust);
>  		if (CHECK_CLUST(curclust, mydata->fatsize)) {
>  			debug("curclust: 0x%x\n", curclust);
> -			debug("Invalid FAT entry\n");
> -			return 0;
> +			printf("Invalid FAT entry\n");
> +			return -1;
>  		}
>  		actsize += bytesperclust;
>  	}
> @@ -374,8 +384,8 @@ static int get_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos,
>  		curclust = get_fatent(mydata, curclust);
>  		if (CHECK_CLUST(curclust, mydata->fatsize)) {
>  			debug("curclust: 0x%x\n", curclust);
> -			debug("Invalid FAT entry\n");
> -			return 0;
> +			printf("Invalid FAT entry\n");
> +			return -1;
>  		}
>  	}
> 
> @@ -390,8 +400,8 @@ static int get_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos,
>  				goto getit;
>  			if (CHECK_CLUST(newclust, mydata->fatsize)) {
>  				debug("curclust: 0x%x\n", newclust);
> -				debug("Invalid FAT entry\n");
> -				return 0;
> +				printf("Invalid FAT entry\n");
> +				return -1;
>  			}
>  			endclust = newclust;
>  			actsize += bytesperclust;
> @@ -418,7 +428,7 @@ getit:
>  		if (CHECK_CLUST(curclust, mydata->fatsize)) {
>  			debug("curclust: 0x%x\n", curclust);
>  			printf("Invalid FAT entry\n");
> -			return 0;
> +			return -1;
>  		}
>  		actsize = bytesperclust;
>  		endclust = curclust;
> --
> 2.23.0
>