Re: [PATCH v1] sd: sdhci: assert data_count is within fifo_buffer

[P J P]

+-- On Thu, 3 Sep 2020, Philippe Mathieu-Daudé wrote --+
| > -            assert(s->data_count <= s->buf_maxsz && s->data_count > begin);
| > +            assert(s->data_count <= s->buf_maxsz);
| > +            assert(s->data_count > begin);
| 
| Doesn't seem enough, guest crash here, having:
| 
| (gdb) p begin
| $1 = 0
| (gdb) p s->data_count
| $2 = 0

I was actually thinking of a case if 's->data_count' and 'begin' are same? It 
may lead to an infinite loop condition.

| (gdb) p s->blksize
| $3 = 0

This is strange. 

| Beh, something is wrong in this model, because when using ADMA2
| length 0 means 65536 bytes (see '1.13.4. Descriptor Table' in
| "SD Host Controller Simplified Specification Version 2.00").

* DMA length 's->data_count - begin'?

* if s->blksize is 65536, it'd set 'block_size = 0' in transfer_multi_blocks()

   #define BLOCK_SIZE_MASK (4 * KiB - 1)  <== 0xFFF

   static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)                  
   
   {
     ...
     const uint16_t block_size = s->blksize & BLOCK_SIZE_MASK;  <== 0


Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D