From 1e10afcdfb2b3725458d7a245aa0fe2d39f6d812 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Sun, 30 Nov 2008 17:41:31 +0000 Subject: [PATCH] volume_id/fat: careful with sector#, it may not fit in 32 bits. +91 bytes volume_id/*: a bit of code shrink --- util-linux/volume_id/fat.c | 14 ++-- util-linux/volume_id/util.c | 96 ++++++++++++----------- util-linux/volume_id/volume_id_internal.h | 24 +++--- 3 files changed, 68 insertions(+), 66 deletions(-) diff --git a/util-linux/volume_id/fat.c b/util-linux/volume_id/fat.c index 816d69d4c..0e0a57d62 100644 --- a/util-linux/volume_id/fat.c +++ b/util-linux/volume_id/fat.c @@ -245,7 +245,7 @@ int volume_id_probe_vfat(struct volume_id *id, uint64_t fat_partition_off) buf_size = dir_entries * sizeof(struct vfat_dir_entry); buf = volume_id_get_buffer(id, fat_partition_off + root_start_off, buf_size); if (buf == NULL) - goto found; + goto ret; label = get_attr_volume_id((struct vfat_dir_entry*) buf, dir_entries); @@ -261,7 +261,7 @@ int volume_id_probe_vfat(struct volume_id *id, uint64_t fat_partition_off) volume_id_set_label_string(id, vs->type.fat.label, 11); } volume_id_set_uuid(id, vs->type.fat.serno, UUID_DOS); - goto found; + goto ret; fat32: /* FAT32 root dir is a cluster chain like any other directory */ @@ -272,20 +272,20 @@ int volume_id_probe_vfat(struct volume_id *id, uint64_t fat_partition_off) next_cluster = root_cluster; maxloop = 100; while (--maxloop) { - uint32_t next_off_sct; + uint64_t next_off_sct; uint64_t next_off; uint64_t fat_entry_off; int count; dbg("next_cluster 0x%x", (unsigned)next_cluster); - next_off_sct = (next_cluster - 2) * vs->sectors_per_cluster; + next_off_sct = (uint64_t)(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; + goto ret; dir = (struct vfat_dir_entry*) buf; count = buf_size / sizeof(struct vfat_dir_entry); @@ -300,7 +300,7 @@ int volume_id_probe_vfat(struct volume_id *id, uint64_t fat_partition_off) dbg("fat_entry_off 0x%llx", (unsigned long long)fat_entry_off); buf = volume_id_get_buffer(id, fat_partition_off + fat_entry_off, buf_size); if (buf == NULL) - goto found; + goto ret; /* set next cluster */ next_cluster = le32_to_cpu(*(uint32_t*)buf) & 0x0fffffff; @@ -323,7 +323,7 @@ int volume_id_probe_vfat(struct volume_id *id, uint64_t fat_partition_off) } volume_id_set_uuid(id, vs->type.fat32.serno, UUID_DOS); - found: + ret: // volume_id_set_usage(id, VOLUME_ID_FILESYSTEM); // id->type = "vfat"; diff --git a/util-linux/volume_id/util.c b/util-linux/volume_id/util.c index c4d20ba47..1a1b3f92e 100644 --- a/util-linux/volume_id/util.c +++ b/util-linux/volume_id/util.c @@ -195,70 +195,73 @@ set: * It's better to ignore such fs and continue. */ void *volume_id_get_buffer(struct volume_id *id, uint64_t off, size_t len) { - ssize_t buf_len; + uint8_t *dst; + unsigned small_off; + ssize_t read_len; + + dbg("get buffer off 0x%llx(%llu), len 0x%zx", + (unsigned long long) off, (unsigned long long) off, len); - dbg("get buffer off 0x%llx(%llu), len 0x%zx", (unsigned long long) off, (unsigned long long) off, len); /* check if requested area fits in superblock buffer */ - if (off + len <= SB_BUFFER_SIZE) { + if (off + len <= SB_BUFFER_SIZE + /* && off <= SB_BUFFER_SIZE - want this paranoid overflow check? */ + ) { if (id->sbbuf == NULL) { id->sbbuf = xmalloc(SB_BUFFER_SIZE); } + small_off = off; + dst = id->sbbuf; /* check if we need to read */ - if ((off + len) > id->sbbuf_len) { - dbg("read sbbuf len:0x%llx", (unsigned long long) (off + len)); - if (lseek(id->fd, 0, SEEK_SET) != 0) { - dbg("seek(0) failed"); - return NULL; - } - buf_len = full_read(id->fd, id->sbbuf, off + len); - if (buf_len < 0) { - dbg("read failed (%s)", strerror(errno)); - return NULL; - } - dbg("got 0x%zx (%zi) bytes", buf_len, buf_len); - id->sbbuf_len = buf_len; - if ((uint64_t)buf_len < off + len) { - dbg("requested 0x%zx bytes, got only 0x%zx bytes", len, buf_len); - return NULL; - } - } + len += off; + if (len <= id->sbbuf_len) + goto ret; /* we already have it */ - return &(id->sbbuf[off]); + dbg("read sbbuf len:0x%x", (unsigned) len); + id->sbbuf_len = len; + off = 0; + goto do_read; } if (len > SEEK_BUFFER_SIZE) { dbg("seek buffer too small %d", SEEK_BUFFER_SIZE); return NULL; } - - /* get seek buffer */ - if (id->seekbuf == NULL) { - id->seekbuf = xmalloc(SEEK_BUFFER_SIZE); - } + dst = id->seekbuf; /* check if we need to read */ - if ((off < id->seekbuf_off) || ((off + len) > (id->seekbuf_off + id->seekbuf_len))) { - 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; - } - dbg("got 0x%zx (%zi) bytes", buf_len, buf_len); - id->seekbuf_off = off; - id->seekbuf_len = buf_len; - if ((size_t)buf_len < len) { - dbg("requested 0x%zx bytes, got only 0x%zx bytes", len, buf_len); - return NULL; - } + if ((off >= id->seekbuf_off) + && ((off + len) <= (id->seekbuf_off + id->seekbuf_len)) + ) { + small_off = off - id->seekbuf_off; /* can't overflow */ + goto ret; /* we already have it */ } - return &(id->seekbuf[off - id->seekbuf_off]); + id->seekbuf_off = off; + id->seekbuf_len = len; + id->seekbuf = xrealloc(id->seekbuf, len); + small_off = 0; + dst = id->seekbuf; + dbg("read seekbuf off:0x%llx len:0x%zx", + (unsigned long long) off, len); + do_read: + if (lseek(id->fd, off, SEEK_SET) != off) { + dbg("seek(0x%llx) failed", (unsigned long long) off); + goto err; + } + read_len = full_read(id->fd, dst, len); + if (read_len != len) { + dbg("requested 0x%x bytes, got 0x%x bytes", + (unsigned) len, (unsigned) read_len); + err: + /* id->seekbuf_len or id->sbbuf_len is wrong now! Fixing. + * Most likely user will not do any additional + * calls anyway, it's a corrupted fs or something. */ + volume_id_free_buffer(id); + return NULL; + } + ret: + return dst + small_off; } void volume_id_free_buffer(struct volume_id *id) @@ -269,4 +272,5 @@ void volume_id_free_buffer(struct volume_id *id) free(id->seekbuf); id->seekbuf = NULL; id->seekbuf_len = 0; + id->seekbuf_off = 0; /* paranoia */ } diff --git a/util-linux/volume_id/volume_id_internal.h b/util-linux/volume_id/volume_id_internal.h index 075ddb344..fe3547d13 100644 --- a/util-linux/volume_id/volume_id_internal.h +++ b/util-linux/volume_id/volume_id_internal.h @@ -61,6 +61,17 @@ struct volume_id_partition { #endif struct volume_id { + int fd; +// int fd_close:1; + size_t sbbuf_len; + size_t seekbuf_len; + uint8_t *sbbuf; + uint8_t *seekbuf; + uint64_t seekbuf_off; +#ifdef UNUSED_PARTITION_CODE + struct volume_id_partition *partitions; + size_t partition_count; +#endif // uint8_t label_raw[VOLUME_ID_LABEL_SIZE]; // size_t label_raw_len; char label[VOLUME_ID_LABEL_SIZE+1]; @@ -72,19 +83,6 @@ struct volume_id { // smallint usage_id; // const char *usage; // const char *type; - -#ifdef UNUSED_PARTITION_CODE - struct volume_id_partition *partitions; - size_t partition_count; -#endif - - int fd; - uint8_t *sbbuf; - uint8_t *seekbuf; - size_t sbbuf_len; - uint64_t seekbuf_off; - size_t seekbuf_len; -// int fd_close:1; }; struct volume_id *volume_id_open_node(int fd);