Re: [PATCH-for-9.0 v2 0/3] hw/block/nand: Fix out-of-bound access in NAND block buffer

[Philippe Mathieu-Daudé]

On 9/4/24 15:59, Philippe Mathieu-Daudé wrote:
> Fix for https://gitlab.com/qemu-project/qemu/-/issues/1446
>
> Since v1:
> - Addressed Kevin trivial suggestions (unsigned offset)

$ git backport-diff
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, 
respectively

001/       3:[0009] [FC] 'hw/block/nand: Factor nand_load_iolen() method 
out'
002/       3:[0004] [FC] 'hw/block/nand: Have blk_load() return boolean 
indicating success'
003/       3:[----] [-C] 'hw/block/nand: Fix out-of-bound access in NAND 
block buffer'

$ git diff
diff --git a/hw/block/nand.c b/hw/block/nand.c
index d90dc965a1..e2433c25bd 100644
--- a/hw/block/nand.c
+++ b/hw/block/nand.c
@@ -88,7 +88,7 @@ struct NANDFlashState {
      * Returns %true when block containing (@addr + @offset) is
      * successfully loaded, otherwise %false.
      */
-    bool (*blk_load)(NANDFlashState *s, uint64_t addr, int offset);
+    bool (*blk_load)(NANDFlashState *s, uint64_t addr, unsigned offset);

     uint32_t ioaddr_vmstate;
 };
@@ -251,18 +251,21 @@ static inline void nand_pushio_byte(NANDFlashState 
*s, uint8_t value)
  * nand_load_block: Load block containing (s->addr + @offset).
  * Returns length of data available at @offset in this block.
  */
-static int nand_load_block(NANDFlashState *s, int offset)
+static unsigned nand_load_block(NANDFlashState *s, unsigned offset)
 {
-    int iolen;
+    unsigned iolen;

     if (!s->blk_load(s, s->addr, offset)) {
         return 0;
     }

-    iolen = (1 << s->page_shift) - offset;
+    iolen = (1 << s->page_shift);
     if (s->gnd) {
         iolen += 1 << s->oob_shift;
     }
+    assert(offset <= iolen);
+    iolen -= offset;
+
     return iolen;
 }

@@ -776,7 +779,7 @@ static void glue(nand_blk_erase_, 
NAND_PAGE_SIZE)(NANDFlashState *s)
 }

 static bool glue(nand_blk_load_, NAND_PAGE_SIZE)(NANDFlashState *s,
-                uint64_t addr, int offset)
+                uint64_t addr, unsigned offset)
 {
     if (PAGE(addr) >= s->pages) {
         return false;
---

> Philippe Mathieu-Daudé (3):
>    hw/block/nand: Factor nand_load_iolen() method out
>    hw/block/nand: Have blk_load() take unsigned offset and return boolean
>    hw/block/nand: Fix out-of-bound access in NAND block buffer
>
>   hw/block/nand.c | 55 ++++++++++++++++++++++++++++++++++---------------
>   1 file changed, 38 insertions(+), 17 deletions(-)