Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
On Sun, Feb 14, 2021 at 1:56 PM Bin Meng wrote: > > Hi Alexander, > > On Fri, Feb 12, 2021 at 5:25 AM Alexander Bulekov wrote: > > > > On 210211 1154, Alexander Bulekov wrote: > > ... > > > I applied this along with <20210208193450.2689517-1-f4...@amsat.org> > > > "hw/sd/sdhci: Do not modify BlockSizeRegister if transaction in progress" > > > > > > I ran through the entire OSS-Fuzz corpus, and could not reproduce the > > > crash. > > > > > > Tested-by: Alexander Bulekov > > > > > Hi Bin, > > Phil explained to me that this patch should fix the problem independent > > of > > "hw/sd/sdhci: Do not modify BlockSizeRegister if transaction in progress" > > Yes, that's what I expect too. > > > > > With only this patch, there are still crashes. Here are three > > reproducers: > > > > Some of these are quite long, so here are pastebins for convenience: > > Repro 1: https://paste.debian.net/plain/1185137 > > Repro 2: https://paste.debian.net/plain/1185141 > > Repro 3: https://paste.debian.net/plain/1185136 > > I will take a look. I have figured out a fix and will send out for review and testing soon. Regards, Bin
Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
Hi Alexander, On Fri, Feb 12, 2021 at 5:25 AM Alexander Bulekov wrote: > > On 210211 1154, Alexander Bulekov wrote: > ... > > I applied this along with <20210208193450.2689517-1-f4...@amsat.org> > > "hw/sd/sdhci: Do not modify BlockSizeRegister if transaction in progress" > > > > I ran through the entire OSS-Fuzz corpus, and could not reproduce the > > crash. > > > > Tested-by: Alexander Bulekov > > > Hi Bin, > Phil explained to me that this patch should fix the problem independent > of > "hw/sd/sdhci: Do not modify BlockSizeRegister if transaction in progress" Yes, that's what I expect too. > > With only this patch, there are still crashes. Here are three > reproducers: > > Some of these are quite long, so here are pastebins for convenience: > Repro 1: https://paste.debian.net/plain/1185137 > Repro 2: https://paste.debian.net/plain/1185141 > Repro 3: https://paste.debian.net/plain/1185136 I will take a look. > > Just wget and pipe them into > ./qemu-system-i386 -display none -machine accel=qtest -nographic \ > -m 512M -nodefaults -device sdhci-pci,sd-spec-version=3 \ > -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive \ > -device sd-card,drive=mydrive -qtest stdio > > Repro 1 > cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest -nographic > \ > -m 512M -nodefaults -device sdhci-pci,sd-spec-version=3 \ > -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive \ > -device sd-card,drive=mydrive -qtest stdio > outl 0xcf8 0x80001010 > outl 0xcfc 0xfbefff00 > outl 0xcf8 0x80001001 > outl 0xcfc 0x0600 > write 0xfbefff2c 0x1 0x05 > write 0xfbefff0f 0x1 0x37 > write 0xfbefff0a 0x1 0x01 > write 0xfbefff0f 0x1 0x29 > write 0xfbefff0f 0x1 0x02 > write 0xfbefff0f 0x1 0x03 > write 0xfbefff04 0x1 0x01 > write 0xfbefff05 0x1 0x01 > write 0xfbefff07 0x1 0x02 > write 0xfbefff0c 0x1 0x33 > write 0xfbefff0e 0x1 0x20 > write 0xfbefff0f 0x1 0x00 > write 0xfbefff2a 0x1 0x01 > write 0xfbefff0c 0x1 0x00 > write 0xfbefff03 0x1 0x00 > write 0xfbefff05 0x1 0x00 > write 0xfbefff2a 0x1 0x02 > write 0xfbefff0c 0x1 0x32 > write 0xfbefff01 0x1 0x01 > write 0xfbefff02 0x1 0x01 > write 0xfbefff03 0x1 0x01 > EOF > > Stack Trace 1 > ==929953==ERROR: AddressSanitizer: heap-buffer-overflow on address > 0x61531880 at pc 0x564cf01ceae7 bp 0x7ffe17361e10 sp 0x7ffe173615d8 > READ of size 520027904 at 0x61531880 thread T0 > #0 0x564cf01ceae6 in __asan_memcpy > (/home/alxndr/Development/qemu/build/qemu-system-i386+0x2a8cae6) > #1 0x564cf19111a5 in flatview_write_continue > /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2781:13 > #2 0x564cf1906beb in flatview_write > /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2816:14 > #3 0x564cf1906beb in address_space_write > /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2908:18 > #4 0x564cf096348c in dma_memory_rw_relaxed > /home/alxndr/Development/qemu/include/sysemu/dma.h:88:12 > #5 0x564cf096348c in dma_memory_rw > /home/alxndr/Development/qemu/include/sysemu/dma.h:127:12 > #6 0x564cf096348c in dma_memory_write > /home/alxndr/Development/qemu/include/sysemu/dma.h:163:12 > #7 0x564cf096348c in sdhci_sdma_transfer_multi_blocks > /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:619:13 > #8 0x564cf097237d in sdhci_write > /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:1131:17 > #9 0x564cf154333c in memory_region_write_accessor > /home/alxndr/Development/qemu/build/../softmmu/memory.c:491:5 > > Repro 2 > > cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest -nographic > \ > -m 512M -nodefaults -device sdhci-pci,sd-spec-version=3 \ > -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive \ > -device sd-card,drive=mydrive -qtest stdio > outl 0xcf8 0x80001013 > outl 0xcfc 0x91 > outl 0xcf8 0x80001001 > outl 0xcfc 0x0600 > write 0x912c 0x1 0x05 > write 0x910f 0x1 0x37 > write 0x910a 0x1 0x01 > write 0x910f 0x1 0x29 > write 0x910f 0x1 0x02 > write 0x910f 0x1 0x03 > write 0x0 0x1 0x01 > write 0x8 0x1 0x01 > write 0x10 0x1 0x01 > write 0x18 0x1 0x01 > write 0x20 0x1 0x01 > write 0x28 0x1 0x01 > write 0x30 0x1 0x01 > write 0x38 0x1 0x01 > write 0x40 0x1 0x01 > write 0x48 0x1 0x01 > write 0x50 0x1 0x01 > write 0x58 0x1 0x01 > write 0x60 0x1 0x01 > write 0x68 0x1 0x01 > write 0x70 0x1 0x01 > write 0x9105 0x1 0x02 > write 0x9107 0x1 0x20 > write 0x78 0x1 0x01 > write 0x80 0x1 0x01 > write 0x88 0x1 0x01 > write 0x90 0x1 0x01 > write 0x98 0x1 0x01 > write 0xa0 0x1 0x01 > write 0xa8 0x1 0x01 > write 0xb0 0x1 0x01 > write 0xb8 0x1 0x01 > write 0xc0 0x1 0x01 > write 0x910e 0x1 0x21 > write 0x9128 0x1 0x10 > write 0x910c 0x1 0x01 > write 0x910f 0x1 0x06 > write 0xc8 0x1 0x01 > write 0xd0 0x1 0x01 > write 0xd8 0x1 0x01 > write 0xe0 0x1 0x01 > write 0xe8 0x1 0x01 > write 0xf0 0x1 0x01 > write 0xf8 0x1 0x01 > write 0x100 0x1 0x01 > write 0x108 0x1 0x01 > write 0x110 0x1 0x01 > write 0x118 0x1
Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
On Thu, Feb 11, 2021 at 8:48 PM Philippe Mathieu-Daudé wrote: > > On 2/11/21 9:52 AM, Mauro Matteo Cascella wrote: > > Hello, > > > > On Wed, Feb 10, 2021 at 11:27 PM Alistair Francis > > wrote: > >> > >> On Tue, Feb 9, 2021 at 2:55 AM Bin Meng wrote: > >>> > >>> At the end of sdhci_send_command(), it starts a data transfer if > >>> the command register indicates a data is associated. However the > >>> data transfer should only be initiated when the command execution > >>> has succeeded. > >> > >> Isn't this already fixed? > > The previous patch was enough to catch the previous reproducer, > but something changed elsewhere making the same reproducer crash > QEMU again... > > > It turned out the bug was still reproducible on master. I'm actually > > thinking of assigning a new CVE for this, to make it possible for > > distros to apply this fix. > > It sounds fair. Do you have an ETA for the new CVE? This is now CVE-2021-3409. RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1928146 -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
On 210211 1154, Alexander Bulekov wrote: ... > I applied this along with <20210208193450.2689517-1-f4...@amsat.org> > "hw/sd/sdhci: Do not modify BlockSizeRegister if transaction in progress" > > I ran through the entire OSS-Fuzz corpus, and could not reproduce the > crash. > > Tested-by: Alexander Bulekov > Hi Bin, Phil explained to me that this patch should fix the problem independent of "hw/sd/sdhci: Do not modify BlockSizeRegister if transaction in progress" With only this patch, there are still crashes. Here are three reproducers: Some of these are quite long, so here are pastebins for convenience: Repro 1: https://paste.debian.net/plain/1185137 Repro 2: https://paste.debian.net/plain/1185141 Repro 3: https://paste.debian.net/plain/1185136 Just wget and pipe them into ./qemu-system-i386 -display none -machine accel=qtest -nographic \ -m 512M -nodefaults -device sdhci-pci,sd-spec-version=3 \ -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive \ -device sd-card,drive=mydrive -qtest stdio Repro 1 cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest -nographic \ -m 512M -nodefaults -device sdhci-pci,sd-spec-version=3 \ -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive \ -device sd-card,drive=mydrive -qtest stdio outl 0xcf8 0x80001010 outl 0xcfc 0xfbefff00 outl 0xcf8 0x80001001 outl 0xcfc 0x0600 write 0xfbefff2c 0x1 0x05 write 0xfbefff0f 0x1 0x37 write 0xfbefff0a 0x1 0x01 write 0xfbefff0f 0x1 0x29 write 0xfbefff0f 0x1 0x02 write 0xfbefff0f 0x1 0x03 write 0xfbefff04 0x1 0x01 write 0xfbefff05 0x1 0x01 write 0xfbefff07 0x1 0x02 write 0xfbefff0c 0x1 0x33 write 0xfbefff0e 0x1 0x20 write 0xfbefff0f 0x1 0x00 write 0xfbefff2a 0x1 0x01 write 0xfbefff0c 0x1 0x00 write 0xfbefff03 0x1 0x00 write 0xfbefff05 0x1 0x00 write 0xfbefff2a 0x1 0x02 write 0xfbefff0c 0x1 0x32 write 0xfbefff01 0x1 0x01 write 0xfbefff02 0x1 0x01 write 0xfbefff03 0x1 0x01 EOF Stack Trace 1 ==929953==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61531880 at pc 0x564cf01ceae7 bp 0x7ffe17361e10 sp 0x7ffe173615d8 READ of size 520027904 at 0x61531880 thread T0 #0 0x564cf01ceae6 in __asan_memcpy (/home/alxndr/Development/qemu/build/qemu-system-i386+0x2a8cae6) #1 0x564cf19111a5 in flatview_write_continue /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2781:13 #2 0x564cf1906beb in flatview_write /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2816:14 #3 0x564cf1906beb in address_space_write /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2908:18 #4 0x564cf096348c in dma_memory_rw_relaxed /home/alxndr/Development/qemu/include/sysemu/dma.h:88:12 #5 0x564cf096348c in dma_memory_rw /home/alxndr/Development/qemu/include/sysemu/dma.h:127:12 #6 0x564cf096348c in dma_memory_write /home/alxndr/Development/qemu/include/sysemu/dma.h:163:12 #7 0x564cf096348c in sdhci_sdma_transfer_multi_blocks /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:619:13 #8 0x564cf097237d in sdhci_write /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:1131:17 #9 0x564cf154333c in memory_region_write_accessor /home/alxndr/Development/qemu/build/../softmmu/memory.c:491:5 Repro 2 cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest -nographic \ -m 512M -nodefaults -device sdhci-pci,sd-spec-version=3 \ -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive \ -device sd-card,drive=mydrive -qtest stdio outl 0xcf8 0x80001013 outl 0xcfc 0x91 outl 0xcf8 0x80001001 outl 0xcfc 0x0600 write 0x912c 0x1 0x05 write 0x910f 0x1 0x37 write 0x910a 0x1 0x01 write 0x910f 0x1 0x29 write 0x910f 0x1 0x02 write 0x910f 0x1 0x03 write 0x0 0x1 0x01 write 0x8 0x1 0x01 write 0x10 0x1 0x01 write 0x18 0x1 0x01 write 0x20 0x1 0x01 write 0x28 0x1 0x01 write 0x30 0x1 0x01 write 0x38 0x1 0x01 write 0x40 0x1 0x01 write 0x48 0x1 0x01 write 0x50 0x1 0x01 write 0x58 0x1 0x01 write 0x60 0x1 0x01 write 0x68 0x1 0x01 write 0x70 0x1 0x01 write 0x9105 0x1 0x02 write 0x9107 0x1 0x20 write 0x78 0x1 0x01 write 0x80 0x1 0x01 write 0x88 0x1 0x01 write 0x90 0x1 0x01 write 0x98 0x1 0x01 write 0xa0 0x1 0x01 write 0xa8 0x1 0x01 write 0xb0 0x1 0x01 write 0xb8 0x1 0x01 write 0xc0 0x1 0x01 write 0x910e 0x1 0x21 write 0x9128 0x1 0x10 write 0x910c 0x1 0x01 write 0x910f 0x1 0x06 write 0xc8 0x1 0x01 write 0xd0 0x1 0x01 write 0xd8 0x1 0x01 write 0xe0 0x1 0x01 write 0xe8 0x1 0x01 write 0xf0 0x1 0x01 write 0xf8 0x1 0x01 write 0x100 0x1 0x01 write 0x108 0x1 0x01 write 0x110 0x1 0x01 write 0x118 0x1 0x01 write 0x120 0x1 0x01 write 0x128 0x1 0x01 write 0x130 0x1 0x01 write 0x138 0x1 0x01 write 0x140 0x1 0x01 write 0x148 0x1 0x01 write 0x150 0x1 0x01 write 0x158 0x1 0x01 write 0x160 0x1 0x01 write 0x168 0x1 0x01 write 0x170 0x1 0x01 write 0x178 0x1 0x01 write 0x180 0x1 0x01 write 0x188 0x1 0x01 write 0x190 0x1 0x01 write 0x198 0x1 0x01 write 0x1a0 0x1 0x01 write 0x1a8 0x1 0x01 write 0x1b0 0x1 0x01 write 0x9
Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
On 2/11/21 9:52 AM, Mauro Matteo Cascella wrote: > Hello, > > On Wed, Feb 10, 2021 at 11:27 PM Alistair Francis > wrote: >> >> On Tue, Feb 9, 2021 at 2:55 AM Bin Meng wrote: >>> >>> At the end of sdhci_send_command(), it starts a data transfer if >>> the command register indicates a data is associated. However the >>> data transfer should only be initiated when the command execution >>> has succeeded. >>> >>> Cc: qemu-sta...@nongnu.org >>> Fixes: CVE-2020-17380 >>> Fixes: CVE-2020-25085 >>> Reported-by: Alexander Bulekov >>> Reported-by: Sergej Schumilo (Ruhr-University Bochum) >>> Reported-by: Cornelius Aschermann (Ruhr-University Bochum) >>> Reported-by: Simon Wrner (Ruhr-University Bochum) >>> Buglink: https://bugs.launchpad.net/qemu/+bug/1892960 >> >> Isn't this already fixed? The previous patch was enough to catch the previous reproducer, but something changed elsewhere making the same reproducer crash QEMU again... > It turned out the bug was still reproducible on master. I'm actually > thinking of assigning a new CVE for this, to make it possible for > distros to apply this fix. It sounds fair. Do you have an ETA for the new CVE? > -- > Mauro Matteo Cascella > Red Hat Product Security > PGP-Key ID: BB3410B0 > >
Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
On 210209 1854, Bin Meng wrote:
> At the end of sdhci_send_command(), it starts a data transfer if
> the command register indicates a data is associated. However the
> data transfer should only be initiated when the command execution
> has succeeded.
>
> Cc: qemu-sta...@nongnu.org
> Fixes: CVE-2020-17380
> Fixes: CVE-2020-25085
> Reported-by: Alexander Bulekov
> Reported-by: Sergej Schumilo (Ruhr-University Bochum)
> Reported-by: Cornelius Aschermann (Ruhr-University Bochum)
> Reported-by: Simon Wrner (Ruhr-University Bochum)
> Buglink: https://bugs.launchpad.net/qemu/+bug/1892960
> Signed-off-by: Bin Meng
I applied this along with <20210208193450.2689517-1-f4...@amsat.org>
"hw/sd/sdhci: Do not modify BlockSizeRegister if transaction in progress"
I ran through the entire OSS-Fuzz corpus, and could not reproduce the
crash.
Tested-by: Alexander Bulekov
Thanks
> ---
>
> 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..0450110 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 cmd_failure = 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 {
> +cmd_failure = 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 (!cmd_failure && s->blksize && (s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
> s->data_count = 0;
> sdhci_data_transfer(s);
> }
> --
> 2.7.4
>
Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
On 210209 1854, Bin Meng wrote:
> At the end of sdhci_send_command(), it starts a data transfer if
> the command register indicates a data is associated. However the
> data transfer should only be initiated when the command execution
> has succeeded.
>
> Cc: qemu-sta...@nongnu.org
> Fixes: CVE-2020-17380
> Fixes: CVE-2020-25085
> Reported-by: Alexander Bulekov
> Reported-by: Sergej Schumilo (Ruhr-University Bochum)
> Reported-by: Cornelius Aschermann (Ruhr-University Bochum)
> Reported-by: Simon Wrner (Ruhr-University Bochum)
Reported-by: Muhammad Ramdhan
(don't know how to get the email from a launchpad report)
and probably:
Buglink: https://bugs.launchpad.net/qemu/+bug/1909418
> Buglink: https://bugs.launchpad.net/qemu/+bug/1892960
> Signed-off-by: Bin Meng
> ---
>
> 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..0450110 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 cmd_failure = 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 {
> +cmd_failure = 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 (!cmd_failure && s->blksize && (s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
> s->data_count = 0;
> sdhci_data_transfer(s);
> }
> --
> 2.7.4
>
Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
Hello, On Wed, Feb 10, 2021 at 11:27 PM Alistair Francis wrote: > > On Tue, Feb 9, 2021 at 2:55 AM Bin Meng wrote: > > > > At the end of sdhci_send_command(), it starts a data transfer if > > the command register indicates a data is associated. However the > > data transfer should only be initiated when the command execution > > has succeeded. > > > > Cc: qemu-sta...@nongnu.org > > Fixes: CVE-2020-17380 > > Fixes: CVE-2020-25085 > > Reported-by: Alexander Bulekov > > Reported-by: Sergej Schumilo (Ruhr-University Bochum) > > Reported-by: Cornelius Aschermann (Ruhr-University Bochum) > > Reported-by: Simon Wrner (Ruhr-University Bochum) > > Buglink: https://bugs.launchpad.net/qemu/+bug/1892960 > > Isn't this already fixed? > It turned out the bug was still reproducible on master. I'm actually thinking of assigning a new CVE for this, to make it possible for distros to apply this fix. -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
On Tue, Feb 9, 2021 at 2:55 AM Bin Meng wrote:
>
> At the end of sdhci_send_command(), it starts a data transfer if
> the command register indicates a data is associated. However the
> data transfer should only be initiated when the command execution
> has succeeded.
>
> Cc: qemu-sta...@nongnu.org
> Fixes: CVE-2020-17380
> Fixes: CVE-2020-25085
> Reported-by: Alexander Bulekov
> Reported-by: Sergej Schumilo (Ruhr-University Bochum)
> Reported-by: Cornelius Aschermann (Ruhr-University Bochum)
> Reported-by: Simon Wrner (Ruhr-University Bochum)
> Buglink: https://bugs.launchpad.net/qemu/+bug/1892960
Isn't this already fixed?
> Signed-off-by: Bin Meng
Acked-by: Alistair Francis
Alistair
> ---
>
> 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..0450110 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 cmd_failure = 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 {
> +cmd_failure = 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 (!cmd_failure && s->blksize && (s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
> s->data_count = 0;
> sdhci_data_transfer(s);
> }
> --
> 2.7.4
>
>
Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
On 2/9/21 11:54 AM, Bin Meng wrote: > At the end of sdhci_send_command(), it starts a data transfer if > the command register indicates a data is associated. However the > data transfer should only be initiated when the command execution > has succeeded. > > Cc: qemu-sta...@nongnu.org > Fixes: CVE-2020-17380 > Fixes: CVE-2020-25085 > Reported-by: Alexander Bulekov > Reported-by: Sergej Schumilo (Ruhr-University Bochum) > Reported-by: Cornelius Aschermann (Ruhr-University Bochum) > Reported-by: Simon Wrner (Ruhr-University Bochum) > Buglink: https://bugs.launchpad.net/qemu/+bug/1892960 > Signed-off-by: Bin Meng > --- > > hw/sd/sdhci.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Tested-by: Philippe Mathieu-Daudé
[PATCH] hw/sd: sdhci: Do not transfer any data when command fails
At the end of sdhci_send_command(), it starts a data transfer if
the command register indicates a data is associated. However the
data transfer should only be initiated when the command execution
has succeeded.
Cc: qemu-sta...@nongnu.org
Fixes: CVE-2020-17380
Fixes: CVE-2020-25085
Reported-by: Alexander Bulekov
Reported-by: Sergej Schumilo (Ruhr-University Bochum)
Reported-by: Cornelius Aschermann (Ruhr-University Bochum)
Reported-by: Simon Wrner (Ruhr-University Bochum)
Buglink: https://bugs.launchpad.net/qemu/+bug/1892960
Signed-off-by: Bin Meng
---
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..0450110 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 cmd_failure = 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 {
+cmd_failure = 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 (!cmd_failure && s->blksize && (s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
s->data_count = 0;
sdhci_data_transfer(s);
}
--
2.7.4
