Dear Mikhail Zolotaryov,
In message 1283954763-22436-1-git-send-email-lebon@lebon.org.ua you wrote:
The U-Boot code has the following bugs related to the processing of Long File Name (LFN) entries scattered across several clusters/sectors :
- 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.
- 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.
- 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.
- 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(-)
Applied, thanks.
Best regards,
Wolfgang Denk