Hello all,
I've recently been looking into a problem loading files from a FAT32 filesystem and (re)discovered that there are issues with this code.
Some of the symptoms are related to problems discussed in a thread back in April titled "USB SUPPORT & get_vfatname".
http://lists.denx.de/pipermail/u-boot/2008-April/033117.html
I ran into the issue(s) on an SD(MMC) card when I noticed a failure to load a certain file from the card.
I've been able to narrow down a set of problems in fs/fat.c. I'm not sure when I'll have the time to generate a proper patch, but wanted to at least write some notes on the nature of the problem to save others some grief.
The first problem I encountered was that the main loop in fs/fat.c/do_fat_read was built for fixed-size root directories. The Microsoft document "FAT: General Overview of On-Disk Format" says this on page 22: "For FAT32, the root directory can be of variable size and is a cluster chain, just like any other ..."
When running the command:
fatls mmc 0
I found that do_fat_read() was walking right past the end of the first cluster of the root directory and spewing garbage to my console when it tried to interpret non-directory data as a set of directory entries.
I also found that Jochen Karrer posted a patch back in 2006 to address this and other related issues, some of which have subsequently been fixed in the main-line U-Boot tree.
http://sourceforge.net/tracker/index.php?func=detail&aid=1490921&gro...
Taking Jochen's lead, I tried to hack the main loop of do_fat_read() and found other issues. The root directory loop looks more or less like this:
804 while (1) { 805 int i; 806 807 if (disk_read (cursect, mydata->clust_size, do_fat_read_block) < 0) { 808 FAT_DPRINT ("Error: reading rootdir block\n"); 809 return -1; 810 } 811 dentptr = (dir_entry *) do_fat_read_block; 812 for (i = 0; i < DIRENTSPERBLOCK; i++) { ...
... different paths either return, bail out of the outer loop or increment dentptr 918 } 919 cursect++; 920 }
I put a check in for cursect > max at line 920 and read the next cluster when processing a FAT32 partition. This eliminated the immediate issue I was seeing, but testing with a large number of files generated using the command below, I found that a handful of files did not show up using 'fatls' and one file showed up as being a directory instead of a file.
for n in `seq 1 1000` ; do echo $n >> /media/disk/testfile$n ; done
Digging deeper, I found that the lost files and the file-as-directory issue all occurred in the last directory entry within a cluster. If I understand correctly, this problem stems from the way in which data is handed off to the get_vfatname() routine by the caller.
The get_vfatname routine has this interface:
static int get_vfatname(fsdata *mydata, int curclust, __u8 *cluster, dir_entry *retdent, char *l_name)
The primary inputs are mydata, which contains filesystem information including the number of sectors per cluster and the start of the current cluster (cluster).
The outputs are retdent (which receives the old 8.3-style directory entry) and l_name, which receives the long filename.
Since get_vfatname() needs to walk a set of vfat entries, it may need another cluster of data, which is the second place problems arise. get_vfatname() does this calculation by rightly assuming that an entire cluster of data is present in the cluster[] variable. Unfortunately, the main root directory loop above appears to be broken:
Line 807 reads an entire cluster 807 if (disk_read (cursect, mydata->clust_size, do_fat_read_block) < 0) {
but the inner loop only appears to walk one sector's worth of data.
812 for (i = 0; i < DIRENTSPERBLOCK; i++) { ... 919 cursect++; 920 }
Besides the inefficiency of re-reading the same sectors, this will read the wrong data at the end of a cluster. In essence, adjacent sectors will be read into the do_fat_read_block[] variable instead of sectors in the 'linked' cluster.
get_vfatname() will then blindly process this data as a set of directory entries...
Off-hand, it appears that this second problem can be fixed by using DIRENTSPERCLUST instead of DIRENTSPERBLOCK in the for loop. Unfortunately, something is preventing this from working, and I'm running out of time to pursue it at the moment.
As I mentioned earlier, I wanted to at least write a few notes about my findings. If I get time, I'll work up a proper patch. In the mean-time, I'll try to keep my root directory small.
Anything under around 40 files (depending on file name length) should fit within the 8-sector cluster on my SD card.
I hope this helps.
Eric Nelson Boundary Devices