On 2/16/21 4:46 AM, Bin Meng wrote:
> At the end of sdhci_send_command(), it starts a data transfer if the
> command register indicates data is associated. But the data transfer
> should only be initiated when the command execution has succeeded.
>
> With this fix, the following reproducer:
>
> outl 0xcf8 0x80001810
> outl 0xcfc 0xe1068000
> outl 0xcf8 0x80001804
> outw 0xcfc 0x7
> write 0xe106802c 0x1 0x0f
> write 0xe1068004 0xc 0x2801d10101fffffbff28a384
> write 0xe106800c 0x1f
> 0x9dacbbcad9e8f7061524334251606f7e8d9cabbac9d8e7f60514233241505f
> write 0xe1068003 0x28
> 0x80d000251480d000252280d000253080d000253e80d000254c80d000255a80d000256880d0002576
> write 0xe1068003 0x1 0xfe
>
> cannot be reproduced with the following QEMU command line:
>
> $ qemu-system-x86_64 -nographic -M pc-q35-5.0 \
> -device sdhci-pci,sd-spec-version=3 \
> -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive \
> -device sd-card,drive=mydrive \
> -monitor none -serial none -qtest stdio
Can you directly add the reproducer in tests/qtest/fuzz-sdhci-test.c
instead, similarly to tests/qtest/fuzz-test.c?
> Cc: qemu-sta...@nongnu.org
> Fixes: CVE-2020-17380
> Fixes: CVE-2020-25085
> Fixes: CVE-2021-3409
> Fixes: d7dfca0807a0 ("hw/sdhci: introduce standard SD host controller")
> Reported-by: Alexander Bulekov <alx...@bu.edu>
> Reported-by: Cornelius Aschermann (Ruhr-University Bochum)
> Reported-by: Muhammad Ramdhan
> Reported-by: Sergej Schumilo (Ruhr-University Bochum)
> Reported-by: Simon Wrner (Ruhr-University Bochum)
> Buglink: https://bugs.launchpad.net/qemu/+bug/1892960
> Buglink: https://bugs.launchpad.net/qemu/+bug/1909418
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1928146
> Signed-off-by: Bin Meng <bmeng...@gmail.com>
> Acked-by: Alistair Francis <alistair.fran...@wdc.com>
> Tested-by: Alexander Bulekov <alx...@bu.edu>
> Tested-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> ---
>
> (no changes since v1)
>
> hw/sd/sdhci.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 8ffa539..1c5ab26 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -326,6 +326,7 @@ static void sdhci_send_command(SDHCIState *s)
> SDRequest request;
> uint8_t response[16];
> int rlen;
> + bool timeout = false;
>
> s->errintsts = 0;
> s->acmd12errsts = 0;
> @@ -349,6 +350,7 @@ static void sdhci_send_command(SDHCIState *s)
> trace_sdhci_response16(s->rspreg[3], s->rspreg[2],
> s->rspreg[1], s->rspreg[0]);
> } else {
> + timeout = true;
> trace_sdhci_error("timeout waiting for command response");
> if (s->errintstsen & SDHC_EISEN_CMDTIMEOUT) {
> s->errintsts |= SDHC_EIS_CMDTIMEOUT;
> @@ -369,7 +371,7 @@ static void sdhci_send_command(SDHCIState *s)
>
> sdhci_update_irq(s);
>
> - if (s->blksize && (s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
> + if (!timeout && s->blksize && (s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
No need to add 'timeout':
if ((s->errintsts & SDHC_EIS_CMDTIMEOUT)
&& s->blksize
&& (s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
> s->data_count = 0;
> sdhci_data_transfer(s);
> }
>
