From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: qemu-devel@nongnu.org
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
qemu-block@nongnu.org,
"Alistair Francis" <alistair@alistair23.me>,
"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
"Wainer dos Santos Moschetta" <wainersm@redhat.com>,
"Niek Linnenbank" <nieklinnenbank@gmail.com>,
"Cleber Rosa" <crosa@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes
Date: Tue, 7 Jul 2020 15:21:16 +0200 [thread overview]
Message-ID: <20200707132116.26207-3-f4bug@amsat.org> (raw)
In-Reply-To: <20200707132116.26207-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.
Before CVE-2020-13253 fix, this would allow OOB read/write accesses
past the image size end.
CVE-2020-13253 has been fixed 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 (expecting at least 64 MiB)
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sd.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index cb81487e5c..c45106b78e 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"
@@ -2130,11 +2131,26 @@ 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 = size_to_str(blk_size);
+ char *blk_size_aligned_str = size_to_str(blk_size_aligned);
+
+ error_setg(errp, "Invalid SD card size: %s (expecting at least %s)",
+ blk_size_str, blk_size_aligned_str);
+ g_free(blk_size_str);
+ g_free(blk_size_aligned_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
next prev parent reply other threads:[~2020-07-07 13:22 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-07 13:21 [PATCH 0/2] hw/sd/sdcard: Fix CVE-2020-13253 (Do not allow invalid SD card sizes) Philippe Mathieu-Daudé
2020-07-07 13:21 ` [PATCH 1/2] tests/acceptance/boot_linux: Truncate SD card image to power of 2 Philippe Mathieu-Daudé
2020-07-07 15:53 ` Alistair Francis
2020-07-07 16:10 ` Philippe Mathieu-Daudé
2020-07-12 18:43 ` Niek Linnenbank
2020-07-07 13:21 ` Philippe Mathieu-Daudé [this message]
2020-07-07 15:55 ` [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes Alistair Francis
2020-07-07 16:06 ` Peter Maydell
2020-07-07 16:11 ` Philippe Mathieu-Daudé
2020-07-07 20:29 ` Niek Linnenbank
2020-07-09 13:56 ` Philippe Mathieu-Daudé
2020-07-09 14:15 ` Peter Maydell
2020-07-09 14:35 ` Philippe Mathieu-Daudé
2020-07-09 16:17 ` Alistair Francis
2020-07-10 9:54 ` Peter Maydell
2020-07-09 17:56 ` Niek Linnenbank
2020-07-10 9:58 ` Kevin Wolf
2020-07-10 9:59 ` Peter Maydell
2020-07-10 12:07 ` Kevin Wolf
2020-07-10 12:30 ` Peter Maydell
2020-07-09 17:53 ` Niek Linnenbank
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=20200707132116.26207-3-f4bug@amsat.org \
--to=f4bug@amsat.org \
--cc=alistair@alistair23.me \
--cc=crosa@redhat.com \
--cc=nieklinnenbank@gmail.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=philmd@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=wainersm@redhat.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).