[U-Boot] [PATCH] FAT: buffer overflow with FAT12/16
Last commit 3831530dcb7b71329c272ccd6181f8038b6a6dd0a was intended "explicitly specify FAT12/16 root directory parsing buffer size, instead of relying on cluster size". Howver, the underlying function requires the size of the buffer in blocks, not in bytes, and instead of passing a double sector size a request for 1024 blocks is sent. This generates a buffer overflow with overwriting of other structure (in the case seen, USB structures were overwritten).
Signed-off-by: Stefano Babic sbabic@denx.de CC: Mikhail Zolotaryov lebon@lebon.org.ua
--- fs/fat/fat.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 744e961..a75e4f2 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -858,7 +858,7 @@ do_fat_read (const char *filename, void *buffer, unsigned long maxsize, if (disk_read(cursect, (mydata->fatsize == 32) ? (mydata->clust_size) : - LINEAR_PREFETCH_SIZE, + LINEAR_PREFETCH_SIZE / SECTOR_SIZE, do_fat_read_block) < 0) { debug("Error: reading rootdir block\n"); return -1;
Dear Stefano Babic,
In message 1287557505-3955-1-git-send-email-sbabic@denx.de you wrote:
Last commit 3831530dcb7b71329c272ccd6181f8038b6a6dd0a was intended "explicitly specify FAT12/16 root directory parsing buffer size, instead of relying on cluster size". Howver, the underlying function requires the size of the buffer in blocks, not in bytes, and instead of passing a double sector size a request for 1024 blocks is sent. This generates a buffer overflow with overwriting of other structure (in the case seen, USB structures were overwritten).
Signed-off-by: Stefano Babic sbabic@denx.de CC: Mikhail Zolotaryov lebon@lebon.org.ua
fs/fat/fat.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Nice catch!
Applied, thanks.
Best regards,
Wolfgang Denk
Stefano,
you're right, sorry for that. Isn't it better to define LINEAR_PREFETCH_SIZE as 2 (blocks) and change get_vfatname() to something like that in such case: __u8 *buflimit = cluster + ((curclust == 0) ? LINEAR_PREFETCH_SIZE : mydata->clust_size ) * SECTOR_SIZE; Regards, Mikhail
On 20.10.10 09:51, Stefano Babic wrote:
Last commit 3831530dcb7b71329c272ccd6181f8038b6a6dd0a was intended "explicitly specify FAT12/16 root directory parsing buffer size, instead of relying on cluster size". Howver, the underlying function requires the size of the buffer in blocks, not in bytes, and instead of passing a double sector size a request for 1024 blocks is sent. This generates a buffer overflow with overwriting of other structure (in the case seen, USB structures were overwritten).
Signed-off-by: Stefano Babic sbabic@denx.de CC: Mikhail Zolotaryov lebon@lebon.org.ua
fs/fat/fat.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 744e961..a75e4f2 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -858,7 +858,7 @@ do_fat_read (const char *filename, void *buffer, unsigned long maxsize, if (disk_read(cursect, (mydata->fatsize == 32) ? (mydata->clust_size) :
LINEAR_PREFETCH_SIZE,
LINEAR_PREFETCH_SIZE / SECTOR_SIZE, do_fat_read_block) < 0) { debug("Error: reading rootdir block\n"); return -1;
participants (3)
-
Mikhail Zolotaryov -
Stefano Babic -
Wolfgang Denk