Re: [PATCH] sd: sdhci: check data_count is within fifo_buffer

[Alexander Bulekov]

Here's a qtest reproducer for this one:

cat << EOF |./i386-softmmu/qemu-system-i386 -nodefaults \
-device sdhci-pci -device sd-card,drive=mydrive \
-drive if=sd,index=0,file=null-co://,format=raw,id=mydrive \
-nographic -accel qtest -qtest stdio -nographic
outl 0xcf8 0x80001001
outl 0xcfc 0x7e6f25b7
outl 0xcf8 0x80001012
outl 0xcfc 0x842b1212
writeb 0x12120005 0xff
writeq 0x12120027 0x5e32b7120584125e
write 0x0 0x1 0x21
write 0x8 0x1 0x21
write 0x10 0x1 0x21
write 0x18 0x1 0x21
write 0x20 0x1 0x21
write 0x23 0x1 0x2b
writeq 0x1212000c 0x123a0584052da3ab
writeq 0x12120000 0xcfff000000000002
writeq 0x12120027 0x5c04c1c9c100005e
clock_step
EOF

Is it related to this https://bugs.launchpad.net/qemu/+bug/1892960 ?
-Alex

On 200827 1723, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
> 
> While doing multi block SDMA, transfer block size may exceed
> the 's->fifo_buffer[s->buf_maxsz]' size. It may leave the
> current element pointer 's->data_count' pointing out of bounds.
> Leading the subsequent DMA r/w operation to OOB access issue.
> Add check to avoid it.
> 
>  -> 
> https://ruhr-uni-bochum.sciebo.de/s/NNWP2GfwzYKeKwE?path=%2Fsdhci_oob_write1
>  ==1459837==ERROR: AddressSanitizer: heap-buffer-overflow
>  WRITE of size 54722048 at 0x61500001e280 thread T3
>     #0  __interceptor_memcpy (/lib64/libasan.so.6+0x3a71d)
>     #1  flatview_read_continue ../exec.c:3245
>     #2  flatview_read ../exec.c:3278
>     #3  address_space_read_full ../exec.c:3291
>     #4  address_space_rw ../exec.c:3319
>     #5  dma_memory_rw_relaxed ../include/sysemu/dma.h:87
>     #6  dma_memory_rw ../include/sysemu/dma.h:110
>     #7  dma_memory_read ../include/sysemu/dma.h:116
>     #8  sdhci_sdma_transfer_multi_blocks ../hw/sd/sdhci.c:629
>     #9  sdhci_write ../hw/sd/sdhci.c:1097
>     #10 memory_region_write_accessor ../softmmu/memory.c:483
>     ...
> 
> Reported-by: Ruhr-University <bugs-syssec@rub.de>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  hw/sd/sdhci.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 1785d7e1f7..155e25ceee 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -604,6 +604,9 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState 
> *s)
>                      s->blkcnt--;
>                  }
>              }
> +            if (s->data_count <= begin || s->data_count > s->buf_maxsz) {
> +                break;
> +            }
>              dma_memory_write(s->dma_as, s->sdmasysad,
>                               &s->fifo_buffer[begin], s->data_count - begin);
>              s->sdmasysad += s->data_count - begin;
> @@ -626,6 +629,9 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState 
> *s)
>                  s->data_count = block_size;
>                  boundary_count -= block_size - begin;
>              }
> +            if (s->data_count <= begin || s->data_count > s->buf_maxsz) {
> +                break;
> +            }
>              dma_memory_read(s->dma_as, s->sdmasysad,
>                              &s->fifo_buffer[begin], s->data_count - begin);
>              s->sdmasysad += s->data_count - begin;
> -- 
> 2.26.2
> 
>