From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mikhail Zolotaryov Date: Wed, 20 Oct 2010 10:23:51 +0300 Subject: [U-Boot] [PATCH] FAT: buffer overflow with FAT12/16 In-Reply-To: <1287557505-3955-1-git-send-email-sbabic@denx.de> References: <1287557505-3955-1-git-send-email-sbabic@denx.de> Message-ID: <4CBE9907.5010307@lebon.org.ua> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > CC: Mikhail Zolotaryov > > --- > 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;