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+mugnu...@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é <[email protected]>
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é <[email protected]>
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+mugnu...@mail.gmail.com/

Fixes: CVE-2022-3872
Reported-by: RivenDell <[email protected]>
Reported-by: Siqi Chen <[email protected]>
Reported-by: ningqiang <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
Tested-by: Mauro Matteo Cascella <[email protected]>
---
   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);
           }
       }




Reply via email to