Patchwork [U-Boot] fat32 root directory handling

login
register
mail settings
Submitter Erik Hansen
Date March 24, 2011, 9:15 a.m.
Message ID <20110324091537.GA11917@ws038732>
Download mbox | patch
Permalink /patch/88148/
State Superseded
Headers show

Comments

Erik Hansen - March 24, 2011, 9:15 a.m.
Fat directory handling didn't check reaching the end of the root directory. It
relied on a stop condition based on a directory entry with a name starting with
a '\0' character. This check in itself is wrong ('\0' indicates free entry, not
end_of_directory) but outside the scope of this fix. For FAT32, the end of the
rootdir is reached when the end of the cluster chain is reached. The code didn't
check this condition and started to read an incorrect cluster. This caused a
subsequent read request of a sector outside the range of the usb stick in
use. On its turn, the usb stick protested with a stall handshake.

Both FAT32 and non-FAT32 (FAT16/FAT12) end or rootdir checks have been put in.

Signed-off-by: Erik Hansen <erik@makarta.com>
---
 fs/fat/fat.c |   38 ++++++++++++++++++++++++--------------
 1 files changed, 24 insertions(+), 14 deletions(-)
Remy Bohmer - March 25, 2011, 7:25 p.m.
Hi Wolfgang,

2011/3/24 Erik Hansen <erik@makarta.com>:
> Fat directory handling didn't check reaching the end of the root directory. It
> relied on a stop condition based on a directory entry with a name starting with
> a '\0' character. This check in itself is wrong ('\0' indicates free entry, not
> end_of_directory) but outside the scope of this fix. For FAT32, the end of the
> rootdir is reached when the end of the cluster chain is reached. The code didn't
> check this condition and started to read an incorrect cluster. This caused a
> subsequent read request of a sector outside the range of the usb stick in
> use. On its turn, the usb stick protested with a stall handshake.
>
> Both FAT32 and non-FAT32 (FAT16/FAT12) end or rootdir checks have been put in.
>
> Signed-off-by: Erik Hansen <erik@makarta.com>
> ---
>  fs/fat/fat.c |   38 ++++++++++++++++++++++++--------------
>  1 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index a75e4f2..2a3414f 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -31,6 +31,7 @@
>  #include <asm/byteorder.h>
>  #include <part.h>
>
> +
>  /*
>  * Convert a string to lowercase.
>  */

Any objections if I pull this patch in the U-boot USB tree after
fixing this whitespace issue?
At least here here my Ack for the remainder of this patch

Acked-by: Remy Bohmer <linux@bohmer.net>

Kind regards,

Remy

> @@ -209,7 +210,7 @@ static __u32 get_fatent (fsdata *mydata, __u32 entry)
>
>        /* Read a new block of FAT entries into the cache. */
>        if (bufnum != mydata->fatbufnum) {
> -               int getsize = FATBUFSIZE / FS_BLOCK_SIZE;
> +               __u32 getsize = FATBUFSIZE / FS_BLOCK_SIZE;
>                __u8 *bufptr = mydata->fatbuf;
>                __u32 fatlength = mydata->fatlength;
>                __u32 startblock = bufnum * FATBUFBLOCKS;
> @@ -279,7 +280,7 @@ static int
>  get_cluster (fsdata *mydata, __u32 clustnum, __u8 *buffer,
>             unsigned long size)
>  {
> -       int idx = 0;
> +       __u32 idx = 0;
>        __u32 startsect;
>
>        if (clustnum > 0) {
> @@ -767,12 +768,13 @@ do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
>        dir_entry *dentptr;
>        __u16 prevcksum = 0xffff;
>        char *subname = "";
> -       int cursect;
> +       __u32 cursect;
>        int idx, isdir = 0;
>        int files = 0, dirs = 0;
>        long ret = 0;
>        int firsttime;
> -       int root_cluster;
> +       __u32 root_cluster;
> +       int rootdir_size = 0;
>        int j;
>
>        if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
> @@ -798,8 +800,6 @@ do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
>                mydata->data_begin = mydata->rootdir_sect -
>                                        (mydata->clust_size * 2);
>        } else {
> -               int rootdir_size;
> -
>                rootdir_size = ((bs.dir_entries[1]  * (int)256 +
>                                 bs.dir_entries[0]) *
>                                 sizeof(dir_entry)) /
> @@ -1006,20 +1006,18 @@ do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
>                 * root directory clusters when a cluster has been
>                 * completely processed.
>                 */
> -               if ((mydata->fatsize == 32) && (++j == mydata->clust_size)) {
> -                       int nxtsect;
> -                       int nxt_clust;
> +               ++j;
> +               int fat32_end = 0;
> +               if ((mydata->fatsize == 32) && (j == mydata->clust_size)) {
> +                       int nxtsect = 0;
> +                       int nxt_clust = 0;
>
>                        nxt_clust = get_fatent(mydata, root_cluster);
> +                       fat32_end = CHECK_CLUST(nxt_clust, 32);
>
>                        nxtsect = mydata->data_begin +
>                                (nxt_clust * mydata->clust_size);
>
> -                       debug("END LOOP: sect=%d, root_clust=%d, "
> -                             "n_sect=%d, n_clust=%d\n",
> -                             cursect, root_cluster,
> -                             nxtsect, nxt_clust);
> -
>                        root_cluster = nxt_clust;
>
>                        cursect = nxtsect;
> @@ -1027,6 +1025,18 @@ do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
>                } else {
>                        cursect++;
>                }
> +
> +               /* If end of rootdir reached */
> +               if ((mydata->fatsize == 32 && fat32_end) ||
> +                   (mydata->fatsize != 32 && j == rootdir_size)) {
> +                       if (dols == LS_ROOT) {
> +                               printf("\n%d file(s), %d dir(s)\n\n",
> +                                      files, dirs);
> +                               return 0;
> +                       } else {
> +                               return -1;
> +                       }
> +               }
>        }
>  rootdir_done:
>
> --
> 1.7.0.4
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

Patch

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index a75e4f2..2a3414f 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -31,6 +31,7 @@ 
 #include <asm/byteorder.h>
 #include <part.h>
 
+
 /*
  * Convert a string to lowercase.
  */
@@ -209,7 +210,7 @@  static __u32 get_fatent (fsdata *mydata, __u32 entry)
 
 	/* Read a new block of FAT entries into the cache. */
 	if (bufnum != mydata->fatbufnum) {
-		int getsize = FATBUFSIZE / FS_BLOCK_SIZE;
+		__u32 getsize = FATBUFSIZE / FS_BLOCK_SIZE;
 		__u8 *bufptr = mydata->fatbuf;
 		__u32 fatlength = mydata->fatlength;
 		__u32 startblock = bufnum * FATBUFBLOCKS;
@@ -279,7 +280,7 @@  static int
 get_cluster (fsdata *mydata, __u32 clustnum, __u8 *buffer,
 	     unsigned long size)
 {
-	int idx = 0;
+	__u32 idx = 0;
 	__u32 startsect;
 
 	if (clustnum > 0) {
@@ -767,12 +768,13 @@  do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
 	dir_entry *dentptr;
 	__u16 prevcksum = 0xffff;
 	char *subname = "";
-	int cursect;
+	__u32 cursect;
 	int idx, isdir = 0;
 	int files = 0, dirs = 0;
 	long ret = 0;
 	int firsttime;
-	int root_cluster;
+	__u32 root_cluster;
+	int rootdir_size = 0;
 	int j;
 
 	if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
@@ -798,8 +800,6 @@  do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
 		mydata->data_begin = mydata->rootdir_sect -
 					(mydata->clust_size * 2);
 	} else {
-		int rootdir_size;
-
 		rootdir_size = ((bs.dir_entries[1]  * (int)256 +
 				 bs.dir_entries[0]) *
 				 sizeof(dir_entry)) /
@@ -1006,20 +1006,18 @@  do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
 		 * root directory clusters when a cluster has been
 		 * completely processed.
 		 */
-		if ((mydata->fatsize == 32) && (++j == mydata->clust_size)) {
-			int nxtsect;
-			int nxt_clust;
+		++j;
+		int fat32_end = 0;
+		if ((mydata->fatsize == 32) && (j == mydata->clust_size)) {
+			int nxtsect = 0;
+			int nxt_clust = 0;
 
 			nxt_clust = get_fatent(mydata, root_cluster);
+			fat32_end = CHECK_CLUST(nxt_clust, 32);
 
 			nxtsect = mydata->data_begin +
 				(nxt_clust * mydata->clust_size);
 
-			debug("END LOOP: sect=%d, root_clust=%d, "
-			      "n_sect=%d, n_clust=%d\n",
-			      cursect, root_cluster,
-			      nxtsect, nxt_clust);
-
 			root_cluster = nxt_clust;
 
 			cursect = nxtsect;
@@ -1027,6 +1025,18 @@  do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
 		} else {
 			cursect++;
 		}
+
+		/* If end of rootdir reached */
+		if ((mydata->fatsize == 32 && fat32_end) ||
+		    (mydata->fatsize != 32 && j == rootdir_size)) {
+			if (dols == LS_ROOT) {
+				printf("\n%d file(s), %d dir(s)\n\n",
+				       files, dirs);
+				return 0;
+			} else {
+				return -1;
+			}
+		}
 	}
 rootdir_done: