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 --- fs/fat/fat.c | 49 +++++++++++++++++++++++++++++++++++++------------ include/fat.h | 4 ++++ 2 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 003666e..2b3c256 100644 --- 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 de48afd..afb2116 100644 --- 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