On Mon, Jul 13, 2020 at 11:33 AM Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
>
> 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).
>
> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
Alistair
> ---
> Since v1:
> Addressed Alistair & Peter comments (error_append_hint message)
> ---
> hw/sd/sd.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index edd60a09c0..5ab945dade 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
>
>
