VFAT: fix processing of scattered long file name entries

The U-Boot code has the following bugs related to the processing of Long File
Name (LFN) entries scattered across several clusters/sectors :

1) get_vfatname() function is designed to gather scattered LFN entries by
cluster chain processing - that doesn't work for FAT12/16 root directory.
In other words, the function expects the following input data:
 1.1) FAT32 directory (which is cluster chain based);
        OR
 1.2) FAT12/16 non-root directory (which is also cluster chain based);
        OR
 1.3) FAT12/16 root directory (allocated as contiguous sectors area), but
 all necessary information MUST be within the input buffer of filesystem cluster
 size (thus cluster-chain jump is never initiated).

In order to accomplish the last condition, root directory parsing code in
do_fat_read() uses the following trick: read-out cluster-size block, process
only first sector (512 bytes), then shift 512 forward, read-out cluster-size
block and so on. This works great unless cluster size is equal to 512 bytes
(in a case you have a small partition), or long file name entries are scattered
across three sectors, see 4) for details.

2) Despite of the fact that get_vfatname() supports FAT32 root directory
browsing, do_fat_read() function doesn't send current cluster number correctly,
so root directory look-up doesn't work correctly.

3) get_vfatname() doesn't gather scattered entries correctly also is the case
when all LFN entries are located at the end of the source cluster, but real
directory entry (which must be returned) is at the only beginning of the
next one. No error detected, the resulting directory entry returned contains
a semi-random information (wrong size, wrong start cluster number and so on)
i.e. the entry is not accessible.

4) LFN (VFAT) allows up to 20 entries (slots) each containing 26 bytes (13
UTF-16 code units) to represent a single long file name i.e. up to 520 bytes.
U-Boot allocates 256 bytes buffer instead, i.e. 10 or more LFN slots record
may cause buffer overflow / memory corruption.
Also, it's worth to mention that 20+1 slots occupy 672 bytes space which may
take more than one cluster of 512 bytes (medium-size FAT32 or small FAT16
partition) - get_vfatname() function doesn't support such case as well.

The patch attached fixes these problems in the following way:
- keep using 256 bytes buffer for a long file name, but safely prevent a
possible buffer overflow (skip LFN processing, if it contains 10 or more
slots).

- explicitly specify FAT12/16 root directory parsing buffer size, instead
of relying on cluster size. The value used is a double sector size (to store
current sector and the next one). This fixes the first problem and increases
performance on big FAT12/16 partitions;

- send current cluster number (FAT32) to get_vfatname() during root
directory processing;

- use LFN counter to seek the real directory entry in get_vfatname() - fixes the
third problem;

- skip deleted entries in the root directory (to prevent bogus buffer
overflow detection and LFN counter steps).

Note: it's not advised to split up the patch, because a separate part may
operate incorrectly.

