* [PULL 1/9] MAINTAINERS: Cc qemu-block mailing list
2020-07-14 13:58 [PULL 0/9] sdcard: Fix CVE-2020-13253 Philippe Mathieu-Daudé
@ 2020-07-14 13:58 ` 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é
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-14 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, qemu-block, Prasad J Pandit, Alexander Bulekov,
Paolo Bonzini, Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé <philmd@redhat.com>
We forgot to include the qemu-block mailing list while adding
this section in commit 076a0fc32a7. Fix this.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200630133912.9428-2-f4bug@amsat.org>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index fe8139f367..d9c71d0bb3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1674,6 +1674,7 @@ F: hw/ssi/xilinx_*
SD (Secure Card)
M: Philippe Mathieu-Daudé <f4bug@amsat.org>
+L: qemu-block@nongnu.org
S: Odd Fixes
F: include/hw/sd/sd*
F: hw/sd/core.c
--
2.21.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PULL 2/9] docs/orangepi: Add instructions for resizing SD image to power of two
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 ` 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é
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-14 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Philippe Mathieu-Daudé,
Prasad J Pandit, Alexander Bulekov, Niek Linnenbank,
Alistair Francis
From: Niek Linnenbank <nieklinnenbank@gmail.com>
SD cards need to have a size of a power of two.
Update the Orange Pi machine documentation to include
instructions for resizing downloaded images using the
qemu-img command.
Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20200712183708.15450-1-nieklinnenbank@gmail.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
docs/system/arm/orangepi.rst | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/docs/system/arm/orangepi.rst b/docs/system/arm/orangepi.rst
index c41adad488..6f23907fb6 100644
--- a/docs/system/arm/orangepi.rst
+++ b/docs/system/arm/orangepi.rst
@@ -127,6 +127,16 @@ can be downloaded from:
Alternatively, you can also choose to build you own image with buildroot
using the orangepi_pc_defconfig. Also see https://buildroot.org for more information.
+When using an image as an SD card, it must be resized to a power of two. This can be
+done with the qemu-img command. It is recommended to only increase the image size
+instead of shrinking it to a power of two, to avoid loss of data. For example,
+to prepare a downloaded Armbian image, first extract it and then increase
+its size to one gigabyte as follows:
+
+.. code-block:: bash
+
+ $ qemu-img resize Armbian_19.11.3_Orangepipc_bionic_current_5.3.9.img 1G
+
You can choose to attach the selected image either as an SD card or as USB mass storage.
For example, to boot using the Orange Pi PC Debian image on SD card, simply add the -sd
argument and provide the proper root= kernel parameter:
@@ -213,12 +223,12 @@ Next, unzip the NetBSD image and write the U-Boot binary including SPL using:
$ dd if=/path/to/u-boot-sunxi-with-spl.bin of=armv7.img bs=1024 seek=8 conv=notrunc
Finally, before starting the machine the SD image must be extended such
-that the NetBSD kernel will not conclude the NetBSD partition is larger than
-the emulated SD card:
+that the size of the SD image is a power of two and that the NetBSD kernel
+will not conclude the NetBSD partition is larger than the emulated SD card:
.. code-block:: bash
- $ dd if=/dev/zero bs=1M count=64 >> armv7.img
+ $ qemu-img resize armv7.img 2G
Start the machine using the following command:
--
2.21.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PULL 3/9] tests/acceptance/boot_linux: Tag tests using a SD card with 'device:sd'
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 ` 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é
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-14 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Philippe Mathieu-Daudé,
Wainer dos Santos Moschetta, Prasad J Pandit, Alexander Bulekov,
Alistair Francis, Cleber Rosa, Philippe Mathieu-Daudé
Avocado tags are handy to automatically select tests matching
the tags. Since these tests use a SD card, tag them.
We can run all the tests using a SD card at once with:
$ avocado --show=app run -t u-boot tests/acceptance/
$ AVOCADO_ALLOW_LARGE_STORAGE=ok \
avocado --show=app \
run -t device:sd tests/acceptance/
Fetching asset from tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd
Fetching asset from tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic
Fetching asset from tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9
(1/3) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd: PASS (19.56 s)
(2/3) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic: PASS (49.97 s)
(3/3) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9: PASS (20.06 s)
RESULTS : PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME : 90.02 s
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20200713183209.26308-4-f4bug@amsat.org>
---
tests/acceptance/boot_linux_console.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index 3d02519660..b7e8858c2d 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -620,6 +620,7 @@ def test_arm_orangepi_sd(self):
"""
:avocado: tags=arch:arm
:avocado: tags=machine:orangepi-pc
+ :avocado: tags=device:sd
"""
deb_url = ('https://apt.armbian.com/pool/main/l/'
'linux-4.20.7-sunxi/linux-image-dev-sunxi_5.75_armhf.deb')
@@ -669,6 +670,7 @@ def test_arm_orangepi_bionic(self):
"""
:avocado: tags=arch:arm
:avocado: tags=machine:orangepi-pc
+ :avocado: tags=device:sd
"""
# This test download a 196MB compressed image and expand it to 932MB...
@@ -710,6 +712,7 @@ def test_arm_orangepi_uboot_netbsd9(self):
"""
:avocado: tags=arch:arm
:avocado: tags=machine:orangepi-pc
+ :avocado: tags=device:sd
"""
# This test download a 304MB compressed image and expand it to 1.3GB...
deb_url = ('http://snapshot.debian.org/archive/debian/'
--
2.21.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PULL 4/9] tests/acceptance/boot_linux: Expand SD card image to power of 2
2020-07-14 13:58 [PULL 0/9] sdcard: Fix CVE-2020-13253 Philippe Mathieu-Daudé
` (2 preceding siblings ...)
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 ` Philippe Mathieu-Daudé
2020-07-14 13:58 ` [PULL 5/9] hw/sd/sdcard: Restrict Class 6 commands to SCSD cards Philippe Mathieu-Daudé
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-14 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Philippe Mathieu-Daudé,
Wainer dos Santos Moschetta, Prasad J Pandit, Alexander Bulekov,
Alistair Francis, Cleber Rosa, Philippe Mathieu-Daudé
In few commits we won't allow SD card images with invalid size
(not aligned to a power of 2). Prepare the tests: add the
pow2ceil() and image_pow2ceil_expand() methods and resize the
images (expanding) of the tests using SD cards.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20200713183209.26308-5-f4bug@amsat.org>
---
tests/acceptance/boot_linux_console.py | 31 ++++++++++++++++++--------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index b7e8858c2d..67c3b2f3d1 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -28,6 +28,22 @@
except CmdNotFoundError:
P7ZIP_AVAILABLE = False
+"""
+Round up to next power of 2
+"""
+def pow2ceil(x):
+ return 1 if x == 0 else 2**(x - 1).bit_length()
+
+"""
+Expand file size to next power of 2
+"""
+def image_pow2ceil_expand(path):
+ size = os.path.getsize(path)
+ size_aligned = pow2ceil(size)
+ if size != size_aligned:
+ with open(path, 'ab+') as fd:
+ fd.truncate(size_aligned)
+
class LinuxKernelTest(Test):
KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
@@ -636,6 +652,7 @@ def test_arm_orangepi_sd(self):
rootfs_path_xz = self.fetch_asset(rootfs_url, asset_hash=rootfs_hash)
rootfs_path = os.path.join(self.workdir, 'rootfs.cpio')
archive.lzma_uncompress(rootfs_path_xz, rootfs_path)
+ image_pow2ceil_expand(rootfs_path)
self.vm.set_console()
kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
@@ -673,7 +690,7 @@ def test_arm_orangepi_bionic(self):
:avocado: tags=device:sd
"""
- # This test download a 196MB compressed image and expand it to 932MB...
+ # This test download a 196MB compressed image and expand it to 1GB
image_url = ('https://dl.armbian.com/orangepipc/archive/'
'Armbian_19.11.3_Orangepipc_bionic_current_5.3.9.7z')
image_hash = '196a8ffb72b0123d92cea4a070894813d305c71e'
@@ -681,6 +698,7 @@ def test_arm_orangepi_bionic(self):
image_name = 'Armbian_19.11.3_Orangepipc_bionic_current_5.3.9.img'
image_path = os.path.join(self.workdir, image_name)
process.run("7z e -o%s %s" % (self.workdir, image_path_7z))
+ image_pow2ceil_expand(image_path)
self.vm.set_console()
self.vm.add_args('-drive', 'file=' + image_path + ',if=sd,format=raw',
@@ -714,7 +732,7 @@ def test_arm_orangepi_uboot_netbsd9(self):
:avocado: tags=machine:orangepi-pc
:avocado: tags=device:sd
"""
- # This test download a 304MB compressed image and expand it to 1.3GB...
+ # This test download a 304MB compressed image and expand it to 2GB
deb_url = ('http://snapshot.debian.org/archive/debian/'
'20200108T145233Z/pool/main/u/u-boot/'
'u-boot-sunxi_2020.01%2Bdfsg-1_armhf.deb')
@@ -731,8 +749,9 @@ def test_arm_orangepi_uboot_netbsd9(self):
image_hash = '2babb29d36d8360adcb39c09e31060945259917a'
image_path_gz = self.fetch_asset(image_url, asset_hash=image_hash)
image_path = os.path.join(self.workdir, 'armv7.img')
- image_drive_args = 'if=sd,format=raw,snapshot=on,file=' + image_path
archive.gzip_uncompress(image_path_gz, image_path)
+ image_pow2ceil_expand(image_path)
+ image_drive_args = 'if=sd,format=raw,snapshot=on,file=' + image_path
# dd if=u-boot-sunxi-with-spl.bin of=armv7.img bs=1K seek=8 conv=notrunc
with open(uboot_path, 'rb') as f_in:
@@ -740,12 +759,6 @@ def test_arm_orangepi_uboot_netbsd9(self):
f_out.seek(8 * 1024)
shutil.copyfileobj(f_in, f_out)
- # Extend image, to avoid that NetBSD thinks the partition
- # inside the image is larger than device size itself
- f_out.seek(0, 2)
- f_out.seek(64 * 1024 * 1024, 1)
- f_out.write(bytearray([0x00]))
-
self.vm.set_console()
self.vm.add_args('-nic', 'user',
'-drive', image_drive_args,
--
2.21.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PULL 5/9] hw/sd/sdcard: Restrict Class 6 commands to SCSD cards
2020-07-14 13:58 [PULL 0/9] sdcard: Fix CVE-2020-13253 Philippe Mathieu-Daudé
` (3 preceding siblings ...)
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 ` Philippe Mathieu-Daudé
2020-07-14 13:58 ` [PULL 6/9] hw/sd/sdcard: Simplify realize() a bit Philippe Mathieu-Daudé
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-14 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, qemu-block, qemu-stable,
Philippe Mathieu-Daudé,
Alexander Bulekov, Alistair Francis, Prasad J Pandit,
Philippe Mathieu-Daudé
Only SCSD cards support Class 6 (Block Oriented Write Protection)
commands.
"SD Specifications Part 1 Physical Layer Simplified Spec. v3.01"
4.3.14 Command Functional Difference in Card Capacity Types
* Write Protected Group
SDHC and SDXC do not support write-protected groups. Issuing
CMD28, CMD29 and CMD30 generates the ILLEGAL_COMMAND error.
Cc: qemu-stable@nongnu.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20200630133912.9428-7-f4bug@amsat.org>
---
hw/sd/sd.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 5137168d66..1cc16bfd31 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -920,6 +920,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
sd->multi_blk_cnt = 0;
}
+ if (sd_cmd_class[req.cmd] == 6 && FIELD_EX32(sd->ocr, OCR, CARD_CAPACITY)) {
+ /* Only Standard Capacity cards support class 6 commands */
+ return sd_illegal;
+ }
+
switch (req.cmd) {
/* Basic commands (Class 0 and Class 1) */
case 0: /* CMD0: GO_IDLE_STATE */
--
2.21.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PULL 6/9] hw/sd/sdcard: Simplify realize() a bit
2020-07-14 13:58 [PULL 0/9] sdcard: Fix CVE-2020-13253 Philippe Mathieu-Daudé
` (4 preceding siblings ...)
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 ` Philippe Mathieu-Daudé
2020-07-14 13:58 ` [PULL 7/9] hw/sd/sdcard: Do not allow invalid SD card sizes Philippe Mathieu-Daudé
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-14 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, qemu-block, Philippe Mathieu-Daudé,
Prasad J Pandit, Alexander Bulekov, Alistair Francis,
Philippe Mathieu-Daudé
We don't need to check if sd->blk is set twice.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20200630133912.9428-18-f4bug@amsat.org>
---
hw/sd/sd.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 1cc16bfd31..edd60a09c0 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2105,12 +2105,12 @@ static void sd_realize(DeviceState *dev, Error **errp)
return;
}
- if (sd->blk && blk_is_read_only(sd->blk)) {
- error_setg(errp, "Cannot use read-only drive as SD card");
- return;
- }
-
if (sd->blk) {
+ if (blk_is_read_only(sd->blk)) {
+ error_setg(errp, "Cannot use read-only drive as SD card");
+ return;
+ }
+
ret = blk_set_perm(sd->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE,
BLK_PERM_ALL, errp);
if (ret < 0) {
--
2.21.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PULL 7/9] hw/sd/sdcard: Do not allow invalid SD card sizes
2020-07-14 13:58 [PULL 0/9] sdcard: Fix CVE-2020-13253 Philippe Mathieu-Daudé
` (5 preceding siblings ...)
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é
2020-07-14 13:58 ` [PULL 8/9] hw/sd/sdcard: Update coding style to make checkpatch.pl happy Philippe Mathieu-Daudé
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-14 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, qemu-block, qemu-stable,
Philippe Mathieu-Daudé,
Alexander Bulekov, Alistair Francis, Prasad J Pandit,
Philippe Mathieu-Daudé
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PULL 8/9] hw/sd/sdcard: Update coding style to make checkpatch.pl happy
2020-07-14 13:58 [PULL 0/9] sdcard: Fix CVE-2020-13253 Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2020-07-14 13:58 ` [PULL 7/9] hw/sd/sdcard: Do not allow invalid SD card sizes Philippe Mathieu-Daudé
@ 2020-07-14 13:58 ` 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
9 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-14 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, qemu-block, Philippe Mathieu-Daudé,
Prasad J Pandit, Alexander Bulekov, Alistair Francis,
Philippe Mathieu-Daudé
To make the next commit easier to review, clean this code first.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200630133912.9428-3-f4bug@amsat.org>
---
hw/sd/sd.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 76d68359a4..f4f76f8fd2 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1175,8 +1175,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
sd->data_start = addr;
sd->data_offset = 0;
- if (sd->data_start + sd->blk_len > sd->size)
+ if (sd->data_start + sd->blk_len > sd->size) {
sd->card_status |= ADDRESS_ERROR;
+ }
return sd_r1;
default:
@@ -1191,8 +1192,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
sd->data_start = addr;
sd->data_offset = 0;
- if (sd->data_start + sd->blk_len > sd->size)
+ if (sd->data_start + sd->blk_len > sd->size) {
sd->card_status |= ADDRESS_ERROR;
+ }
return sd_r1;
default:
@@ -1237,12 +1239,15 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
sd->data_offset = 0;
sd->blk_written = 0;
- if (sd->data_start + sd->blk_len > sd->size)
+ if (sd->data_start + sd->blk_len > sd->size) {
sd->card_status |= ADDRESS_ERROR;
- if (sd_wp_addr(sd, sd->data_start))
+ }
+ if (sd_wp_addr(sd, sd->data_start)) {
sd->card_status |= WP_VIOLATION;
- if (sd->csd[14] & 0x30)
+ }
+ if (sd->csd[14] & 0x30) {
sd->card_status |= WP_VIOLATION;
+ }
return sd_r1;
default:
@@ -1261,12 +1266,15 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
sd->data_offset = 0;
sd->blk_written = 0;
- if (sd->data_start + sd->blk_len > sd->size)
+ if (sd->data_start + sd->blk_len > sd->size) {
sd->card_status |= ADDRESS_ERROR;
- if (sd_wp_addr(sd, sd->data_start))
+ }
+ if (sd_wp_addr(sd, sd->data_start)) {
sd->card_status |= WP_VIOLATION;
- if (sd->csd[14] & 0x30)
+ }
+ if (sd->csd[14] & 0x30) {
sd->card_status |= WP_VIOLATION;
+ }
return sd_r1;
default:
--
2.21.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PULL 9/9] hw/sd/sdcard: Do not switch to ReceivingData if address is invalid
2020-07-14 13:58 [PULL 0/9] sdcard: Fix CVE-2020-13253 Philippe Mathieu-Daudé
` (7 preceding siblings ...)
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 ` Philippe Mathieu-Daudé
2020-07-15 11:20 ` [PULL 0/9] sdcard: Fix CVE-2020-13253 Peter Maydell
9 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-14 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, qemu-block, qemu-stable,
Philippe Mathieu-Daudé,
Alexander Bulekov, Alistair Francis, Prasad J Pandit,
Philippe Mathieu-Daudé
Only move the state machine to ReceivingData if there is no
pending error. This avoids later OOB access while processing
commands queued.
"SD Specifications Part 1 Physical Layer Simplified Spec. v3.01"
4.3.3 Data Read
Read command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR
occurred and no data transfer is performed.
4.3.4 Data Write
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().
Fixes: CVE-2020-13253
Cc: qemu-stable@nongnu.org
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Buglink: https://bugs.launchpad.net/qemu/+bug/1880822
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20200630133912.9428-6-f4bug@amsat.org>
---
hw/sd/sd.c | 38 ++++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index f4f76f8fd2..fad9cf1ee7 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1171,13 +1171,15 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
case 17: /* CMD17: READ_SINGLE_BLOCK */
switch (sd->state) {
case sd_transfer_state:
+
+ if (addr + sd->blk_len > sd->size) {
+ sd->card_status |= ADDRESS_ERROR;
+ return sd_r1;
+ }
+
sd->state = sd_sendingdata_state;
sd->data_start = addr;
sd->data_offset = 0;
-
- if (sd->data_start + sd->blk_len > sd->size) {
- sd->card_status |= ADDRESS_ERROR;
- }
return sd_r1;
default:
@@ -1188,13 +1190,15 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
case 18: /* CMD18: READ_MULTIPLE_BLOCK */
switch (sd->state) {
case sd_transfer_state:
+
+ if (addr + sd->blk_len > sd->size) {
+ sd->card_status |= ADDRESS_ERROR;
+ return sd_r1;
+ }
+
sd->state = sd_sendingdata_state;
sd->data_start = addr;
sd->data_offset = 0;
-
- if (sd->data_start + sd->blk_len > sd->size) {
- sd->card_status |= ADDRESS_ERROR;
- }
return sd_r1;
default:
@@ -1234,14 +1238,17 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
/* Writing in SPI mode not implemented. */
if (sd->spi)
break;
+
+ if (addr + sd->blk_len > sd->size) {
+ sd->card_status |= ADDRESS_ERROR;
+ return sd_r1;
+ }
+
sd->state = sd_receivingdata_state;
sd->data_start = addr;
sd->data_offset = 0;
sd->blk_written = 0;
- if (sd->data_start + sd->blk_len > sd->size) {
- sd->card_status |= ADDRESS_ERROR;
- }
if (sd_wp_addr(sd, sd->data_start)) {
sd->card_status |= WP_VIOLATION;
}
@@ -1261,14 +1268,17 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
/* Writing in SPI mode not implemented. */
if (sd->spi)
break;
+
+ if (addr + sd->blk_len > sd->size) {
+ sd->card_status |= ADDRESS_ERROR;
+ return sd_r1;
+ }
+
sd->state = sd_receivingdata_state;
sd->data_start = addr;
sd->data_offset = 0;
sd->blk_written = 0;
- if (sd->data_start + sd->blk_len > sd->size) {
- sd->card_status |= ADDRESS_ERROR;
- }
if (sd_wp_addr(sd, sd->data_start)) {
sd->card_status |= WP_VIOLATION;
}
--
2.21.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PULL 0/9] sdcard: Fix CVE-2020-13253
2020-07-14 13:58 [PULL 0/9] sdcard: Fix CVE-2020-13253 Philippe Mathieu-Daudé
` (8 preceding siblings ...)
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 ` Peter Maydell
9 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2020-07-15 11:20 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Alexander Bulekov, QEMU Developers, Qemu-block, Prasad J Pandit
On Tue, 14 Jul 2020 at 15:00, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The following changes since commit 20c1df5476e1e9b5d3f5b94f9f3ce01d21f14c46:
>
> Merge remote-tracking branch 'remotes/kraxel/tags/fixes-20200713-pull-reque=
> st' into staging (2020-07-13 16:58:44 +0100)
>
> are available in the Git repository at:
>
> https://gitlab.com/philmd/qemu.git tags/sdcard-CVE-2020-13253-pull-request
>
> for you to fetch changes up to 790762e5487114341cccc5bffcec4cb3c022c3cd:
>
> hw/sd/sdcard: Do not switch to ReceivingData if address is invalid (2020-07=
> -14 15:46:14 +0200)
>
> ----------------------------------------------------------------
> Fix CVE-2020-13253
>
> By using invalidated address, guest can do out-of-bounds accesses.
> These patches fix the issue by only allowing SD card image sizes
> power of 2, and not switching to SEND_DATA state when the address
> is invalid (out of range).
>
> This issue was found using QEMU fuzzing mode (using --enable-fuzzing,
> see docs/devel/fuzzing.txt) and reported by Alexander Bulekov.
>
> Reproducer:
> https://bugs.launchpad.net/qemu/+bug/1880822/comments/1
>
> CI jobs results:
> . https://cirrus-ci.com/build/5157142548185088
> . https://gitlab.com/philmd/qemu/-/pipelines/166381731
> . https://travis-ci.org/github/philmd/qemu/builds/707956535
> ----------------------------------------------------------------
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread