* [U-Boot] [PATCH 0/2] fs: fat: error handling in get_contents() @ 2019-09-12 17:19 Heinrich Schuchardt 2019-09-12 17:19 ` [U-Boot] [PATCH 1/2] fs: fat: treat invalid FAT clusters as errors Heinrich Schuchardt 2019-09-12 17:19 ` [U-Boot] [PATCH 2/2] fs: fat: get_contents() always returns -1 for errors Heinrich Schuchardt 0 siblings, 2 replies; 6+ messages in thread From: Heinrich Schuchardt @ 2019-09-12 17:19 UTC (permalink / raw) To: u-boot If a FAT entry is corrupted, reading a file should always lead to an error. get_contents() should always return -1 when an error occurs. Heinrich Schuchardt (2): fs: fat: treat invalid FAT clusters as errors fs: fat: get_contents() always returns -1 for errors fs/fat/fat.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) -- 2.23.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/2] fs: fat: treat invalid FAT clusters as errors 2019-09-12 17:19 [U-Boot] [PATCH 0/2] fs: fat: error handling in get_contents() Heinrich Schuchardt @ 2019-09-12 17:19 ` Heinrich Schuchardt 2019-09-18 4:37 ` AKASHI Takahiro 2019-10-12 20:28 ` Tom Rini 2019-09-12 17:19 ` [U-Boot] [PATCH 2/2] fs: fat: get_contents() always returns -1 for errors Heinrich Schuchardt 1 sibling, 2 replies; 6+ messages in thread From: Heinrich Schuchardt @ 2019-09-12 17:19 UTC (permalink / raw) To: u-boot When hitting an invalid FAT cluster while reading a file always print an error message and return an error code. Signed-off-by: Heinrich Schuchardt <xypron.glpk@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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/2] fs: fat: treat invalid FAT clusters as errors 2019-09-12 17:19 ` [U-Boot] [PATCH 1/2] fs: fat: treat invalid FAT clusters as errors Heinrich Schuchardt @ 2019-09-18 4:37 ` AKASHI Takahiro 2019-10-12 20:28 ` Tom Rini 1 sibling, 0 replies; 6+ messages in thread From: AKASHI Takahiro @ 2019-09-18 4:37 UTC (permalink / raw) To: u-boot 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@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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/2] fs: fat: treat invalid FAT clusters as errors 2019-09-12 17:19 ` [U-Boot] [PATCH 1/2] fs: fat: treat invalid FAT clusters as errors Heinrich Schuchardt 2019-09-18 4:37 ` AKASHI Takahiro @ 2019-10-12 20:28 ` Tom Rini 1 sibling, 0 replies; 6+ messages in thread From: Tom Rini @ 2019-10-12 20:28 UTC (permalink / raw) To: u-boot 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. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> Applied to u-boot/master, thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.denx.de/pipermail/u-boot/attachments/20191012/c36d26d1/attachment.sig> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] fs: fat: get_contents() always returns -1 for errors 2019-09-12 17:19 [U-Boot] [PATCH 0/2] fs: fat: error handling in get_contents() Heinrich Schuchardt 2019-09-12 17:19 ` [U-Boot] [PATCH 1/2] fs: fat: treat invalid FAT clusters as errors Heinrich Schuchardt @ 2019-09-12 17:19 ` Heinrich Schuchardt 2019-10-12 20:28 ` Tom Rini 1 sibling, 1 reply; 6+ messages in thread From: Heinrich Schuchardt @ 2019-09-12 17:19 UTC (permalink / raw) To: u-boot If out of memory, return -1 and not -ENOMEM from get_contents(). Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- fs/fat/fat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/fat/fat.c b/fs/fat/fat.c index b4e8083734..da822f4f38 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -364,7 +364,7 @@ static int get_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos, tmp_buffer = malloc_cache_aligned(actsize); if (!tmp_buffer) { debug("Error: allocating buffer\n"); - return -ENOMEM; + return -1; } if (get_cluster(mydata, curclust, tmp_buffer, actsize) != 0) { -- 2.23.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] fs: fat: get_contents() always returns -1 for errors 2019-09-12 17:19 ` [U-Boot] [PATCH 2/2] fs: fat: get_contents() always returns -1 for errors Heinrich Schuchardt @ 2019-10-12 20:28 ` Tom Rini 0 siblings, 0 replies; 6+ messages in thread From: Tom Rini @ 2019-10-12 20:28 UTC (permalink / raw) To: u-boot On Thu, Sep 12, 2019 at 07:19:30PM +0200, Heinrich Schuchardt wrote: > If out of memory, return -1 and not -ENOMEM from get_contents(). > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> Applied to u-boot/master, thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.denx.de/pipermail/u-boot/attachments/20191012/861d49eb/attachment.sig> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-12 20:28 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-09-12 17:19 [U-Boot] [PATCH 0/2] fs: fat: error handling in get_contents() Heinrich Schuchardt 2019-09-12 17:19 ` [U-Boot] [PATCH 1/2] fs: fat: treat invalid FAT clusters as errors Heinrich Schuchardt 2019-09-18 4:37 ` AKASHI Takahiro 2019-10-12 20:28 ` Tom Rini 2019-09-12 17:19 ` [U-Boot] [PATCH 2/2] fs: fat: get_contents() always returns -1 for errors Heinrich Schuchardt 2019-10-12 20:28 ` Tom Rini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox