[PATCH] hw/sd: sdhci: Do not transfer any data when command fails

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
@ 2021-02-09 10:54 Bin Meng
  2021-02-09 11:03 ` Philippe Mathieu-Daudé
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Bin Meng @ 2021-02-09 10:54 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Mauro Matteo Cascella, Li Qiang,
	Alexander Bulekov, Alistair Francis, Prasad J Pandit, Bandan Das
  Cc: qemu-stable, qemu-block, qemu-devel

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-stable@nongnu.org
Fixes: CVE-2020-17380
Fixes: CVE-2020-25085
Reported-by: Alexander Bulekov <alxndr@bu.edu>
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 <bmeng.cn@gmail.com>
---

 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



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
  2021-02-09 10:54 [PATCH] hw/sd: sdhci: Do not transfer any data when command fails Bin Meng
@ 2021-02-09 11:03 ` Philippe Mathieu-Daudé
  2021-02-10 22:27 ` Alistair Francis
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-09 11:03 UTC (permalink / raw)
  To: Bin Meng, Mauro Matteo Cascella, Li Qiang, Alexander Bulekov,
	Alistair Francis, Prasad J Pandit, Bandan Das
  Cc: qemu-stable, qemu-block, qemu-devel

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-stable@nongnu.org
> Fixes: CVE-2020-17380
> Fixes: CVE-2020-25085
> Reported-by: Alexander Bulekov <alxndr@bu.edu>
> 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 <bmeng.cn@gmail.com>
> ---
> 
>  hw/sd/sdhci.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
  2021-02-09 10:54 [PATCH] hw/sd: sdhci: Do not transfer any data when command fails Bin Meng
  2021-02-09 11:03 ` Philippe Mathieu-Daudé
@ 2021-02-10 22:27 ` Alistair Francis
  2021-02-11  8:52   ` Mauro Matteo Cascella
  2021-02-11 15:49 ` Alexander Bulekov
  2021-02-11 16:53 ` Alexander Bulekov
  3 siblings, 1 reply; 11+ messages in thread
From: Alistair Francis @ 2021-02-10 22:27 UTC (permalink / raw)
  To: Bin Meng
  Cc: Mauro Matteo Cascella, Qemu-block,
	qemu-devel@nongnu.org Developers, qemu-stable, Li Qiang,
	Philippe Mathieu-Daudé, Prasad J Pandit, Alexander Bulekov,
	Bandan Das, Alistair Francis

On Tue, Feb 9, 2021 at 2:55 AM Bin Meng <bmeng.cn@gmail.com> 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-stable@nongnu.org
> Fixes: CVE-2020-17380
> Fixes: CVE-2020-25085
> Reported-by: Alexander Bulekov <alxndr@bu.edu>
> 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 <bmeng.cn@gmail.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

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
>
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
  2021-02-10 22:27 ` Alistair Francis
@ 2021-02-11  8:52   ` Mauro Matteo Cascella
  2021-02-11 19:48     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 11+ messages in thread
From: Mauro Matteo Cascella @ 2021-02-11  8:52 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Qemu-block, qemu-devel@nongnu.org Developers, qemu-stable,
	Li Qiang, Philippe Mathieu-Daudé, Prasad J Pandit,
	Alexander Bulekov, Bandan Das, Alistair Francis, Bin Meng

Hello,

On Wed, Feb 10, 2021 at 11:27 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Feb 9, 2021 at 2:55 AM Bin Meng <bmeng.cn@gmail.com> 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-stable@nongnu.org
> > Fixes: CVE-2020-17380
> > Fixes: CVE-2020-25085
> > Reported-by: Alexander Bulekov <alxndr@bu.edu>
> > 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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
  2021-02-11  8:52   ` Mauro Matteo Cascella
@ 2021-02-11 19:48     ` Philippe Mathieu-Daudé
  2021-02-12 13:55       ` Mauro Matteo Cascella
  0 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-11 19:48 UTC (permalink / raw)
  To: Mauro Matteo Cascella, Alistair Francis
  Cc: Qemu-block, qemu-stable, Li Qiang,
	qemu-devel@nongnu.org Developers, Prasad J Pandit,
	Alexander Bulekov, Bandan Das, Alistair Francis, Bin Meng

On 2/11/21 9:52 AM, Mauro Matteo Cascella wrote:
> Hello,
> 
> On Wed, Feb 10, 2021 at 11:27 PM Alistair Francis <alistair23@gmail.com> wrote:
>>
>> On Tue, Feb 9, 2021 at 2:55 AM Bin Meng <bmeng.cn@gmail.com> 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-stable@nongnu.org
>>> Fixes: CVE-2020-17380
>>> Fixes: CVE-2020-25085
>>> Reported-by: Alexander Bulekov <alxndr@bu.edu>
>>> 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
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
  2021-02-11 19:48     ` Philippe Mathieu-Daudé
@ 2021-02-12 13:55       ` Mauro Matteo Cascella
  0 siblings, 0 replies; 11+ messages in thread
From: Mauro Matteo Cascella @ 2021-02-12 13:55 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Qemu-block, qemu-stable, Li Qiang,
	qemu-devel@nongnu.org Developers, Prasad J Pandit,
	Alexander Bulekov, Bandan Das, Alistair Francis, Alistair Francis,
	Bin Meng

On Thu, Feb 11, 2021 at 8:48 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 2/11/21 9:52 AM, Mauro Matteo Cascella wrote:
> > Hello,
> >
> > On Wed, Feb 10, 2021 at 11:27 PM Alistair Francis <alistair23@gmail.com> wrote:
> >>
> >> On Tue, Feb 9, 2021 at 2:55 AM Bin Meng <bmeng.cn@gmail.com> 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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
  2021-02-09 10:54 [PATCH] hw/sd: sdhci: Do not transfer any data when command fails Bin Meng
  2021-02-09 11:03 ` Philippe Mathieu-Daudé
  2021-02-10 22:27 ` Alistair Francis
@ 2021-02-11 15:49 ` Alexander Bulekov
  2021-02-11 16:53 ` Alexander Bulekov
  3 siblings, 0 replies; 11+ messages in thread
From: Alexander Bulekov @ 2021-02-11 15:49 UTC (permalink / raw)
  To: Bin Meng
  Cc: Mauro Matteo Cascella, qemu-block, qemu-devel, qemu-stable,
	Li Qiang, Philippe Mathieu-Daudé, Prasad J Pandit,
	Bandan Das, Alistair Francis

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-stable@nongnu.org
> Fixes: CVE-2020-17380
> Fixes: CVE-2020-25085
> Reported-by: Alexander Bulekov <alxndr@bu.edu>
> 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 <bmeng.cn@gmail.com>
> ---
> 
>  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
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
  2021-02-09 10:54 [PATCH] hw/sd: sdhci: Do not transfer any data when command fails Bin Meng
                   ` (2 preceding siblings ...)
  2021-02-11 15:49 ` Alexander Bulekov
@ 2021-02-11 16:53 ` Alexander Bulekov
  2021-02-11 21:24   ` Alexander Bulekov
  3 siblings, 1 reply; 11+ messages in thread
From: Alexander Bulekov @ 2021-02-11 16:53 UTC (permalink / raw)
  To: Bin Meng
  Cc: Mauro Matteo Cascella, qemu-block, qemu-devel, qemu-stable,
	Li Qiang, Philippe Mathieu-Daudé, Prasad J Pandit,
	Bandan Das, Alistair Francis

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-stable@nongnu.org
> Fixes: CVE-2020-17380
> Fixes: CVE-2020-25085
> Reported-by: Alexander Bulekov <alxndr@bu.edu>
> 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 <bmeng.cn@gmail.com>

I applied this along with <20210208193450.2689517-1-f4bug@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 <alxndr@bu.edu>

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
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
  2021-02-11 16:53 ` Alexander Bulekov
@ 2021-02-11 21:24   ` Alexander Bulekov
  2021-02-14  5:56     ` Bin Meng
  0 siblings, 1 reply; 11+ messages in thread
From: Alexander Bulekov @ 2021-02-11 21:24 UTC (permalink / raw)
  To: Bin Meng
  Cc: Mauro Matteo Cascella, qemu-block, qemu-devel, qemu-stable,
	Li Qiang, Philippe Mathieu-Daudé, Prasad J Pandit,
	Bandan Das, Alistair Francis

On 210211 1154, Alexander Bulekov wrote:
...
> I applied this along with <20210208193450.2689517-1-f4bug@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 <alxndr@bu.edu>
> 
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 0x06000000
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 0x615000031880 at pc 0x564cf01ceae7 bp 0x7ffe17361e10 sp 0x7ffe173615d8
READ of size 520027904 at 0x615000031880 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 0x06000000
write 0x9100002c 0x1 0x05
write 0x9100000f 0x1 0x37
write 0x9100000a 0x1 0x01
write 0x9100000f 0x1 0x29
write 0x9100000f 0x1 0x02
write 0x9100000f 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 0x91000005 0x1 0x02
write 0x91000007 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 0x9100000e 0x1 0x21
write 0x91000028 0x1 0x10
write 0x9100000c 0x1 0x01
write 0x9100000f 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 0x91000037 0x1 0x00
write 0x91000038 0x1 0x00
write 0x1b8 0x1 0x01
write 0x1c0 0x1 0x01
write 0x1c8 0x1 0x01
write 0x1d0 0x1 0x01
write 0x1d8 0x1 0x01
write 0x1e0 0x1 0x01
write 0x1e8 0x1 0x01
write 0x1f0 0x1 0x01
write 0x1f8 0x1 0x01
write 0x200 0x1 0x01
write 0x208 0x1 0x01
write 0x210 0x1 0x01
write 0x218 0x1 0x01
write 0x220 0x1 0x01
write 0x228 0x1 0x01
write 0x9100000d 0x1 0x00
write 0x9100000f 0x1 0x10
write 0x91000011 0x1 0x00
write 0x230 0x1 0x01
write 0x238 0x1 0x01
write 0x240 0x1 0x01
write 0x248 0x1 0x01
write 0x250 0x1 0x01
write 0x258 0x1 0x01
write 0x260 0x1 0x01
write 0x268 0x1 0x01
write 0x270 0x1 0x01
write 0x278 0x1 0x01
write 0x280 0x1 0x01
write 0x288 0x1 0x01
write 0x290 0x1 0x01
write 0x298 0x1 0x01
write 0x2a0 0x1 0x01
write 0x9100000a 0x2 0x0000
write 0x9100000c 0x6 0x010000
write 0x2a8 0x1 0x01
write 0x2b0 0x1 0x01
write 0x2b8 0x1 0x01
write 0x2c0 0x1 0x01
write 0x2c8 0x1 0x01
write 0x2d0 0x1 0x01
write 0x2d8 0x1 0x01
write 0x2e0 0x1 0x01
write 0x2e8 0x1 0x01
write 0x2f0 0x1 0x01
write 0x2f8 0x1 0x01
write 0x300 0x1 0x01
write 0x308 0x1 0x01
write 0x310 0x1 0x01
write 0x318 0x1 0x01
write 0x320 0x1 0x01
write 0x328 0x1 0x01
write 0x330 0x1 0x01
write 0x338 0x1 0x01
write 0x340 0x1 0x01
write 0x348 0x1 0x01
write 0x350 0x1 0x01
write 0x358 0x1 0x01
write 0x360 0x1 0x01
write 0x368 0x1 0x01
write 0x370 0x1 0x01
write 0x378 0x1 0x01
write 0x380 0x1 0x01
write 0x388 0x1 0x01
write 0x390 0x1 0x01
write 0x9100000f 0x1 0x00
write 0x91000011 0x1 0x00
write 0x398 0x1 0x01
write 0x3a0 0x1 0x01
write 0x3a8 0x1 0x01
write 0x3b0 0x1 0x01
write 0x3b8 0x1 0x21
write 0x3bb 0x1 0x01
write 0x3c0 0x1 0x21
write 0x9100000a 0x2 0x0000
write 0x9100000c 0x6 0x010000
write 0x9100000a 0x2 0x00
write 0x9100000c 0x6 0x01
write 0x9100000a 0x2 0x0000
write 0x9100000c 0x6 0x010000
write 0x9100000a 0x2 0x00
write 0x9100000c 0x6 0x010000
write 0x91000005 0x1 0x00
write 0x9100000c 0x1 0x00
EOF

==== Stack Trace 2 ====
==837609==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x615000032280 at pc 0x564afb30eb6a bp 0x7ffdda140d90 sp 0x7ffdda140558
WRITE of size 483589332 at 0x615000032280 thread T0
#0 0x564afb30eb69 in __asan_memcpy (/home/alxndr/Development/qemu/build/qemu-fuzz-i386+0x2bccb69)
#1 0x564afca598bd in flatview_read_continue /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2846:13
#2 0x564afca5b09b in flatview_read /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2879:12
#3 0x564afca5b09b in address_space_read_full /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2892:18
#4 0x564afbab9e9d in dma_memory_rw_relaxed /home/alxndr/Development/qemu/include/sysemu/dma.h:88:12
#5 0x564afbab9e9d in dma_memory_rw /home/alxndr/Development/qemu/include/sysemu/dma.h:127:12
#6 0x564afbab9e9d in dma_memory_read /home/alxndr/Development/qemu/include/sysemu/dma.h:145:12
#7 0x564afbab9e9d in sdhci_do_adma /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:809:21
#8 0x564afbab2b81 in sdhci_data_transfer /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c
#9 0x564afbac2966 in sdhci_resume_pending_transfer /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:964:5
#10 0x564afbac2966 in sdhci_write /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:1120:9
#11 0x564afc697c6c in memory_region_write_accessor /home/alxndr/Development/qemu/build/../softmmu/memory.c:491:5


==== Repro 3 ====
(There is an identical one for a heap overflow through read ldl_he_p.
Let me know if it would be useful to provide it):

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 0xe0000000
outl 0xcf8 0x80001004
outw 0xcfc 0x06
write 0xe0000004 0x1 0x41
write 0xe0000006 0x1 0x80
write 0xe0000028 0x1 0x10
write 0xe000002c 0x1 0x05
write 0x0 0x1 0x21
write 0x2 0x1 0x0a
write 0x8 0x1 0x01
write 0xa 0x1 0x00
write 0x10 0x1 0x01
write 0x12 0x1 0x00
write 0x18 0x1 0x21
write 0x1a 0x1 0x16
write 0x20 0x1 0x01
write 0x22 0x1 0x00
write 0x28 0x1 0x01
write 0x2a 0x1 0x00
write 0x30 0x1 0x21
write 0x32 0x1 0x16
write 0x38 0x1 0x01
write 0x3a 0x1 0x00
write 0x40 0x1 0x01
write 0x42 0x1 0x00
write 0x48 0x1 0x21
write 0x4a 0x1 0x16
write 0x50 0x1 0x01
write 0x52 0x1 0x00
write 0x58 0x1 0x01
write 0x5a 0x1 0x00
write 0x60 0x1 0x21
write 0x62 0x1 0x16
write 0x68 0x1 0x01
write 0x6a 0x1 0x00
write 0x70 0x1 0x01
write 0x72 0x1 0x00
write 0x78 0x1 0x21
write 0x80 0x1 0x21
write 0x82 0x1 0x58
write 0x88 0x1 0x01
write 0x8a 0x1 0x00
write 0x90 0x1 0x01
write 0x92 0x1 0x00
write 0x98 0x1 0x21
write 0x9a 0x1 0x16
write 0xa0 0x1 0x21
write 0xa8 0x1 0x01
write 0xaa 0x1 0x00
write 0xb0 0x1 0x01
write 0xb2 0x1 0x00
write 0xb8 0x1 0x21
write 0xba 0x1 0x18
write 0xc0 0x1 0x01
write 0xc2 0x1 0x00
write 0xc8 0x1 0x01
write 0xca 0x1 0x00
write 0xd0 0x1 0x21
write 0xd2 0x1 0x16
write 0xd8 0x1 0x01
write 0xda 0x1 0x00
write 0xe0 0x1 0x01
write 0xe2 0x1 0x00
write 0xe8 0x1 0x21
write 0xea 0x1 0x18
write 0xf0 0x1 0x01
write 0xf2 0x1 0x00
write 0xf8 0x1 0x01
write 0xfa 0x1 0x00
write 0x100 0x1 0x21
write 0x102 0x1 0x16
write 0x108 0x1 0x01
write 0x10a 0x1 0x00
write 0x110 0x1 0x01
write 0x112 0x1 0x00
write 0x118 0x1 0x21
write 0x11a 0x1 0x18
write 0x120 0x1 0x01
write 0x122 0x1 0x00
write 0x128 0x1 0x01
write 0x12a 0x1 0x00
write 0x130 0x1 0x21
write 0x132 0x1 0x16
write 0x138 0x1 0x01
write 0x13a 0x1 0x00
write 0x140 0x1 0x01
write 0x142 0x1 0x00
write 0x148 0x1 0x21
write 0x14a 0x1 0x18
write 0x150 0x1 0x01
write 0x152 0x1 0x00
write 0x158 0x1 0x01
write 0x15a 0x1 0x00
write 0x160 0x1 0x21
write 0x162 0x1 0x16
write 0xe000000c 0x1 0x01
write 0xe000000e 0x1 0x20
write 0xe000000f 0x1 0x00
write 0x168 0x1 0x21
write 0x16d 0x1 0xff
write 0x16e 0x1 0xff
write 0x16f 0x1 0x1f
write 0xe0000000 0x4 0x00
write 0xe0000004 0x4 0x03010000
write 0xe0000085 0x1 0x00
write 0xe0000086 0x6 0x00
write 0xe000008c 0x1 0x00
write 0xe0000000 0x4 0x00
write 0xe0000004 0x2 0x00
write 0xe0000038 0x1 0x00
EOF


Stack Trace 3:
=817509==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x615000032280 at pc 0x564afca59f2b bp 0x7ffdda140d90 sp 0x7ffdda140d88
WRITE of size 4 at 0x615000032280 thread T0
#0 0x564afca59f2a in stl_he_p /home/alxndr/Development/qemu/include/qemu/bswap.h:353:5
#1 0x564afca59f2a in stn_he_p /home/alxndr/Development/qemu/include/qemu/bswap.h:546:1
#2 0x564afca59f2a in flatview_read_continue /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2841:13
#3 0x564afca5b09b in flatview_read /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2879:12
#4 0x564afca5b09b in address_space_read_full /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2892:18
#5 0x564afbab9e9d in dma_memory_rw_relaxed /home/alxndr/Development/qemu/include/sysemu/dma.h:88:12
#6 0x564afbab9e9d in dma_memory_rw /home/alxndr/Development/qemu/include/sysemu/dma.h:127:12
#7 0x564afbab9e9d in dma_memory_read /home/alxndr/Development/qemu/include/sysemu/dma.h:145:12
#8 0x564afbab9e9d in sdhci_do_adma /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:809:21
#9 0x564afbab2b81 in sdhci_data_transfer /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c
#10 0x564afbac2966 in sdhci_resume_pending_transfer /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:964:5
#11 0x564afbac2966 in sdhci_write /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:1120:9
#12 0x564afc697c6c in memory_region_write_accessor /home/alxndr/Development/qemu/build/../softmmu/memory.c:491:5



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
  2021-02-11 21:24   ` Alexander Bulekov
@ 2021-02-14  5:56     ` Bin Meng
  2021-02-14 15:53       ` Bin Meng
  0 siblings, 1 reply; 11+ messages in thread
From: Bin Meng @ 2021-02-14  5:56 UTC (permalink / raw)
  To: Alexander Bulekov
  Cc: Mauro Matteo Cascella, Qemu-block,
	qemu-devel@nongnu.org Developers, qemu-stable, Li Qiang,
	Philippe Mathieu-Daudé, Prasad J Pandit, Bandan Das,
	Alistair Francis

Hi Alexander,

On Fri, Feb 12, 2021 at 5:25 AM Alexander Bulekov <alxndr@bu.edu> wrote:
>
> On 210211 1154, Alexander Bulekov wrote:
> ...
> > I applied this along with <20210208193450.2689517-1-f4bug@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 <alxndr@bu.edu>
> >
> 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 0x06000000
> 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 0x615000031880 at pc 0x564cf01ceae7 bp 0x7ffe17361e10 sp 0x7ffe173615d8
> READ of size 520027904 at 0x615000031880 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 0x06000000
> write 0x9100002c 0x1 0x05
> write 0x9100000f 0x1 0x37
> write 0x9100000a 0x1 0x01
> write 0x9100000f 0x1 0x29
> write 0x9100000f 0x1 0x02
> write 0x9100000f 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 0x91000005 0x1 0x02
> write 0x91000007 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 0x9100000e 0x1 0x21
> write 0x91000028 0x1 0x10
> write 0x9100000c 0x1 0x01
> write 0x9100000f 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 0x91000037 0x1 0x00
> write 0x91000038 0x1 0x00
> write 0x1b8 0x1 0x01
> write 0x1c0 0x1 0x01
> write 0x1c8 0x1 0x01
> write 0x1d0 0x1 0x01
> write 0x1d8 0x1 0x01
> write 0x1e0 0x1 0x01
> write 0x1e8 0x1 0x01
> write 0x1f0 0x1 0x01
> write 0x1f8 0x1 0x01
> write 0x200 0x1 0x01
> write 0x208 0x1 0x01
> write 0x210 0x1 0x01
> write 0x218 0x1 0x01
> write 0x220 0x1 0x01
> write 0x228 0x1 0x01
> write 0x9100000d 0x1 0x00
> write 0x9100000f 0x1 0x10
> write 0x91000011 0x1 0x00
> write 0x230 0x1 0x01
> write 0x238 0x1 0x01
> write 0x240 0x1 0x01
> write 0x248 0x1 0x01
> write 0x250 0x1 0x01
> write 0x258 0x1 0x01
> write 0x260 0x1 0x01
> write 0x268 0x1 0x01
> write 0x270 0x1 0x01
> write 0x278 0x1 0x01
> write 0x280 0x1 0x01
> write 0x288 0x1 0x01
> write 0x290 0x1 0x01
> write 0x298 0x1 0x01
> write 0x2a0 0x1 0x01
> write 0x9100000a 0x2 0x0000
> write 0x9100000c 0x6 0x010000
> write 0x2a8 0x1 0x01
> write 0x2b0 0x1 0x01
> write 0x2b8 0x1 0x01
> write 0x2c0 0x1 0x01
> write 0x2c8 0x1 0x01
> write 0x2d0 0x1 0x01
> write 0x2d8 0x1 0x01
> write 0x2e0 0x1 0x01
> write 0x2e8 0x1 0x01
> write 0x2f0 0x1 0x01
> write 0x2f8 0x1 0x01
> write 0x300 0x1 0x01
> write 0x308 0x1 0x01
> write 0x310 0x1 0x01
> write 0x318 0x1 0x01
> write 0x320 0x1 0x01
> write 0x328 0x1 0x01
> write 0x330 0x1 0x01
> write 0x338 0x1 0x01
> write 0x340 0x1 0x01
> write 0x348 0x1 0x01
> write 0x350 0x1 0x01
> write 0x358 0x1 0x01
> write 0x360 0x1 0x01
> write 0x368 0x1 0x01
> write 0x370 0x1 0x01
> write 0x378 0x1 0x01
> write 0x380 0x1 0x01
> write 0x388 0x1 0x01
> write 0x390 0x1 0x01
> write 0x9100000f 0x1 0x00
> write 0x91000011 0x1 0x00
> write 0x398 0x1 0x01
> write 0x3a0 0x1 0x01
> write 0x3a8 0x1 0x01
> write 0x3b0 0x1 0x01
> write 0x3b8 0x1 0x21
> write 0x3bb 0x1 0x01
> write 0x3c0 0x1 0x21
> write 0x9100000a 0x2 0x0000
> write 0x9100000c 0x6 0x010000
> write 0x9100000a 0x2 0x00
> write 0x9100000c 0x6 0x01
> write 0x9100000a 0x2 0x0000
> write 0x9100000c 0x6 0x010000
> write 0x9100000a 0x2 0x00
> write 0x9100000c 0x6 0x010000
> write 0x91000005 0x1 0x00
> write 0x9100000c 0x1 0x00
> EOF
>
> ==== Stack Trace 2 ====
> ==837609==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x615000032280 at pc 0x564afb30eb6a bp 0x7ffdda140d90 sp 0x7ffdda140558
> WRITE of size 483589332 at 0x615000032280 thread T0
> #0 0x564afb30eb69 in __asan_memcpy (/home/alxndr/Development/qemu/build/qemu-fuzz-i386+0x2bccb69)
> #1 0x564afca598bd in flatview_read_continue /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2846:13
> #2 0x564afca5b09b in flatview_read /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2879:12
> #3 0x564afca5b09b in address_space_read_full /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2892:18
> #4 0x564afbab9e9d in dma_memory_rw_relaxed /home/alxndr/Development/qemu/include/sysemu/dma.h:88:12
> #5 0x564afbab9e9d in dma_memory_rw /home/alxndr/Development/qemu/include/sysemu/dma.h:127:12
> #6 0x564afbab9e9d in dma_memory_read /home/alxndr/Development/qemu/include/sysemu/dma.h:145:12
> #7 0x564afbab9e9d in sdhci_do_adma /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:809:21
> #8 0x564afbab2b81 in sdhci_data_transfer /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c
> #9 0x564afbac2966 in sdhci_resume_pending_transfer /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:964:5
> #10 0x564afbac2966 in sdhci_write /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:1120:9
> #11 0x564afc697c6c in memory_region_write_accessor /home/alxndr/Development/qemu/build/../softmmu/memory.c:491:5
>
>
> ==== Repro 3 ====
> (There is an identical one for a heap overflow through read ldl_he_p.
> Let me know if it would be useful to provide it):
>
> 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 0xe0000000
> outl 0xcf8 0x80001004
> outw 0xcfc 0x06
> write 0xe0000004 0x1 0x41
> write 0xe0000006 0x1 0x80
> write 0xe0000028 0x1 0x10
> write 0xe000002c 0x1 0x05
> write 0x0 0x1 0x21
> write 0x2 0x1 0x0a
> write 0x8 0x1 0x01
> write 0xa 0x1 0x00
> write 0x10 0x1 0x01
> write 0x12 0x1 0x00
> write 0x18 0x1 0x21
> write 0x1a 0x1 0x16
> write 0x20 0x1 0x01
> write 0x22 0x1 0x00
> write 0x28 0x1 0x01
> write 0x2a 0x1 0x00
> write 0x30 0x1 0x21
> write 0x32 0x1 0x16
> write 0x38 0x1 0x01
> write 0x3a 0x1 0x00
> write 0x40 0x1 0x01
> write 0x42 0x1 0x00
> write 0x48 0x1 0x21
> write 0x4a 0x1 0x16
> write 0x50 0x1 0x01
> write 0x52 0x1 0x00
> write 0x58 0x1 0x01
> write 0x5a 0x1 0x00
> write 0x60 0x1 0x21
> write 0x62 0x1 0x16
> write 0x68 0x1 0x01
> write 0x6a 0x1 0x00
> write 0x70 0x1 0x01
> write 0x72 0x1 0x00
> write 0x78 0x1 0x21
> write 0x80 0x1 0x21
> write 0x82 0x1 0x58
> write 0x88 0x1 0x01
> write 0x8a 0x1 0x00
> write 0x90 0x1 0x01
> write 0x92 0x1 0x00
> write 0x98 0x1 0x21
> write 0x9a 0x1 0x16
> write 0xa0 0x1 0x21
> write 0xa8 0x1 0x01
> write 0xaa 0x1 0x00
> write 0xb0 0x1 0x01
> write 0xb2 0x1 0x00
> write 0xb8 0x1 0x21
> write 0xba 0x1 0x18
> write 0xc0 0x1 0x01
> write 0xc2 0x1 0x00
> write 0xc8 0x1 0x01
> write 0xca 0x1 0x00
> write 0xd0 0x1 0x21
> write 0xd2 0x1 0x16
> write 0xd8 0x1 0x01
> write 0xda 0x1 0x00
> write 0xe0 0x1 0x01
> write 0xe2 0x1 0x00
> write 0xe8 0x1 0x21
> write 0xea 0x1 0x18
> write 0xf0 0x1 0x01
> write 0xf2 0x1 0x00
> write 0xf8 0x1 0x01
> write 0xfa 0x1 0x00
> write 0x100 0x1 0x21
> write 0x102 0x1 0x16
> write 0x108 0x1 0x01
> write 0x10a 0x1 0x00
> write 0x110 0x1 0x01
> write 0x112 0x1 0x00
> write 0x118 0x1 0x21
> write 0x11a 0x1 0x18
> write 0x120 0x1 0x01
> write 0x122 0x1 0x00
> write 0x128 0x1 0x01
> write 0x12a 0x1 0x00
> write 0x130 0x1 0x21
> write 0x132 0x1 0x16
> write 0x138 0x1 0x01
> write 0x13a 0x1 0x00
> write 0x140 0x1 0x01
> write 0x142 0x1 0x00
> write 0x148 0x1 0x21
> write 0x14a 0x1 0x18
> write 0x150 0x1 0x01
> write 0x152 0x1 0x00
> write 0x158 0x1 0x01
> write 0x15a 0x1 0x00
> write 0x160 0x1 0x21
> write 0x162 0x1 0x16
> write 0xe000000c 0x1 0x01
> write 0xe000000e 0x1 0x20
> write 0xe000000f 0x1 0x00
> write 0x168 0x1 0x21
> write 0x16d 0x1 0xff
> write 0x16e 0x1 0xff
> write 0x16f 0x1 0x1f
> write 0xe0000000 0x4 0x00
> write 0xe0000004 0x4 0x03010000
> write 0xe0000085 0x1 0x00
> write 0xe0000086 0x6 0x00
> write 0xe000008c 0x1 0x00
> write 0xe0000000 0x4 0x00
> write 0xe0000004 0x2 0x00
> write 0xe0000038 0x1 0x00
> EOF
>
>
> Stack Trace 3:
> =817509==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x615000032280 at pc 0x564afca59f2b bp 0x7ffdda140d90 sp 0x7ffdda140d88
> WRITE of size 4 at 0x615000032280 thread T0
> #0 0x564afca59f2a in stl_he_p /home/alxndr/Development/qemu/include/qemu/bswap.h:353:5
> #1 0x564afca59f2a in stn_he_p /home/alxndr/Development/qemu/include/qemu/bswap.h:546:1
> #2 0x564afca59f2a in flatview_read_continue /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2841:13
> #3 0x564afca5b09b in flatview_read /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2879:12
> #4 0x564afca5b09b in address_space_read_full /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2892:18
> #5 0x564afbab9e9d in dma_memory_rw_relaxed /home/alxndr/Development/qemu/include/sysemu/dma.h:88:12
> #6 0x564afbab9e9d in dma_memory_rw /home/alxndr/Development/qemu/include/sysemu/dma.h:127:12
> #7 0x564afbab9e9d in dma_memory_read /home/alxndr/Development/qemu/include/sysemu/dma.h:145:12
> #8 0x564afbab9e9d in sdhci_do_adma /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:809:21
> #9 0x564afbab2b81 in sdhci_data_transfer /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c
> #10 0x564afbac2966 in sdhci_resume_pending_transfer /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:964:5
> #11 0x564afbac2966 in sdhci_write /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:1120:9
> #12 0x564afc697c6c in memory_region_write_accessor /home/alxndr/Development/qemu/build/../softmmu/memory.c:491:5

Regards,
Bin


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] hw/sd: sdhci: Do not transfer any data when command fails
  2021-02-14  5:56     ` Bin Meng
