volume_id/fat: careful with sector#, it may not fit in 32 bits. +91 bytes

volume_id/*: a bit of code shrink

diff --git a/util-linux/volume_id/fat.c b/util-linux/volume_id/fat.c
index 816d69d4c6e909d6490deb09c74b1d9b2fedb473..0e0a57d62c53599c648d3d8b56b3d0ecb8693ec9 100644 (file)
--- 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 c4d20ba470245a48f01899813d3d67f642a2f472..1a1b3f92e4210aef48d054faa17030e9d1019f48 100644 (file)
--- 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 075ddb344fca8eb0cd3f6c86c998ebedd6afc822..fe3547d13394a2bfc76dd6922c0a679bcf86e497 100644 (file)
--- 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);