[v2] hw/display: refine upper limit for offset value in assert check

[PATCH v2] hw/display: refine upper limit for offset value in assert check

gerben@altlinux.org posted 1 patch 1 year, 1 month ago

Diff against v1 v1 v3
Download mbox

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20241212160158.527246-1-gerben@altlinux.org

Maintainers: Alistair Francis <alistair@alistair23.me>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Peter Maydell <peter.maydell@linaro.org>

There is a newer version of this series

hw/display/xlnx_dp.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

Expand all Fold all

[PATCH v2] hw/display: refine upper limit for offset value in assert check

Posted by gerben@altlinux.org 1 year, 1 month ago

From: Denis Rastyogin <gerben@altlinux.org>

Accessing an element of the s->core_registers array,
which has a size of 236 (0x3AC), may lead to a buffer overflow
if the 'offset' index exceeds the valid range, potentially
reaching values up to 5139 (0x504C >> 2). Therefore, the bounds
check has been extended to DP_CORE_REG_ARRAY_SIZE (0x3B0 >> 2).
This change addresses a potential vulnerability by ensuring
the offset stays within the valid range before writing data.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Reported-by: David Meliksetyan <d.meliksetyan@fobos-nt.ru>
Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
---
 hw/display/xlnx_dp.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index 6ab2335499..3f1f5d81bd 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -896,7 +896,11 @@ static void xlnx_dp_write(void *opaque, hwaddr offset, uint64_t value,
         xlnx_dp_update_irq(s);
         break;
     default:
-        assert(offset <= (0x504C >> 2));
+        /*
+         * Check to ensure the offset is within the bounds of
+         * the core_registers[] array.
+        */
+        assert(offset < DP_CORE_REG_ARRAY_SIZE);
         s->core_registers[offset] = value;
         break;
     }
-- 
2.42.2

Re: [PATCH v2] hw/display: refine upper limit for offset value in assert check

Posted by Richard Henderson 1 year, 1 month ago

On 12/12/24 10:01, gerben@altlinux.org wrote:
> From: Denis Rastyogin <gerben@altlinux.org>
> 
> Accessing an element of the s->core_registers array,
> which has a size of 236 (0x3AC), may lead to a buffer overflow
> if the 'offset' index exceeds the valid range, potentially
> reaching values up to 5139 (0x504C >> 2). Therefore, the bounds
> check has been extended to DP_CORE_REG_ARRAY_SIZE (0x3B0 >> 2).
> This change addresses a potential vulnerability by ensuring
> the offset stays within the valid range before writing data.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Reported-by: David Meliksetyan <d.meliksetyan@fobos-nt.ru>
> Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
> ---
>   hw/display/xlnx_dp.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
> index 6ab2335499..3f1f5d81bd 100644
> --- a/hw/display/xlnx_dp.c
> +++ b/hw/display/xlnx_dp.c
> @@ -896,7 +896,11 @@ static void xlnx_dp_write(void *opaque, hwaddr offset, uint64_t value,
>           xlnx_dp_update_irq(s);
>           break;
>       default:
> -        assert(offset <= (0x504C >> 2));
> +        /*
> +         * Check to ensure the offset is within the bounds of
> +         * the core_registers[] array.
> +        */
> +        assert(offset < DP_CORE_REG_ARRAY_SIZE);

More accurately,

   The memory region is registered to match the size of
   the core_registers array. The guest cannot issue an
   out-of-bounds write.

And that's why this can be an assertion instead of needing
to issue an error to the guest.


r~