@ 2021-02-14 15:53       ` Bin Meng
  0 siblings, 0 replies; 11+ messages in thread
From: Bin Meng @ 2021-02-14 15:53 UTC (permalink / raw)
  To: Alexander Bulekov
  Cc: Mauro Matteo Cascella, Qemu-block,
	qemu-devel@nongnu.org Developers, qemu-stable, Li Qiang,
	Philippe Mathieu-Daudé, Prasad J Pandit, Bandan Das,
	Alistair Francis

On Sun, Feb 14, 2021 at 1:56 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Alexander,
>
> On Fri, Feb 12, 2021 at 5:25 AM Alexander Bulekov <alxndr@bu.edu> wrote:
> >
> > On 210211 1154, Alexander Bulekov wrote:
> > ...
> > > I applied this along with <20210208193450.2689517-1-f4bug@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 <alxndr@bu.edu>
> > >
> > 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-02-14 15:55 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-02-09 10:54 [PATCH] hw/sd: sdhci: Do not transfer any data when command fails Bin Meng
2021-02-09 11:03 ` Philippe Mathieu-Daudé
2021-02-10 22:27 ` Alistair Francis
2021-02-11  8:52   ` Mauro Matteo Cascella
2021-02-11 19:48     ` Philippe Mathieu-Daudé
2021-02-12 13:55       ` Mauro Matteo Cascella
2021-02-11 15:49 ` Alexander Bulekov
2021-02-11 16:53 ` Alexander Bulekov
2021-02-11 21:24   ` Alexander Bulekov
2021-02-14  5:56     ` Bin Meng
2021-02-14 15:53       ` Bin Meng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).