All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: qemu-devel@nongnu.org
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	qemu-block@nongnu.org, qemu-stable@nongnu.org,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Alexander Bulekov" <alxndr@bu.edu>,
	"Alistair Francis" <alistair.francis@wdc.com>,
	"Prasad J Pandit" <ppandit@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: [PULL 7/9] hw/sd/sdcard: Do not allow invalid SD card sizes
Date: Tue, 14 Jul 2020 15:58:12 +0200	[thread overview]
Message-ID: <20200714135814.19910-8-f4bug@amsat.org> (raw)
In-Reply-To: <20200714135814.19910-1-f4bug@amsat.org>

QEMU allows to create SD card with unrealistic sizes. This could
work, but some guests (at least Linux) consider sizes that are not
a power of 2 as a firmware bug and fix the card size to the next
power of 2.

While the possibility to use small SD card images has been seen as
a feature, it became a bug with CVE-2020-13253, where the guest is
able to do OOB read/write accesses past the image size end.

In a pair of commits we will fix CVE-2020-13253 as:

    Read command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR
    occurred and no data transfer is performed.

    Write command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR
    occurred and no data transfer is performed.

    WP_VIOLATION errors are not modified: the error bit is set, we
    stay in receive-data state, wait for a stop command. All further
    data transfer is ignored. See the check on sd->card_status at the
    beginning of sd_read_data() and sd_write_data().

While this is the correct behavior, in case QEMU create smaller SD
cards, guests still try to access past the image size end, and QEMU
considers this is an invalid address, thus "all further data transfer
is ignored". This is wrong and make the guest looping until
eventually timeouts.

Fix by not allowing invalid SD card sizes (suggesting the expected
size as a hint):

  $ qemu-system-arm -M orangepi-pc -drive file=rootfs.ext2,if=sd,format=raw
  qemu-system-arm: Invalid SD card size: 60 MiB
  SD card size has to be a power of 2, e.g. 64 MiB.
  You can resize disk images with 'qemu-img resize <imagefile> <new-size>'
  (note that this will lose data if you make the image smaller than it currently is).

Cc: qemu-stable@nongnu.org
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20200713183209.26308-8-f4bug@amsat.org>
---
 hw/sd/sd.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index edd60a09c0..76d68359a4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -32,6 +32,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/units.h"
+#include "qemu/cutils.h"
 #include "hw/irq.h"
 #include "hw/registerfields.h"
 #include "sysemu/block-backend.h"
@@ -2106,11 +2107,35 @@ static void sd_realize(DeviceState *dev, Error **errp)
     }
 
     if (sd->blk) {
+        int64_t blk_size;
+
         if (blk_is_read_only(sd->blk)) {
             error_setg(errp, "Cannot use read-only drive as SD card");
             return;
         }
 
+        blk_size = blk_getlength(sd->blk);
+        if (blk_size > 0 && !is_power_of_2(blk_size)) {
+            int64_t blk_size_aligned = pow2ceil(blk_size);
+            char *blk_size_str;
+
+            blk_size_str = size_to_str(blk_size);
+            error_setg(errp, "Invalid SD card size: %s", blk_size_str);
+            g_free(blk_size_str);
+
+            blk_size_str = size_to_str(blk_size_aligned);
+            error_append_hint(errp,
+                              "SD card size has to be a power of 2, e.g. %s.\n"
+                              "You can resize disk images with"
+                              " 'qemu-img resize <imagefile> <new-size>'\n"
+                              "(note that this will lose data if you make the"
+                              " image smaller than it currently is).\n",
+                              blk_size_str);
+            g_free(blk_size_str);
+
+            return;
+        }
+
         ret = blk_set_perm(sd->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE,
                            BLK_PERM_ALL, errp);
         if (ret < 0) {
-- 
2.21.3



  parent reply	other threads:[~2020-07-14 14:01 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-14 13:58 [PULL 0/9] sdcard: Fix CVE-2020-13253 Philippe Mathieu-Daudé
2020-07-14 13:58 ` [PULL 1/9] MAINTAINERS: Cc qemu-block mailing list Philippe Mathieu-Daudé
2020-07-14 13:58 ` [PULL 2/9] docs/orangepi: Add instructions for resizing SD image to power of two Philippe Mathieu-Daudé
2020-07-14 13:58 ` [PULL 3/9] tests/acceptance/boot_linux: Tag tests using a SD card with 'device:sd' Philippe Mathieu-Daudé
2020-07-14 13:58 ` [PULL 4/9] tests/acceptance/boot_linux: Expand SD card image to power of 2 Philippe Mathieu-Daudé
2020-07-14 13:58 ` [PULL 5/9] hw/sd/sdcard: Restrict Class 6 commands to SCSD cards Philippe Mathieu-Daudé
2020-07-14 13:58 ` [PULL 6/9] hw/sd/sdcard: Simplify realize() a bit Philippe Mathieu-Daudé
2020-07-14 13:58 ` Philippe Mathieu-Daudé [this message]
2020-07-14 13:58 ` [PULL 8/9] hw/sd/sdcard: Update coding style to make checkpatch.pl happy Philippe Mathieu-Daudé
2020-07-14 13:58 ` [PULL 9/9] hw/sd/sdcard: Do not switch to ReceivingData if address is invalid Philippe Mathieu-Daudé
2020-07-15 11:20 ` [PULL 0/9] sdcard: Fix CVE-2020-13253 Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200714135814.19910-8-f4bug@amsat.org \
    --to=f4bug@amsat.org \
    --cc=alistair.francis@wdc.com \
    --cc=alxndr@bu.edu \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@redhat.com \
    --cc=ppandit@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.