It seems like the erase operation can write off the end of the storage,
so let's add a size guard to make sure we don't do that.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4049
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/block/m25p80.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 545e0b5728..54eb12ebbe 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -648,6 +648,13 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd)
         qemu_log_mask(LOG_GUEST_ERROR, "M25P80: erase with write protect!\n");
         return;
     }
+
+    if (offset + len >= s->size) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "M25P80: erase operation overflowing storage size!\n");
+        return;
+    }
+
     memset(s->storage + offset, 0xff, len);
     flash_sync_area(s, offset, len);
 }
-- 
2.55.0

Hi,

I’m not sure the proposed fix fully matches the actual flash behavior.

Many SPI flash devices accept any address within the sector address range to define the actual sector to erase.
It think the offset should first be masked to the sector size, then only this new test may be be applied.

Excerpt from M25P32 datasheet (section 6.9):

> The Sector Erase (SE) instruction is entered by driving Chip Select (S) Low, followed by the
> instruction code, and three address bytes on Serial Data Input (D). Any address inside the
> Sector (see Table 3) is a valid address for the Sector Erase (SE) instruction.

It seems it is already implemented for DIE_ERASE, but not SECTOR_ERASE variants for some reason.

   offset &= ~(len - 1);

Otherwise, last sector could not be erased if a non-zero sector-relative address is specified.

Emmanuel.

On 22 Jul 2026, at 17:15, Alistair Francis wrote:

> It seems like the erase operation can write off the end of the storage,
> so let's add a size guard to make sure we don't do that.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4049
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/block/m25p80.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 545e0b5728..54eb12ebbe 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -648,6 +648,13 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd)
>          qemu_log_mask(LOG_GUEST_ERROR, "M25P80: erase with write protect!\n");
>          return;
>      }
> +
> +    if (offset + len >= s->size) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "M25P80: erase operation overflowing storage size!\n");
> +        return;
> +    }
> +
>      memset(s->storage + offset, 0xff, len);
>      flash_sync_area(s, offset, len);
>  }
> -- 
> 2.55.0

Hi Alistair,

On 22/7/26 17:15, Alistair Francis wrote:
> It seems like the erase operation can write off the end of the storage,
> so let's add a size guard to make sure we don't do that.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4049
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   hw/block/m25p80.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 545e0b5728..54eb12ebbe 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -648,6 +648,13 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd)
>           qemu_log_mask(LOG_GUEST_ERROR, "M25P80: erase with write protect!\n");
>           return;
>       }
> +
> +    if (offset + len >= s->size) {

Should this be "offset + len > s->size"?

> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "M25P80: erase operation overflowing storage size!\n");
> +        return;
> +    }
> +
>       memset(s->storage + offset, 0xff, len);
>       flash_sync_area(s, offset, len);
>   }