[PATCH] platform: generic: allwinner: Fix PLIC array bounds

[PATCH] platform: generic: allwinner: Fix PLIC array bounds

[Samuel Holland]

The two referenced commits passed incorrect bounds to the PLIC save/
restore functions, causing out-of-bounds memory access. The functions
expect "num" to be the 1-based number of interrupt sources, equivalent
to the "riscv,ndev" devicetree property. Thus, "num" must be strictly
smaller than the 0-based size of the array storing the register values.

However, the referenced commits incorrectly passed in the unmodified
size of the array as "num". Fix this by reducing PLIC_SOURCES (matching
"riscv,ndev" on this platform), while keeping the same array sizes.

Addresses-Coverity-ID: 1530251 ("Out-of-bounds access")
Addresses-Coverity-ID: 1530252 ("Out-of-bounds access")
Fixes: 8509e46ca63a ("lib: utils/irqchip: plic: Ensure no out-of-bound access in priority save/restore helpers")
Fixes: 9a2eeb4aaeac ("lib: utils/irqchip: plic: Ensure no out-of-bound access in context save/restore helpers")
Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 platform/generic/allwinner/sun20i-d1.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/platform/generic/allwinner/sun20i-d1.c b/platform/generic/allwinner/sun20i-d1.c
index 1da9e5b..e2b76a3 100644
--- a/platform/generic/allwinner/sun20i-d1.c
+++ b/platform/generic/allwinner/sun20i-d1.c
@@ -69,10 +69,10 @@ static void sun20i_d1_csr_restore(void)
  * PLIC
  */
 
-#define PLIC_SOURCES			176
-#define PLIC_IE_WORDS			((PLIC_SOURCES + 31) / 32)
+#define PLIC_SOURCES			175
+#define PLIC_IE_WORDS			(PLIC_SOURCES / 32 + 1)
 
-static u8 plic_priority[PLIC_SOURCES];
+static u8 plic_priority[1 + PLIC_SOURCES];
 static u32 plic_sie[PLIC_IE_WORDS];
 static u32 plic_threshold;
 
-- 
2.37.4

[PATCH] platform: generic: allwinner: Fix PLIC array bounds

[Andreas Schwab]

On Dez 27 2022, Samuel Holland wrote:

> -static u8 plic_priority[PLIC_SOURCES];
> +static u8 plic_priority[1 + PLIC_SOURCES];

This is unnecessarily wasting space.

-- 
Andreas Schwab, schwab at linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

[PATCH] platform: generic: allwinner: Fix PLIC array bounds

[Anup Patel]

On Wed, Dec 28, 2022 at 12:14 AM Samuel Holland <samuel@sholland.org> wrote:
>
> The two referenced commits passed incorrect bounds to the PLIC save/
> restore functions, causing out-of-bounds memory access. The functions
> expect "num" to be the 1-based number of interrupt sources, equivalent
> to the "riscv,ndev" devicetree property. Thus, "num" must be strictly
> smaller than the 0-based size of the array storing the register values.
>
> However, the referenced commits incorrectly passed in the unmodified
> size of the array as "num". Fix this by reducing PLIC_SOURCES (matching
> "riscv,ndev" on this platform), while keeping the same array sizes.
>
> Addresses-Coverity-ID: 1530251 ("Out-of-bounds access")
> Addresses-Coverity-ID: 1530252 ("Out-of-bounds access")
> Fixes: 8509e46ca63a ("lib: utils/irqchip: plic: Ensure no out-of-bound access in priority save/restore helpers")
> Fixes: 9a2eeb4aaeac ("lib: utils/irqchip: plic: Ensure no out-of-bound access in context save/restore helpers")
> Signed-off-by: Samuel Holland <samuel@sholland.org>

This looks similar to Heinrich's patch. I am merging this since it
came after enough discussion on Heinrich's patch.

Reviewed-by: Anup Patel <anup@brainfault.org>

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup

> ---
>
>  platform/generic/allwinner/sun20i-d1.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/platform/generic/allwinner/sun20i-d1.c b/platform/generic/allwinner/sun20i-d1.c
> index 1da9e5b..e2b76a3 100644
> --- a/platform/generic/allwinner/sun20i-d1.c
> +++ b/platform/generic/allwinner/sun20i-d1.c
> @@ -69,10 +69,10 @@ static void sun20i_d1_csr_restore(void)
>   * PLIC
>   */
>
> -#define PLIC_SOURCES                   176
> -#define PLIC_IE_WORDS                  ((PLIC_SOURCES + 31) / 32)
> +#define PLIC_SOURCES                   175
> +#define PLIC_IE_WORDS                  (PLIC_SOURCES / 32 + 1)
>
> -static u8 plic_priority[PLIC_SOURCES];
> +static u8 plic_priority[1 + PLIC_SOURCES];
>  static u32 plic_sie[PLIC_IE_WORDS];
>  static u32 plic_threshold;
>
> --
> 2.37.4
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi