From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Wed, 18 Sep 2019 13:37:02 +0900 Subject: [U-Boot] [PATCH 1/2] fs: fat: treat invalid FAT clusters as errors In-Reply-To: <20190912171930.17163-2-xypron.glpk@gmx.de> References: <20190912171930.17163-1-xypron.glpk@gmx.de> <20190912171930.17163-2-xypron.glpk@gmx.de> Message-ID: <20190918043701.GC4398@linaro.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > --- > 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 >