[Qemu-devel] [PATCH v7] pflash: Require backend size to match device, improve errors

[Markus Armbruster <armbru@redhat.com>]

From: Alex Bennée <alex.bennee@linaro.org>

We reject undersized backends with a rather enigmatic "failed to read
the initial flash content" error.

We happily accept oversized images, ignoring their tail.  Throwing
away parts of firmware that way is pretty much certain to end in an
even more enigmatic failure to boot.

Require the backend's size to match the device's size exactly.  Report
mismatch as "device requires N bytes, block backend 'NAME' provides M
bytes".

Improve the error for actual read failures to "can't read initial
flash content from block backend 'NAME'.

We disabled code to limit device sizes to 8, 16, 32 or 64MiB more than
a decade ago in commit 95d1f3edd5e and c8b153d7949, v0.9.1.  Bury.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/block/pflash_cfi01.c | 31 ++++++++++++++++++++++---------
 hw/block/pflash_cfi02.c | 31 +++++++++++++++++++++++--------
 2 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 9d1c356eb6..2e3a05c0b3 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -730,13 +730,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     }
     device_len = sector_len_per_device * blocks_per_device;
 
-    /* XXX: to be fixed */
-#if 0
-    if (total_len != (8 * 1024 * 1024) && total_len != (16 * 1024 * 1024) &&
-        total_len != (32 * 1024 * 1024) && total_len != (64 * 1024 * 1024))
-        return NULL;
-#endif
-
     memory_region_init_rom_device(
         &pfl->mem, OBJECT(dev),
         &pflash_cfi01_ops,
@@ -763,12 +756,32 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     }
 
     if (pfl->blk) {
+        /*
+         * Validate the backing store is the right size for pflash
+         * devices. If the user supplies a larger file we ignore the
+         * tail.
+         */
+        int64_t backing_len = blk_getlength(pfl->blk);
+
+        if (backing_len < 0) {
+            error_setg(errp, "can't get size of block backend '%s'",
+                       blk_name(pfl->blk));
+            return;
+        }
+        if (backing_len != total_len) {
+            error_setg(errp, "device requires %" PRIu64 " bytes, "
+                       "block backend '%s' provides %" PRIu64 " bytes",
+                       total_len, blk_name(pfl->blk), backing_len);
+            return;
+        }
+
         /* read the initial flash content */
         ret = blk_pread(pfl->blk, 0, pfl->storage, total_len);
-
         if (ret < 0) {
             vmstate_unregister_ram(&pfl->mem, DEVICE(pfl));
-            error_setg(errp, "failed to read the initial flash content");
+            error_setg(errp, "can't read initial flash content"
+                       " from block backend '%s'",
+                       blk_name(pfl->blk));
             return;
         }
     }
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 33779ce807..45b88d65d8 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -550,12 +550,6 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
     }
 
     chip_len = pfl->sector_len * pfl->nb_blocs;
-    /* XXX: to be fixed */
-#if 0
-    if (total_len != (8 * 1024 * 1024) && total_len != (16 * 1024 * 1024) &&
-        total_len != (32 * 1024 * 1024) && total_len != (64 * 1024 * 1024))
-        return NULL;
-#endif
 
     memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl), pfl->be ?
                                   &pflash_cfi02_ops_be : &pflash_cfi02_ops_le,
@@ -581,11 +575,32 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
     }
 
     if (pfl->blk) {
+        /*
+         * Validate the backing store is the right size for pflash
+         * devices. If the user supplies a larger file we ignore the
+         * tail.
+         */
+        int64_t backing_len = blk_getlength(pfl->blk);
+
+        if (backing_len < 0) {
+            error_setg(errp, "can't get size of block backend '%s'",
+                       blk_name(pfl->blk));
+            return;
+        }
+        if (backing_len != chip_len) {
+            error_setg(errp, "device requires %" PRIu32 " bytes, "
+                       "block backend '%s' provides %" PRIu64 " bytes",
+                       chip_len, blk_name(pfl->blk), backing_len);
+            return;
+        }
+
         /* read the initial flash content */
         ret = blk_pread(pfl->blk, 0, pfl->storage, chip_len);
         if (ret < 0) {
-            vmstate_unregister_ram(&pfl->orig_mem, DEVICE(pfl));
-            error_setg(errp, "failed to read the initial flash content");
+            vmstate_unregister_ram(&pfl->mem, DEVICE(pfl));
+            error_setg(errp, "can't read initial flash content"
+                       " from block backend '%s'",
+                       blk_name(pfl->blk));
             return;
         }
     }
-- 
2.17.2