qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[RFC.PATCH v1 1/2] sd:sdhci: Fix boundary_count overflow in sdhci_sdma_t


From: Jamin Lin
Subject: [RFC.PATCH v1 1/2] sd:sdhci: Fix boundary_count overflow in sdhci_sdma_transfer_multi_blocks
Date: Wed, 11 Dec 2024 17:51:09 +0800

How to reproduce it:
1. The value of "s->blksie" was 0x7200. The bits[14:12] was "111", so the buffer
   boundary was 0x80000.(512Kbytes). This SDMA buffer boundary the same as
   u-boot default value.
   The bit[11:0] is "001000000000", so the block size is 0x200.(512bytes)
2. The SDMA address was 0x83123456 which was not page aligned and
   "s->sdmasysad % boundary_chk" was 0x23456. The value of boundary_count was
   0x5cbaa.("boundary_chk - (s->sdmasysad % boundary_chk)" -->
   "(0x80000 - 0x23456)")

However, boundary_count did not aligned the block size 512 bytes and the SDMA
address is not page aligned(0x80000), so the following if-statement never be 
true,
```
if (((boundary_count + begin) < block_size) && page_aligned)
````

Finally, it caused boundary_count overflow because its data type was uint32_t.
Ex: the last boundary_count was 0x1aa and "0x1aa - 0x200" became "0xffffffaa".
It is the wrong behavior.

To fix it, it seems we can directly check the "boundary_count < blocksize"
instead of "boundary_count < blocksize && page_aligned"

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/sd/sdhci.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 37875c02c3..47d96b935b 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -590,7 +590,6 @@ static void sdhci_write_dataport(SDHCIState *s, uint32_t 
value, unsigned size)
 /* Multi block SDMA transfer */
 static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
 {
-    bool page_aligned = false;
     unsigned int begin;
     const uint16_t block_size = s->blksize & BLOCK_SIZE_MASK;
     uint32_t boundary_chk = 1 << (((s->blksize & ~BLOCK_SIZE_MASK) >> 12) + 
12);
@@ -601,15 +600,6 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
         return;
     }
 
-    /*
-     * XXX: Some sd/mmc drivers (for example, u-boot-slp) do not account for
-     * possible stop at page boundary if initial address is not page aligned,
-     * allow them to work properly
-     */
-    if ((s->sdmasysad % boundary_chk) == 0) {
-        page_aligned = true;
-    }
-
     s->prnsts |= SDHC_DATA_INHIBIT | SDHC_DAT_LINE_ACTIVE;
     if (s->trnmod & SDHC_TRNS_READ) {
         s->prnsts |= SDHC_DOING_READ;
@@ -618,7 +608,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
                 sdbus_read_data(&s->sdbus, s->fifo_buffer, block_size);
             }
             begin = s->data_count;
-            if (((boundary_count + begin) < block_size) && page_aligned) {
+            if (((boundary_count + begin) < block_size)) {
                 s->data_count = boundary_count + begin;
                 boundary_count = 0;
              } else {
@@ -634,7 +624,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
             if (s->data_count == block_size) {
                 s->data_count = 0;
             }
-            if (page_aligned && boundary_count == 0) {
+            if (boundary_count == 0) {
                 break;
             }
         }
@@ -642,7 +632,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
         s->prnsts |= SDHC_DOING_WRITE;
         while (s->blkcnt) {
             begin = s->data_count;
-            if (((boundary_count + begin) < block_size) && page_aligned) {
+            if (((boundary_count + begin) < block_size)) {
                 s->data_count = boundary_count + begin;
                 boundary_count = 0;
              } else {
@@ -659,7 +649,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
                     s->blkcnt--;
                 }
             }
-            if (page_aligned && boundary_count == 0) {
+            if (boundary_count == 0) {
                 break;
             }
         }
-- 
2.34.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]