[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes |
Date: |
Tue, 7 Jul 2020 18:11:43 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 7/7/20 6:06 PM, Peter Maydell wrote:
> On Tue, 7 Jul 2020 at 17:04, Alistair Francis <alistair23@gmail.com> wrote:
>>
>> On Tue, Jul 7, 2020 at 6:22 AM Philippe Mathieu-Daudé <f4bug@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.
>>>
>>> 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);
>>
>> Should we print that we expect a power of 2? This isn't always obvious
>> from the message.
>
> Mmm, I was thinking that. Perhaps
> "expecting a power of 2, e.g. %s"
> ?
OK, thanks guys!
>
> thanks
> -- PMM
>
- [PATCH 0/2] hw/sd/sdcard: Fix CVE-2020-13253 (Do not allow invalid SD card sizes), Philippe Mathieu-Daudé, 2020/07/07
- [PATCH 1/2] tests/acceptance/boot_linux: Truncate SD card image to power of 2, Philippe Mathieu-Daudé, 2020/07/07
- [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes, Philippe Mathieu-Daudé, 2020/07/07
- Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes, Alistair Francis, 2020/07/07
- Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes, Peter Maydell, 2020/07/07
- Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes,
Philippe Mathieu-Daudé <=
- Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes, Niek Linnenbank, 2020/07/07
- Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes, Philippe Mathieu-Daudé, 2020/07/09
- Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes, Peter Maydell, 2020/07/09
- Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes, Philippe Mathieu-Daudé, 2020/07/09
- Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes, Alistair Francis, 2020/07/09
- Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes, Peter Maydell, 2020/07/10
- Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes, Niek Linnenbank, 2020/07/09
- Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes, Kevin Wolf, 2020/07/10
- Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes, Peter Maydell, 2020/07/10
- Re: [PATCH 2/2] hw/sd/sdcard: Do not allow invalid SD card sizes, Kevin Wolf, 2020/07/10