diff mbox

[U-Boot,v2,2/3] fs/fat: Improve error handling

Message ID 1324487292-7299-3-git-send-email-Kyle.D.Moffett@boeing.com
State Accepted
Headers show

Commit Message

Kyle Moffett Dec. 21, 2011, 5:08 p.m. UTC 
The FAT filesystem fails silently in inexplicable ways when given a
filesystem with a block-size that does not match the device sector size.
In theory this is not an unsupportable combination but requires a major
rewrite of a lot of the filesystem.  Until that occurs, the filesystem
should detect that scenario and display a helpful error message.

This scenario in particular occurred on a 512-byte blocksize FAT fs
stored in an El-Torito boot volume on a CD-ROM (2048-byte sector size).

Additionally, in many circumstances the ->block_read method will not
return a negative number to indicate an error but instead return 0 to
indicate the number of blocks successfully read (IE: None).

The FAT filesystem should defensively check to ensure that it got all of
the sectors that it asked for when reading.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>

--

v2: No change

---
 fs/fat/fat.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

Comments

Wolfgang Denk Jan. 5, 2012, 4:15 p.m. UTC | #1 
Dear Kyle Moffett,

In message <1324487292-7299-3-git-send-email-Kyle.D.Moffett@boeing.com> you wrote:
> The FAT filesystem fails silently in inexplicable ways when given a
> filesystem with a block-size that does not match the device sector size.
> In theory this is not an unsupportable combination but requires a major
> rewrite of a lot of the filesystem.  Until that occurs, the filesystem
> should detect that scenario and display a helpful error message.
> 
> This scenario in particular occurred on a 512-byte blocksize FAT fs
> stored in an El-Torito boot volume on a CD-ROM (2048-byte sector size).
> 
> Additionally, in many circumstances the ->block_read method will not
> return a negative number to indicate an error but instead return 0 to
> indicate the number of blocks successfully read (IE: None).
> 
> The FAT filesystem should defensively check to ensure that it got all of
> the sectors that it asked for when reading.
> 
> Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
> 
> --
> 
> v2: No change
> 
> ---
>  fs/fat/fat.c |   18 ++++++++++++++----
>  1 files changed, 14 insertions(+), 4 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk
diff mbox

Patch

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 9ceb3ec..afe486f 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -276,6 +276,8 @@  get_cluster (fsdata *mydata, __u32 clustnum, __u8 *buffer,
 {
 	__u32 idx = 0;
 	__u32 startsect;
+	__u32 nr_sect;
+	int ret;
 
 	if (clustnum > 0) {
 		startsect = mydata->data_begin +
@@ -286,16 +288,19 @@  get_cluster (fsdata *mydata, __u32 clustnum, __u8 *buffer,
 
 	debug("gc - clustnum: %d, startsect: %d\n", clustnum, startsect);
 
-	if (disk_read(startsect, size / mydata->sect_size, buffer) < 0) {
-		debug("Error reading data\n");
+	nr_sect = size / mydata->sect_size;
+	ret = disk_read(startsect, nr_sect, buffer);
+	if (ret != nr_sect) {
+		debug("Error reading data (got %d)\n", ret);
 		return -1;
 	}
 	if (size % mydata->sect_size) {
 		__u8 tmpbuf[mydata->sect_size];
 
 		idx = size / mydata->sect_size;
-		if (disk_read(startsect + idx, 1, tmpbuf) < 0) {
-			debug("Error reading data\n");
+		ret = disk_read(startsect + idx, 1, tmpbuf);
+		if (ret != 1) {
+			debug("Error reading data (got %d)\n", ret);
 			return -1;
 		}
 		buffer += idx * mydata->sect_size;
@@ -804,6 +809,11 @@  do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
 
 	mydata->sect_size = (bs.sector_size[1] << 8) + bs.sector_size[0];
 	mydata->clust_size = bs.cluster_size;
+	if (mydata->sect_size != cur_part_info.blksz) {
+		printf("Error: FAT sector size mismatch (fs=%hu, dev=%lu)\n",
+				mydata->sect_size, cur_part_info.blksz);
+		return -1;
+	}
 
 	if (mydata->fatsize == 32) {
 		mydata->data_begin = mydata->rootdir_sect -