Centralize read/write size calculation; test partial reads

This trims read sizes so that they can't read past the end of the media.
This didn't fail before thanks to the OS trimming read operations. This
change makes fwup more precise.

The regression test for partial reads is silly, but exercises the code
path.

src/block_cache.c

@@ -152,16 +152,48 @@ static void init_segment(struct block_cache *bc, off_t offset, struct block_cach
     seg->streamed = true;
 }
 
+static int calculate_io_size(struct block_cache *bc, off_t offset, size_t *count)
+{
+    // If there's a max destination size, then check if it's been hit or
+    // whether this should be a partial write.
+    if (bc->end_offset > 0) {
+        off_t last_offset = offset + BLOCK_CACHE_SEGMENT_SIZE;
+        if (last_offset <= bc->end_offset) {
+            // Common case: reading or writing before the end
+            *count = BLOCK_CACHE_SEGMENT_SIZE;
+        } else if (last_offset > bc->end_offset) {
+            // At least some reads or writes are after the end
+            if (offset <= bc->end_offset) {
+                // Shrink the read/write count to support the partial segment
+                *count = bc->end_offset - offset;
+            } else {
+                // Fail if the write is completely past the end
+                *count = 0;
+                ERR_RETURN("read/write failed at offset %" PRId64 " since past end of media (%" PRId64 ").", offset, bc->end_offset);
+            }
+        }
+    } else {
+        // Regular file with unknown max size. Just read or write 128KB and see
+        // what happens. Reads will be truncated by the OS and writes will
+        // extend the file.
+        *count = BLOCK_CACHE_SEGMENT_SIZE;
+    }
+
+    return 0;
+}
+
 static int read_segment(struct block_cache *bc, struct block_cache_segment *seg, void *data)
 {
     if (is_trimmed(bc, seg->offset)) {
         // Trimmed, so we'd be reading uninitialized data (in theory), if we called pread.
         memset(data, 0, BLOCK_CACHE_SEGMENT_SIZE);
     } else {
-        ssize_t bytes_read = pread(bc->fd, data, BLOCK_CACHE_SEGMENT_SIZE, seg->offset);
+        size_t count;
+        OK_OR_RETURN(calculate_io_size(bc, seg->offset, &count));
+        ssize_t bytes_read = pread(bc->fd, data, count, seg->offset);
         if (bytes_read < 0) {
             ERR_RETURN("unexpected error reading %d bytes at offset %" PRId64 ": %s.\nPossible causes are that the destination is too small, the device (e.g., an SD card) is going bad, or the connection to it is flaky.",
-                    BLOCK_CACHE_SEGMENT_SIZE, seg->offset, strerror(errno));
+                    count, seg->offset, strerror(errno));
         } else if (bytes_read < BLOCK_CACHE_SEGMENT_SIZE) {
             // Didn't read enough bytes. This occurs if the destination media is
             // not a multiple of the segment size. Fill the remainder with zeros
@@ -201,25 +233,9 @@ static int verified_segment_write(struct block_cache *bc, volatile struct block_
 {
     off_t offset = seg->offset;
     const uint8_t *data = seg->data;
-    size_t count = BLOCK_CACHE_SEGMENT_SIZE;
 
-    // If there's a max destination size, then check if it's been hit or
-    // whether this should be a partial write.
-    if (bc->end_offset > 0) {
-        off_t last_offset = offset + count;
-        if (last_offset > bc->end_offset) {
-            // Fail if the write is completely past the end
-            if (offset > bc->end_offset)
-                ERR_RETURN("write failed at offset %" PRId64 " since past end of media (%" PRId64 ").", offset, bc->end_offset);
-
-            // Succeed if writing 0 bytes
-            if (offset == bc->end_offset)
-               return 0;
-
-            // Shrink the read/write count to support the partial block write
-            count = bc->end_offset - offset;
-        }
-    }
+    size_t count;
+    OK_OR_RETURN(calculate_io_size(bc, seg->offset, &count));
 
     if (bc->minimize_writes) {
         if (pread(bc->fd, temp, count, offset) == count &&

tests/199_partial_read.test

@@ -0,0 +1,117 @@
+#!/bin/sh
+
+#
+# Exercise reading a partial segment using require-partition on a silly small
+# disk.
+#
+
+. "$(cd "$(dirname "$0")" && pwd)/common.sh"
+
+cat >$CONFIG <<EOF
+
+# +-----------------------------+
+# | MBR                         |
+# +-----------------------------+
+# | p0: Boot (Simulated)        |
+# +-----------------------------+
+# | p1*: Rootfs A (Simulated)   |
+# +-----------------------------+
+# | p1*: Rootfs B (Simulated)   |
+# +-----------------------------+
+# | p2: Data (Simulated)        |
+# +-----------------------------+
+
+define(BOOT_PART_OFFSET, 2)
+define(BOOT_PART_COUNT, 4)
+define(ROOTFS_A_PART_OFFSET, 8)
+define(ROOTFS_A_PART_COUNT, 8)
+define(ROOTFS_B_PART_OFFSET, 16)
+define(ROOTFS_B_PART_COUNT, 8)
+define(APP_PART_OFFSET, 24)
+define(APP_PART_COUNT, 16)
+
+mbr mbr-a {
+    partition 0 {
+        block-offset = \${BOOT_PART_OFFSET}
+        block-count = \${BOOT_PART_COUNT}
+        type = 0xc # FAT32
+        boot = true
+    }
+    partition 1 {
+        block-offset = \${ROOTFS_A_PART_OFFSET}
+        block-count = \${ROOTFS_A_PART_COUNT}
+        type = 0x83 # Linux
+    }
+    partition 2 {
+        block-offset = \${APP_PART_OFFSET}
+        block-count = \${APP_PART_COUNT}
+        type = 0xc # FAT32
+    }
+    # partition 3 is unused
+}
+
+mbr mbr-b {
+    partition 0 {
+        block-offset = \${BOOT_PART_OFFSET}
+        block-count = \${BOOT_PART_COUNT}
+        type = 0xc # FAT32
+        boot = true
+    }
+    partition 1 {
+        block-offset = \${ROOTFS_B_PART_OFFSET}
+        block-count = \${ROOTFS_B_PART_COUNT}
+        type = 0x83 # Linux
+    }
+    partition 2 {
+        block-offset = \${APP_PART_OFFSET}
+        block-count = \${APP_PART_COUNT}
+        type = 0xc # FAT32
+    }
+    # partition 3 is unused
+}
+
+# This firmware task writes everything to the destination media
+task complete {
+    on-init {
+        mbr_write(mbr-a)
+    }
+}
+task upgrade.a {
+    # This task upgrades the A partition and runs when partition B
+    # is being used.
+    require-partition-offset(1, \${ROOTFS_B_PART_OFFSET})
+    on-finish { mbr_write(mbr-a) }
+}
+task upgrade.b {
+    # This task upgrades the B partition and runs when partition B
+    # is being used.
+    require-partition-offset(1, \${ROOTFS_A_PART_OFFSET})
+    on-finish { mbr_write(mbr-b) }
+}
+
+# This task is just needed to help support the unit test
+task dump_mbr_b {
+    on-init {
+        mbr_write(mbr-b)
+    }
+}
+EOF
+
+# Create the firmware file, then "burn it"
+$FWUP_CREATE -c -f $CONFIG -o $FWFILE
+$FWUP_APPLY -a -d $IMGFILE -i $FWFILE -t complete --max-size=64
+cp $IMGFILE $WORK/mbr_a.bin
+
+# Now upgrade the IMGFILE file
+# VERIFY_LAST_WRITE checks that MBR is written last
+VERIFY_LAST_WRITE=0 $FWUP_APPLY -a -d $IMGFILE -i $FWFILE -t upgrade --max-size=64
+
+VERIFY_SYSCALLS_CHECKPATH=$WORK/mbr_b.bin $FWUP_APPLY -a -d $WORK/mbr_b.bin -i $FWFILE -t dump_mbr_b --max-size=64
+cmp_bytes 512 $WORK/mbr_b.bin $IMGFILE 0 0         # Updated
+
+# Do it again
+VERIFY_LAST_WRITE=0 $FWUP_APPLY -a -d $IMGFILE -i $FWFILE -t upgrade --max-size=64
+cmp_bytes 512 $WORK/mbr_a.bin $IMGFILE 0 0         # Updated
+
+# Check that the verify logic works on this file
+$FWUP_VERIFY -V -i $FWFILE

tests/Makefile.am

@@ -198,6 +198,7 @@ TESTS = 001_simple_fw.test \
 	195_minimize_writes.test \
 	196_small_destination.test \
 	197_fractional_end_segment.test \
-	198_gpt_partial.test
+	198_gpt_partial.test \
+	199_partial_read.test
 
 EXTRA_DIST = $(TESTS) common.sh 1K.bin 1K-corrupt.bin 150K.bin