[PATCH] hw/misc: Fix invalid size assertions in exynos4210_rng read/write functions

[PATCH] hw/misc: Fix invalid size assertions in exynos4210_rng read/write functions

[Zheyu Ma]

This commit updates the exynos4210_rng_read() and exynos4210_rng_write()
functions to handle cases where the size is not 4 bytes. Instead of
asserting, which causes the program to abort, the functions now log an
error message and return a default value for reads or do nothing for
writes when the size is invalid.

Reproducer:
cat << EOF | qemu-system-aarch64 -display none \
-machine accel=qtest, -m 512M -machine smdkc210 -qtest stdio
readb 0x10830454
EOF

Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
---
 hw/misc/exynos4210_rng.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/misc/exynos4210_rng.c b/hw/misc/exynos4210_rng.c
index 0756bd3205..307d4eea43 100644
--- a/hw/misc/exynos4210_rng.c
+++ b/hw/misc/exynos4210_rng.c
@@ -146,7 +146,12 @@ static uint64_t exynos4210_rng_read(void *opaque, hwaddr offset,
     Exynos4210RngState *s = (Exynos4210RngState *)opaque;
     uint32_t val = 0;
 
-    assert(size == 4);
+    if (size != 4) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid read size %u at offset 0x%" HWADDR_PRIx
+                      "\n", __func__, size, offset);
+        return 0;
+    }
 
     switch (offset) {
     case EXYNOS4210_RNG_CONTROL_1:
@@ -181,7 +186,12 @@ static void exynos4210_rng_write(void *opaque, hwaddr offset,
 {
     Exynos4210RngState *s = (Exynos4210RngState *)opaque;
 
-    assert(size == 4);
+    if (size != 4) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid write size %u at offset 0x%" HWADDR_PRIx
+                      "\n", __func__, size, offset);
+        return;
+    }
 
     switch (offset) {
     case EXYNOS4210_RNG_CONTROL_1:
-- 
2.34.1

Re: [PATCH] hw/misc: Fix invalid size assertions in exynos4210_rng read/write functions

[Philippe Mathieu-Daudé]

On 18/6/24 16:50, Zheyu Ma wrote:
> This commit updates the exynos4210_rng_read() and exynos4210_rng_write()
> functions to handle cases where the size is not 4 bytes. Instead of
> asserting, which causes the program to abort, the functions now log an
> error message and return a default value for reads or do nothing for
> writes when the size is invalid.
> 
> Reproducer:
> cat << EOF | qemu-system-aarch64 -display none \
> -machine accel=qtest, -m 512M -machine smdkc210 -qtest stdio
> readb 0x10830454
> EOF
> 
> Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
> ---
>   hw/misc/exynos4210_rng.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/misc/exynos4210_rng.c b/hw/misc/exynos4210_rng.c
> index 0756bd3205..307d4eea43 100644
> --- a/hw/misc/exynos4210_rng.c
> +++ b/hw/misc/exynos4210_rng.c
> @@ -146,7 +146,12 @@ static uint64_t exynos4210_rng_read(void *opaque, hwaddr offset,
>       Exynos4210RngState *s = (Exynos4210RngState *)opaque;
>       uint32_t val = 0;
>   
> -    assert(size == 4);

Here if these registers are 32-bit only:

-- >8 --
diff --git a/hw/misc/exynos4210_rng.c b/hw/misc/exynos4210_rng.c
index 0756bd3205..674d8eece5 100644
--- a/hw/misc/exynos4210_rng.c
+++ b/hw/misc/exynos4210_rng.c
@@ -217,6 +217,8 @@ static const MemoryRegionOps exynos4210_rng_ops = {
      .read = exynos4210_rng_read,
      .write = exynos4210_rng_write,
      .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 4,
  };
---

Otherwise:

-- >8 --
static const MemoryRegionOps exynos4210_rng_ops = {
     .read = exynos4210_rng_read,
     .write = exynos4210_rng_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
     .impl.min_access_size = 4,
     .impl.max_access_size = 4,
};
---

> +    if (size != 4) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: invalid read size %u at offset 0x%" HWADDR_PRIx
> +                      "\n", __func__, size, offset);
> +        return 0;
> +    }
>   
>       switch (offset) {
>       case EXYNOS4210_RNG_CONTROL_1:
> @@ -181,7 +186,12 @@ static void exynos4210_rng_write(void *opaque, hwaddr offset,
>   {
>       Exynos4210RngState *s = (Exynos4210RngState *)opaque;
>   
> -    assert(size == 4);
> +    if (size != 4) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: invalid write size %u at offset 0x%" HWADDR_PRIx
> +                      "\n", __func__, size, offset);
> +        return;
> +    }
>   
>       switch (offset) {
>       case EXYNOS4210_RNG_CONTROL_1: