On Mon, Aug 11, 2025 at 11:38 AM Michael Tokarev <m...@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+mugnu...@mail.gmail.com/
> >
> > Fixes: CVE-2022-3872
> > Reported-by: RivenDell <xrivend...@outlook.com>
> > Reported-by: Siqi Chen <coc.c...@gmail.com>
> > Reported-by: ningqiang <ningqia...@huawei.com>
> > Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> > Tested-by: Mauro Matteo Cascella <mcasc...@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
