[PATCH] invalid lseek when determining uuid of (possibly corrupt) fat

Denys Vlasenko vda.linux at googlemail.com
Sun Nov 30 16:35:06 UTC 2008


On Saturday 29 November 2008 20:35, Clemens Helfmeier wrote:
> Hi Denys,
> 
> I just tested 1.13.1 and it works fine so far.
> But why did you use a constant for the upper boundary?
> 
> > #define FAT32_MAX                       0x0ffffff6
> >   if (next_cluster < 2 || next_cluster > FAT32_MAX)
> 
> I think that can still result in the same behaviour when the next_cluster 
> points to a cluster between the actual size of the partition and FAT32_MAX.

Because that is how FAT chain ends. If has 0x0fffffff "eof marker"
as a next cluster no.

If you are worried what will happen if damaged FAT table
points to, say, bogus cluster no. 0x0fffff00,
code elsewhere would detect reads past partition end:

                dbg("read seekbuf off:0x%llx len:0x%zx", (unsigned long long) off, len);
                if (lseek(id->fd, off, SEEK_SET) != off) {
                        dbg("seek(0x%llx) failed", (unsigned long long) off);
                        return NULL;
                }
                buf_len = full_read(id->fd, id->seekbuf, len);
                if (buf_len < 0) {
                        dbg("read failed (%s)", strerror(errno));
                        return NULL;
                }

and then here volume_id_get_buffer() would return NULL:

                next_off_sct = (next_cluster - 2) * vs->sectors_per_cluster;
                next_off = (start_data_sct + next_off_sct) * sector_size_bytes;
                dbg("cluster offset 0x%llx", (unsigned long long) next_off);

                /* get cluster */
                buf = volume_id_get_buffer(id, fat_partition_off + next_off, buf_size);
                if (buf == NULL)
                        goto found;

root dir scan will end without aborting the program.

I probably need to throw in (off_t) conversion to prevent overflows...
--
vda



More information about the busybox mailing list