Signed-off-by: Mikhail Zolotaryov <lebon@lebon.org.ua>

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 003666eaec449008b61721dd645b26f4423c45ce..744e961847263f17da8058bb06736daf967178c0 100644 (file)
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -439,11 +439,19 @@ get_vfatname (fsdata *mydata, int curclust, __u8 *cluster,
 {
        dir_entry *realdent;
        dir_slot *slotptr = (dir_slot *)retdent;
-       __u8 *nextclust = cluster + mydata->clust_size * SECTOR_SIZE;
+       __u8 *buflimit = cluster + ((curclust == 0) ?
+                                       LINEAR_PREFETCH_SIZE :
+                                       (mydata->clust_size * SECTOR_SIZE)
+                                  );
        __u8 counter = (slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff;
        int idx = 0;
 
-       while ((__u8 *)slotptr < nextclust) {
+       if (counter > VFAT_MAXSEQ) {
+               debug("Error: VFAT name is too long\n");
+               return -1;
+       }
+
+       while ((__u8 *)slotptr < buflimit) {
                if (counter == 0)
                        break;
                if (((slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff) != counter)
@@ -452,10 +460,11 @@ get_vfatname (fsdata *mydata, int curclust, __u8 *cluster,
                counter--;
        }
 
-       if ((__u8 *)slotptr >= nextclust) {
+       if ((__u8 *)slotptr >= buflimit) {
                dir_slot *slotptr2;
 
-               slotptr--;
+               if (curclust == 0)
+                       return -1;
                curclust = get_fatent(mydata, curclust);
                if (CHECK_CLUST(curclust, mydata->fatsize)) {
                        debug("curclust: 0x%x\n", curclust);
@@ -470,14 +479,19 @@ get_vfatname (fsdata *mydata, int curclust, __u8 *cluster,
                }
 
                slotptr2 = (dir_slot *)get_vfatname_block;
-               while (slotptr2->id > 0x01)
+               while (counter > 0) {
+                       if (((slotptr2->id & ~LAST_LONG_ENTRY_MASK)
+                           & 0xff) != counter)
+                               return -1;
                        slotptr2++;
+                       counter--;
+               }
 
                /* Save the real directory entry */
-               realdent = (dir_entry *)slotptr2 + 1;
-               while ((__u8 *)slotptr2 >= get_vfatname_block) {
-                       slot2str(slotptr2, l_name, &idx);
+               realdent = (dir_entry *)slotptr2;
+               while ((__u8 *)slotptr2 > get_vfatname_block) {
                        slotptr2--;
+                       slot2str(slotptr2, l_name, &idx);
                }
        } else {
                /* Save the real directory entry */
@@ -549,7 +563,7 @@ static dir_entry *get_dentfromdir (fsdata *mydata, int startsect,
                dentptr = (dir_entry *)get_dentfromdir_block;
 
                for (i = 0; i < DIRENTSPERCLUST; i++) {
-                       char s_name[14], l_name[256];
+                       char s_name[14], l_name[VFAT_MAXLEN_BYTES];
 
                        l_name[0] = '\0';
                        if (dentptr->name[0] == DELETED_FLAG) {
@@ -841,7 +855,11 @@ do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
                debug("FAT read sect=%d, clust_size=%d, DIRENTSPERBLOCK=%d\n",
                        cursect, mydata->clust_size, DIRENTSPERBLOCK);
 
-               if (disk_read(cursect, mydata->clust_size, do_fat_read_block) < 0) {
+               if (disk_read(cursect,
+                               (mydata->fatsize == 32) ?
+                               (mydata->clust_size) :
+                               LINEAR_PREFETCH_SIZE,
+                               do_fat_read_block) < 0) {
                        debug("Error: reading rootdir block\n");
                        return -1;
                }
@@ -849,9 +867,13 @@ do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
                dentptr = (dir_entry *) do_fat_read_block;
 
                for (i = 0; i < DIRENTSPERBLOCK; i++) {
-                       char s_name[14], l_name[256];
+                       char s_name[14], l_name[VFAT_MAXLEN_BYTES];
 
                        l_name[0] = '\0';
+                       if (dentptr->name[0] == DELETED_FLAG) {
+                               dentptr++;
+                               continue;
+                       }
                        if ((dentptr->attr & ATTR_VOLUME)) {
 #ifdef CONFIG_SUPPORT_VFAT
                                if ((dentptr->attr & ATTR_VFAT) &&
@@ -859,7 +881,10 @@ do_fat_read (const char *filename, void *buffer, unsigned long maxsize,
                                        prevcksum =
                                                ((dir_slot *)dentptr)->alias_checksum;
 
-                                       get_vfatname(mydata, 0,
+                                       get_vfatname(mydata,
+                                                    (mydata->fatsize == 32) ?
+                                                    root_cluster :
+                                                    0,
                                                     do_fat_read_block,
                                                     dentptr, l_name);
 

diff --git a/include/fat.h b/include/fat.h
index de48afd730654630f7e7fc803a2ca017feb575a0..afb2116e8e6de29a54db6bac1bc2781ae76d1af8 100644 (file)
--- a/include/fat.h
+++ b/include/fat.h
@@ -30,6 +30,10 @@
 #include <asm/byteorder.h>
 
 #define CONFIG_SUPPORT_VFAT
+/* Maximum Long File Name length supported here is 128 UTF-16 code units */
+#define VFAT_MAXLEN_BYTES      256 /* Maximum LFN buffer in bytes */
+#define VFAT_MAXSEQ            9   /* Up to 9 of 13 2-byte UTF-16 entries */
+#define LINEAR_PREFETCH_SIZE   (SECTOR_SIZE*2) /* Prefetch buffer size */
 
 #define SECTOR_SIZE FS_BLOCK_SIZE