When sdhci_write_block_to_card() is called to transfer data from
the FIFO to the SD bus, the data is already present in the buffer
and we have to consume it directly.
See the description of the 'Buffer Write Enable' bit from the
'Present State' register (prnsts::SDHC_SPACE_AVAILABLE) in Table
2.14 from the SDHCI spec v2:
Buffer Write Enable
This status is used for non-DMA write transfers.
The Host Controller can implement multiple buffers to transfer
data efficiently. This read only flag indicates if space is
available for write data. If this bit is 1, data can be written
to the buffer. A change of this bit from 1 to 0 occurs when all
the block data is written to the buffer. A change of this bit
from 0 to 1 occurs when top of block data can be written to the
buffer and generates the Buffer Write Ready interrupt.
In our case, we do not want to overwrite the buffer, so we want
this bit to be 0, then set it to 1 once the data is written onto
the bus.
This is probably a copy/paste error from commit d7dfca0807
("hw/sdhci: introduce standard SD host controller").
Reproducer:
https://lore.kernel.org/qemu-devel/CAA8xKjXrmS0fkr28AKvNNpyAtM0y0B+5FichpsrhD+mUgnuyKg@mail.gmail.com/
Fixes: CVE-2022-3872
Reported-by: RivenDell <XRivenDell@outlook.com>
Reported-by: Siqi Chen <coc.cyqh@gmail.com>
Reported-by: ningqiang <ningqiang1@huawei.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/sd/sdhci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 306070c872..f230e7475f 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -954,7 +954,7 @@ static void sdhci_data_transfer(void *opaque)
sdhci_read_block_from_card(s);
} else {
s->prnsts |= SDHC_DOING_WRITE | SDHC_DAT_LINE_ACTIVE |
- SDHC_SPACE_AVAILABLE | SDHC_DATA_INHIBIT;
+ SDHC_DATA_INHIBIT;
sdhci_write_block_to_card(s);
}
}
--
2.38.1
Ping once again?
Thanks,
/mjt
On 11/8/22 01:12, Philippe Mathieu-Daudé wrote:
> When sdhci_write_block_to_card() is called to transfer data from
> the FIFO to the SD bus, the data is already present in the buffer
> and we have to consume it directly.
>
> See the description of the 'Buffer Write Enable' bit from the
> 'Present State' register (prnsts::SDHC_SPACE_AVAILABLE) in Table
> 2.14 from the SDHCI spec v2:
>
> Buffer Write Enable
>
> This status is used for non-DMA write transfers.
>
> The Host Controller can implement multiple buffers to transfer
> data efficiently. This read only flag indicates if space is
> available for write data. If this bit is 1, data can be written
> to the buffer. A change of this bit from 1 to 0 occurs when all
> the block data is written to the buffer. A change of this bit
> from 0 to 1 occurs when top of block data can be written to the
> buffer and generates the Buffer Write Ready interrupt.
>
> In our case, we do not want to overwrite the buffer, so we want
> this bit to be 0, then set it to 1 once the data is written onto
> the bus.
>
> This is probably a copy/paste error from commit d7dfca0807
> ("hw/sdhci: introduce standard SD host controller").
>
> Reproducer:
> https://lore.kernel.org/qemu-devel/CAA8xKjXrmS0fkr28AKvNNpyAtM0y0B+5FichpsrhD+mUgnuyKg@mail.gmail.com/
>
> Fixes: CVE-2022-3872
> Reported-by: RivenDell <XRivenDell@outlook.com>
> Reported-by: Siqi Chen <coc.cyqh@gmail.com>
> Reported-by: ningqiang <ningqiang1@huawei.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Mauro Matteo Cascella <mcascell@redhat.com>
> ---
> hw/sd/sdhci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 306070c872..f230e7475f 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -954,7 +954,7 @@ static void sdhci_data_transfer(void *opaque)
> sdhci_read_block_from_card(s);
> } else {
> s->prnsts |= SDHC_DOING_WRITE | SDHC_DAT_LINE_ACTIVE |
> - SDHC_SPACE_AVAILABLE | SDHC_DATA_INHIBIT;
> + SDHC_DATA_INHIBIT;
> sdhci_write_block_to_card(s);
> }
> }
On 260213 1205, Michael Tokarev wrote:
> Ping once again?
>
FWIW, none of the reproducers in the thread work for me anymore and
OSS-Fuzz claims the issue was fixed sometime in April 2024:
https://issues.oss-fuzz.com/issues/42524205#comment5
-Alex
> Thanks,
>
> /mjt
>
> On 11/8/22 01:12, Philippe Mathieu-Daudé wrote:
> > When sdhci_write_block_to_card() is called to transfer data from
> > the FIFO to the SD bus, the data is already present in the buffer
> > and we have to consume it directly.
> >
> > See the description of the 'Buffer Write Enable' bit from the
> > 'Present State' register (prnsts::SDHC_SPACE_AVAILABLE) in Table
> > 2.14 from the SDHCI spec v2:
> >
> > Buffer Write Enable
> >
> > This status is used for non-DMA write transfers.
> >
> > The Host Controller can implement multiple buffers to transfer
> > data efficiently. This read only flag indicates if space is
> > available for write data. If this bit is 1, data can be written
> > to the buffer. A change of this bit from 1 to 0 occurs when all
> > the block data is written to the buffer. A change of this bit
> > from 0 to 1 occurs when top of block data can be written to the
> > buffer and generates the Buffer Write Ready interrupt.
> >
> > In our case, we do not want to overwrite the buffer, so we want
> > this bit to be 0, then set it to 1 once the data is written onto
> > the bus.
> >
> > This is probably a copy/paste error from commit d7dfca0807
> > ("hw/sdhci: introduce standard SD host controller").
> >
> > Reproducer:
> > https://lore.kernel.org/qemu-devel/CAA8xKjXrmS0fkr28AKvNNpyAtM0y0B+5FichpsrhD+mUgnuyKg@mail.gmail.com/
> >
> > Fixes: CVE-2022-3872
> > Reported-by: RivenDell <XRivenDell@outlook.com>
> > Reported-by: Siqi Chen <coc.cyqh@gmail.com>
> > Reported-by: ningqiang <ningqiang1@huawei.com>
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Tested-by: Mauro Matteo Cascella <mcascell@redhat.com>
> > ---
> > hw/sd/sdhci.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> > index 306070c872..f230e7475f 100644
> > --- a/hw/sd/sdhci.c
> > +++ b/hw/sd/sdhci.c
> > @@ -954,7 +954,7 @@ static void sdhci_data_transfer(void *opaque)
> > sdhci_read_block_from_card(s);
> > } else {
> > s->prnsts |= SDHC_DOING_WRITE | SDHC_DAT_LINE_ACTIVE |
> > - SDHC_SPACE_AVAILABLE | SDHC_DATA_INHIBIT;
> > + SDHC_DATA_INHIBIT;
> > sdhci_write_block_to_card(s);
> > }
> > }
>
On 2/13/26 17:43, Alexander Bulekov wrote:
> On 260213 1205, Michael Tokarev wrote:
>> Ping once again?
>>
>
> FWIW, none of the reproducers in the thread work for me anymore and
> OSS-Fuzz claims the issue was fixed sometime in April 2024:
> https://issues.oss-fuzz.com/issues/42524205#comment5
ok. I bisected using the reproducer from
https://lore.kernel.org/qemu-devel/CAA8xKjXrmS0fkr28AKvNNpyAtM0y0B+5FichpsrhD+mUgnuyKg@mail.gmail.com/
-
and it looks like first, the invalid write were replaced with an
assertion failure after this commit, which is itself is a fix for
CVE-2024-3447:
commit 9e4b27ca6bf4974f169bbca7f3dca117b1208b6f (v9.0.0-rc2-64-g9e4b27ca6b)
Author: Philippe Mathieu-Daudé <philmd@linaro.org>
Date: Tue Apr 9 16:19:27 2024 +0200
hw/sd/sdhci: Do not update TRNMOD when Command Inhibit (DAT) is set
https://gitlab.com/qemu-project/qemu/-/commit/9e4b27ca6bf4974f169bbca7f3dca117b1208b6f
So yes, this is in Apr-24, and it is in v9.0.0.
After this commit, the invalid write has gone, instead, we started
hitting assertion failure in sdhci_write_dataport():
hw/sd/sdhci.c:565: sdhci_write_dataport: Assertion `s->data_count <
s->buf_maxsz' failed.
next, in
commit ed5a159c3de48a581f46de4c8c02b4b295e6c52d (v9.1.0-rc0-80-ged5a159c3d)
Author: Philippe Mathieu-Daudé <philmd@linaro.org>
Date: Tue Jul 30 10:41:25 2024 +0200
hw/sd/sdhci: Reset @data_count index on invalid ADMA transfers
https://gitlab.com/qemu-project/qemu/-/commit/ed5a159c3de48a581f46de4c8c02b4b295e6c52d
the assertion failure has gone for good, at least with this
reproducer.
(the first commit is v7.2.10-56-g2429cb7a9f in 7.2.x series,
second - v7.2.13-25-gfc2e706f4c).
So it looks like the issue has been fixed for good in v9.1.0
and v7.2.14.
Please take a look at the original bug report and the final
logic as per this message - does it look sane?
>
>> Thanks,
>>
>> /mjt
>>
>> On 11/8/22 01:12, Philippe Mathieu-Daudé wrote:
>>> When sdhci_write_block_to_card() is called to transfer data from
>>> the FIFO to the SD bus, the data is already present in the buffer
>>> and we have to consume it directly.
>>>
>>> See the description of the 'Buffer Write Enable' bit from the
>>> 'Present State' register (prnsts::SDHC_SPACE_AVAILABLE) in Table
>>> 2.14 from the SDHCI spec v2:
>>>
>>> Buffer Write Enable
>>>
>>> This status is used for non-DMA write transfers.
>>>
>>> The Host Controller can implement multiple buffers to transfer
>>> data efficiently. This read only flag indicates if space is
>>> available for write data. If this bit is 1, data can be written
>>> to the buffer. A change of this bit from 1 to 0 occurs when all
>>> the block data is written to the buffer. A change of this bit
>>> from 0 to 1 occurs when top of block data can be written to the
>>> buffer and generates the Buffer Write Ready interrupt.
>>>
>>> In our case, we do not want to overwrite the buffer, so we want
>>> this bit to be 0, then set it to 1 once the data is written onto
>>> the bus.
>>>
>>> This is probably a copy/paste error from commit d7dfca0807
>>> ("hw/sdhci: introduce standard SD host controller").
>>>
>>> Reproducer:
>>> https://lore.kernel.org/qemu-devel/CAA8xKjXrmS0fkr28AKvNNpyAtM0y0B+5FichpsrhD+mUgnuyKg@mail.gmail.com/
>>>
>>> Fixes: CVE-2022-3872
>>> Reported-by: RivenDell <XRivenDell@outlook.com>
>>> Reported-by: Siqi Chen <coc.cyqh@gmail.com>
>>> Reported-by: ningqiang <ningqiang1@huawei.com>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Tested-by: Mauro Matteo Cascella <mcascell@redhat.com>
>>> ---
>>> hw/sd/sdhci.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>>> index 306070c872..f230e7475f 100644
>>> --- a/hw/sd/sdhci.c
>>> +++ b/hw/sd/sdhci.c
>>> @@ -954,7 +954,7 @@ static void sdhci_data_transfer(void *opaque)
>>> sdhci_read_block_from_card(s);
>>> } else {
>>> s->prnsts |= SDHC_DOING_WRITE | SDHC_DAT_LINE_ACTIVE |
>>> - SDHC_SPACE_AVAILABLE | SDHC_DATA_INHIBIT;
>>> + SDHC_DATA_INHIBIT;
>>> sdhci_write_block_to_card(s);
>>> }
>>> }
>>
>
On 16.02.2026 16:44, Michael Tokarev wrote: > On 2/13/26 17:43, Alexander Bulekov wrote: >> On 260213 1205, Michael Tokarev wrote: >>> Ping once again? >>> >> >> FWIW, none of the reproducers in the thread work for me anymore and >> OSS-Fuzz claims the issue was fixed sometime in April 2024: >> https://issues.oss-fuzz.com/issues/42524205#comment5 > > > ok. I bisected using the reproducer from > https://lore.kernel.org/qemu-devel/ > CAA8xKjXrmS0fkr28AKvNNpyAtM0y0B+5FichpsrhD+mUgnuyKg@mail.gmail.com/ - > and it looks like first, the invalid write were replaced with an > assertion failure after this commit, which is itself is a fix for > CVE-2024-3447: > > commit 9e4b27ca6bf4974f169bbca7f3dca117b1208b6f (v9.0.0-rc2-64-g9e4b27ca6b) > Author: Philippe Mathieu-Daudé <philmd@linaro.org> > Date: Tue Apr 9 16:19:27 2024 +0200 > > hw/sd/sdhci: Do not update TRNMOD when Command Inhibit (DAT) is set > > https://gitlab.com/qemu-project/qemu/-/ > commit/9e4b27ca6bf4974f169bbca7f3dca117b1208b6f > > So yes, this is in Apr-24, and it is in v9.0.0. > > After this commit, the invalid write has gone, instead, we started > hitting assertion failure in sdhci_write_dataport(): > > hw/sd/sdhci.c:565: sdhci_write_dataport: Assertion `s->data_count < s- > >buf_maxsz' failed. > > next, in > > commit ed5a159c3de48a581f46de4c8c02b4b295e6c52d (v9.1.0-rc0-80-ged5a159c3d) > Author: Philippe Mathieu-Daudé <philmd@linaro.org> > Date: Tue Jul 30 10:41:25 2024 +0200 > > hw/sd/sdhci: Reset @data_count index on invalid ADMA transfers > > https://gitlab.com/qemu-project/qemu/-/commit/ > ed5a159c3de48a581f46de4c8c02b4b295e6c52d > > the assertion failure has gone for good, at least with this > reproducer. > > (the first commit is v7.2.10-56-g2429cb7a9f in 7.2.x series, > second - v7.2.13-25-gfc2e706f4c). > > So it looks like the issue has been fixed for good in v9.1.0 > and v7.2.14. > > Please take a look at the original bug report and the final > logic as per this message - does it look sane? Hi! Can someone more knowlegeable in this area than me actually take a look there please? Does this change actually fix the prob? Thanks, /mjt
On 08.11.2022 01:12, Philippe Mathieu-Daudé wrote:
> When sdhci_write_block_to_card() is called to transfer data from
> the FIFO to the SD bus, the data is already present in the buffer
> and we have to consume it directly.
>
> See the description of the 'Buffer Write Enable' bit from the
> 'Present State' register (prnsts::SDHC_SPACE_AVAILABLE) in Table
> 2.14 from the SDHCI spec v2:
>
> Buffer Write Enable
>
> This status is used for non-DMA write transfers.
>
> The Host Controller can implement multiple buffers to transfer
> data efficiently. This read only flag indicates if space is
> available for write data. If this bit is 1, data can be written
> to the buffer. A change of this bit from 1 to 0 occurs when all
> the block data is written to the buffer. A change of this bit
> from 0 to 1 occurs when top of block data can be written to the
> buffer and generates the Buffer Write Ready interrupt.
>
> In our case, we do not want to overwrite the buffer, so we want
> this bit to be 0, then set it to 1 once the data is written onto
> the bus.
>
> This is probably a copy/paste error from commit d7dfca0807
> ("hw/sdhci: introduce standard SD host controller").
>
> Reproducer:
> https://lore.kernel.org/qemu-devel/CAA8xKjXrmS0fkr28AKvNNpyAtM0y0B+5FichpsrhD+mUgnuyKg@mail.gmail.com/
>
> Fixes: CVE-2022-3872
> Reported-by: RivenDell <XRivenDell@outlook.com>
> Reported-by: Siqi Chen <coc.cyqh@gmail.com>
> Reported-by: ningqiang <ningqiang1@huawei.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Mauro Matteo Cascella <mcascell@redhat.com>
Hi!
It's been almost 3 years since this patch.
Is it still relevant?
Or maybe the prob has been fixed by later changes in this area?
Thanks,
/mjt
> hw/sd/sdhci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 306070c872..f230e7475f 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -954,7 +954,7 @@ static void sdhci_data_transfer(void *opaque)
> sdhci_read_block_from_card(s);
> } else {
> s->prnsts |= SDHC_DOING_WRITE | SDHC_DAT_LINE_ACTIVE |
> - SDHC_SPACE_AVAILABLE | SDHC_DATA_INHIBIT;
> + SDHC_DATA_INHIBIT;
> sdhci_write_block_to_card(s);
> }
> }
On Mon, Aug 11, 2025 at 11:38 AM Michael Tokarev <mjt@tls.msk.ru> wrote:
>
> On 08.11.2022 01:12, Philippe Mathieu-Daudé wrote:
> > When sdhci_write_block_to_card() is called to transfer data from
> > the FIFO to the SD bus, the data is already present in the buffer
> > and we have to consume it directly.
> >
> > See the description of the 'Buffer Write Enable' bit from the
> > 'Present State' register (prnsts::SDHC_SPACE_AVAILABLE) in Table
> > 2.14 from the SDHCI spec v2:
> >
> > Buffer Write Enable
> >
> > This status is used for non-DMA write transfers.
> >
> > The Host Controller can implement multiple buffers to transfer
> > data efficiently. This read only flag indicates if space is
> > available for write data. If this bit is 1, data can be written
> > to the buffer. A change of this bit from 1 to 0 occurs when all
> > the block data is written to the buffer. A change of this bit
> > from 0 to 1 occurs when top of block data can be written to the
> > buffer and generates the Buffer Write Ready interrupt.
> >
> > In our case, we do not want to overwrite the buffer, so we want
> > this bit to be 0, then set it to 1 once the data is written onto
> > the bus.
> >
> > This is probably a copy/paste error from commit d7dfca0807
> > ("hw/sdhci: introduce standard SD host controller").
> >
> > Reproducer:
> > https://lore.kernel.org/qemu-devel/CAA8xKjXrmS0fkr28AKvNNpyAtM0y0B+5FichpsrhD+mUgnuyKg@mail.gmail.com/
> >
> > Fixes: CVE-2022-3872
> > Reported-by: RivenDell <XRivenDell@outlook.com>
> > Reported-by: Siqi Chen <coc.cyqh@gmail.com>
> > Reported-by: ningqiang <ningqiang1@huawei.com>
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Tested-by: Mauro Matteo Cascella <mcascell@redhat.com>
>
> Hi!
>
> It's been almost 3 years since this patch.
> Is it still relevant?
> Or maybe the prob has been fixed by later changes in this area?
SDHC_SPACE_AVAILABLE is still set in the write path:
https://gitlab.com/qemu-project/qemu/-/blob/master/hw/sd/sdhci.c?ref_type=heads#L988
If that is a bug, this patch should be applied regardless of its
security implications (if any).
> Thanks,
>
> /mjt
>
> > hw/sd/sdhci.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> > index 306070c872..f230e7475f 100644
> > --- a/hw/sd/sdhci.c
> > +++ b/hw/sd/sdhci.c
> > @@ -954,7 +954,7 @@ static void sdhci_data_transfer(void *opaque)
> > sdhci_read_block_from_card(s);
> > } else {
> > s->prnsts |= SDHC_DOING_WRITE | SDHC_DAT_LINE_ACTIVE |
> > - SDHC_SPACE_AVAILABLE | SDHC_DATA_INHIBIT;
> > + SDHC_DATA_INHIBIT;
> > sdhci_write_block_to_card(s);
> > }
> > }
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
On 221107 2312, Philippe Mathieu-Daudé wrote:
> When sdhci_write_block_to_card() is called to transfer data from
> the FIFO to the SD bus, the data is already present in the buffer
> and we have to consume it directly.
>
> See the description of the 'Buffer Write Enable' bit from the
> 'Present State' register (prnsts::SDHC_SPACE_AVAILABLE) in Table
> 2.14 from the SDHCI spec v2:
>
> Buffer Write Enable
>
> This status is used for non-DMA write transfers.
>
> The Host Controller can implement multiple buffers to transfer
> data efficiently. This read only flag indicates if space is
> available for write data. If this bit is 1, data can be written
> to the buffer. A change of this bit from 1 to 0 occurs when all
> the block data is written to the buffer. A change of this bit
> from 0 to 1 occurs when top of block data can be written to the
> buffer and generates the Buffer Write Ready interrupt.
>
> In our case, we do not want to overwrite the buffer, so we want
> this bit to be 0, then set it to 1 once the data is written onto
> the bus.
>
> This is probably a copy/paste error from commit d7dfca0807
> ("hw/sdhci: introduce standard SD host controller").
>
> Reproducer:
> https://lore.kernel.org/qemu-devel/CAA8xKjXrmS0fkr28AKvNNpyAtM0y0B+5FichpsrhD+mUgnuyKg@mail.gmail.com/
>
> Fixes: CVE-2022-3872
> Reported-by: RivenDell <XRivenDell@outlook.com>
> Reported-by: Siqi Chen <coc.cyqh@gmail.com>
> Reported-by: ningqiang <ningqiang1@huawei.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Seems like OSS-Fuzz also found this, not sure why it never made it into
a gitlab issue:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45986#c4
Slightly shorter reproducer:
cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, -m \
512M -nodefaults -device sdhci-pci -device sd-card,drive=mydrive -drive \
if=none,index=0,file=null-co://,format=raw,id=mydrive -nographic -qtest \
stdio
outl 0xcf8 0x80001010
outl 0xcfc 0xe0000000
outl 0xcf8 0x80001001
outl 0xcfc 0x06000000
write 0xe0000058 0x1 0x6e
write 0xe0000059 0x1 0x5a
write 0xe0000028 0x1 0x10
write 0xe000002c 0x1 0x05
write 0x5a6e 0x1 0x21
write 0x5a75 0x1 0x20
write 0xe0000005 0x1 0x02
write 0xe000000c 0x1 0x01
write 0xe000000e 0x1 0x20
write 0xe000000f 0x1 0x00
write 0xe000000c 0x1 0x00
write 0xe0000020 0x1 0x00
EOF
On 221108 1225, Alexander Bulekov wrote:
> On 221107 2312, Philippe Mathieu-Daudé wrote:
> > When sdhci_write_block_to_card() is called to transfer data from
> > the FIFO to the SD bus, the data is already present in the buffer
> > and we have to consume it directly.
> >
> > See the description of the 'Buffer Write Enable' bit from the
> > 'Present State' register (prnsts::SDHC_SPACE_AVAILABLE) in Table
> > 2.14 from the SDHCI spec v2:
> >
> > Buffer Write Enable
> >
> > This status is used for non-DMA write transfers.
> >
> > The Host Controller can implement multiple buffers to transfer
> > data efficiently. This read only flag indicates if space is
> > available for write data. If this bit is 1, data can be written
> > to the buffer. A change of this bit from 1 to 0 occurs when all
> > the block data is written to the buffer. A change of this bit
> > from 0 to 1 occurs when top of block data can be written to the
> > buffer and generates the Buffer Write Ready interrupt.
> >
> > In our case, we do not want to overwrite the buffer, so we want
> > this bit to be 0, then set it to 1 once the data is written onto
> > the bus.
> >
> > This is probably a copy/paste error from commit d7dfca0807
> > ("hw/sdhci: introduce standard SD host controller").
> >
> > Reproducer:
> > https://lore.kernel.org/qemu-devel/CAA8xKjXrmS0fkr28AKvNNpyAtM0y0B+5FichpsrhD+mUgnuyKg@mail.gmail.com/
> >
> > Fixes: CVE-2022-3872
> > Reported-by: RivenDell <XRivenDell@outlook.com>
> > Reported-by: Siqi Chen <coc.cyqh@gmail.com>
> > Reported-by: ningqiang <ningqiang1@huawei.com>
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Seems like OSS-Fuzz also found this, not sure why it never made it into
> a gitlab issue:
> https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45986#c4
>
> Slightly shorter reproducer:
>
> cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, -m \
> 512M -nodefaults -device sdhci-pci -device sd-card,drive=mydrive -drive \
> if=none,index=0,file=null-co://,format=raw,id=mydrive -nographic -qtest \
> stdio
> outl 0xcf8 0x80001010
> outl 0xcfc 0xe0000000
> outl 0xcf8 0x80001001
> outl 0xcfc 0x06000000
> write 0xe0000058 0x1 0x6e
> write 0xe0000059 0x1 0x5a
> write 0xe0000028 0x1 0x10
> write 0xe000002c 0x1 0x05
> write 0x5a6e 0x1 0x21
> write 0x5a75 0x1 0x20
> write 0xe0000005 0x1 0x02
> write 0xe000000c 0x1 0x01
> write 0xe000000e 0x1 0x20
> write 0xe000000f 0x1 0x00
> write 0xe000000c 0x1 0x00
> write 0xe0000020 0x1 0x00
> EOF
Hi Philippe,
I ran the fuzzer with this series applied and it found:
cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, -m \
512M -nodefaults -device sdhci-pci -device sd-card,drive=mydrive -drive \
if=none,index=0,file=null-co://,format=raw,id=mydrive -nographic -qtest \
stdio
outl 0xcf8 0x80001010
outl 0xcfc 0xe0000000
outl 0xcf8 0x80001004
outw 0xcfc 0x06
write 0xe0000028 0x1 0x08
write 0xe000002c 0x1 0x05
write 0xe0000005 0x1 0x02
write 0x0 0x1 0x21
write 0x3 0x1 0x20
write 0xe000000c 0x1 0x01
write 0xe000000e 0x1 0x20
write 0xe000000f 0x1 0x00
write 0xe000000c 0x1 0x20
write 0xe0000020 0x1 0x00
EOF
The crash seems very similar, but it looks like instead of
SDHC_TRNS_READ this reproducer sets SDHC_TRNS_MULTI
-Alex
On Mon, Nov 7, 2022 at 11:12 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> When sdhci_write_block_to_card() is called to transfer data from
> the FIFO to the SD bus, the data is already present in the buffer
> and we have to consume it directly.
>
> See the description of the 'Buffer Write Enable' bit from the
> 'Present State' register (prnsts::SDHC_SPACE_AVAILABLE) in Table
> 2.14 from the SDHCI spec v2:
>
> Buffer Write Enable
>
> This status is used for non-DMA write transfers.
>
> The Host Controller can implement multiple buffers to transfer
> data efficiently. This read only flag indicates if space is
> available for write data. If this bit is 1, data can be written
> to the buffer. A change of this bit from 1 to 0 occurs when all
> the block data is written to the buffer. A change of this bit
> from 0 to 1 occurs when top of block data can be written to the
> buffer and generates the Buffer Write Ready interrupt.
>
> In our case, we do not want to overwrite the buffer, so we want
> this bit to be 0, then set it to 1 once the data is written onto
> the bus.
>
> This is probably a copy/paste error from commit d7dfca0807
> ("hw/sdhci: introduce standard SD host controller").
>
> Reproducer:
> https://lore.kernel.org/qemu-devel/CAA8xKjXrmS0fkr28AKvNNpyAtM0y0B+5FichpsrhD+mUgnuyKg@mail.gmail.com/
>
> Fixes: CVE-2022-3872
> Reported-by: RivenDell <XRivenDell@outlook.com>
> Reported-by: Siqi Chen <coc.cyqh@gmail.com>
> Reported-by: ningqiang <ningqiang1@huawei.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/sd/sdhci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 306070c872..f230e7475f 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -954,7 +954,7 @@ static void sdhci_data_transfer(void *opaque)
> sdhci_read_block_from_card(s);
> } else {
> s->prnsts |= SDHC_DOING_WRITE | SDHC_DAT_LINE_ACTIVE |
> - SDHC_SPACE_AVAILABLE | SDHC_DATA_INHIBIT;
> + SDHC_DATA_INHIBIT;
> sdhci_write_block_to_card(s);
> }
> }
> --
> 2.38.1
>
Tested-by: Mauro Matteo Cascella <mcascell@redhat.com>
Thank you,
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
© 2016 - 2026 Red Hat, Inc.