[PATCH] hw/misc/allwinner-r40-ccu.c: Correct handling of out of range accesses

Peter Maydell posted 1 patch 2 weeks, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260709104802.1989086-1-peter.maydell@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
hw/misc/allwinner-r40-ccu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH] hw/misc/allwinner-r40-ccu.c: Correct handling of out of range accesses
Posted by Peter Maydell 2 weeks, 2 days ago
In allwinner_r40_ccu_write() we handle writes to a MemoryRegion of
size AW_R40_CCU_IOSIZE, and the register array is sized accordingly
at (AW_R40_CCU_IOSIZE / sizeof(uint32_t)).  However, one of the cases
in the switch is a range up to AW_R40_CCU_IOSIZE, which makes
Coverity think we might index off the end of the array. We also
have a similar case in the read function, but since that returns
early it doesn't have the same issue.

Adjust the handling of out of range accesses:
 - use AW_R40_CCU_IOSIZE - 4 as the upper bound, as this is the
   largest value we will actually see
 - return early in the write case, as we do in the read case

Coverity CID: 1663687
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/allwinner-r40-ccu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/misc/allwinner-r40-ccu.c b/hw/misc/allwinner-r40-ccu.c
index 8ba4e7aa42..00840e9850 100644
--- a/hw/misc/allwinner-r40-ccu.c
+++ b/hw/misc/allwinner-r40-ccu.c
@@ -71,7 +71,7 @@ static uint64_t allwinner_r40_ccu_read(void *opaque, hwaddr offset,
     const uint32_t idx = REG_INDEX(offset);
 
     switch (offset) {
-    case 0x324 ... AW_R40_CCU_IOSIZE:
+    case 0x324 ... AW_R40_CCU_IOSIZE - 4:
         qemu_log_mask(LOG_GUEST_ERROR, "%s: out-of-bounds offset 0x%04x\n",
                       __func__, (uint32_t)offset);
         return 0;
@@ -113,10 +113,10 @@ static void allwinner_r40_ccu_write(void *opaque, hwaddr offset,
             val |= REG_PLL_LOCK;
         }
         break;
-    case 0x324 ... AW_R40_CCU_IOSIZE:
+    case 0x324 ... AW_R40_CCU_IOSIZE - 4:
         qemu_log_mask(LOG_GUEST_ERROR, "%s: out-of-bounds offset 0x%04x\n",
                       __func__, (uint32_t)offset);
-        break;
+        return;
     default:
         qemu_log_mask(LOG_UNIMP, "%s: unimplemented write offset 0x%04x\n",
                       __func__, (uint32_t)offset);
-- 
2.43.0
Re: [PATCH] hw/misc/allwinner-r40-ccu.c: Correct handling of out of range accesses
Posted by Philippe Mathieu-Daudé 2 weeks, 2 days ago
On 9/7/26 12:48, Peter Maydell wrote:
> In allwinner_r40_ccu_write() we handle writes to a MemoryRegion of
> size AW_R40_CCU_IOSIZE, and the register array is sized accordingly
> at (AW_R40_CCU_IOSIZE / sizeof(uint32_t)).  However, one of the cases
> in the switch is a range up to AW_R40_CCU_IOSIZE, which makes
> Coverity think we might index off the end of the array. We also
> have a similar case in the read function, but since that returns
> early it doesn't have the same issue.
> 
> Adjust the handling of out of range accesses:
>   - use AW_R40_CCU_IOSIZE - 4 as the upper bound, as this is the
>     largest value we will actually see
>   - return early in the write case, as we do in the read case
> 
> Coverity CID: 1663687
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/misc/allwinner-r40-ccu.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH] hw/misc/allwinner-r40-ccu.c: Correct handling of out of range accesses
Posted by Strahinja Jankovic 2 weeks, 2 days ago
On Thu, Jul 9, 2026 at 1:03 PM Philippe Mathieu-Daudé <
philmd@oss.qualcomm.com> wrote:

> On 9/7/26 12:48, Peter Maydell wrote:
> > In allwinner_r40_ccu_write() we handle writes to a MemoryRegion of
> > size AW_R40_CCU_IOSIZE, and the register array is sized accordingly
> > at (AW_R40_CCU_IOSIZE / sizeof(uint32_t)).  However, one of the cases
> > in the switch is a range up to AW_R40_CCU_IOSIZE, which makes
> > Coverity think we might index off the end of the array. We also
> > have a similar case in the read function, but since that returns
> > early it doesn't have the same issue.
> >
> > Adjust the handling of out of range accesses:
> >   - use AW_R40_CCU_IOSIZE - 4 as the upper bound, as this is the
> >     largest value we will actually see
> >   - return early in the write case, as we do in the read case
> >
> > Coverity CID: 1663687
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   hw/misc/allwinner-r40-ccu.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
>

Reviewed-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>