From: Alistair Francis <alistair23@gmail.com>
To: Qiang Liu <cyruscyliu@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
Alistair Francis <alistair@alistair23.me>,
"open list:All patches CC here" <qemu-devel@nongnu.org>,
"open list:Xilinx ZynqMP and..." <qemu-arm@nongnu.org>,
"Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Subject: Re: [PATCH] hw/display/xlnx_dp: fix an out-of-bounds read in xlnx_dp_read
Date: Tue, 13 Jul 2021 13:54:37 +1000 [thread overview]
Message-ID: <CAKmqyKO3vHYiS_fDqnQh66jjXDZUvxx2XYxKbwNzgHJWBc_gfw@mail.gmail.com> (raw)
In-Reply-To: <1626146083-19434-1-git-send-email-cyruscyliu@gmail.com>
On Tue, Jul 13, 2021 at 1:15 PM Qiang Liu <cyruscyliu@gmail.com> wrote:
>
> xlnx_dp_read allows an out-of-bounds read at its default branch because
> of an improper index.
>
> According to
> https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-ultrascale-registers.html
> (DP Module), registers 0x3A4/0x3A4/0x3AC are allowed.
>
> DP_INT_MASK 0x000003A4 32 mixed 0xFFFFF03F Interrupt Mask Register for intrN.
> DP_INT_EN 0x000003A8 32 mixed 0x00000000 Interrupt Enable Register.
> DP_INT_DS 0x000003AC 32 mixed 0x00000000 Interrupt Disable Register.
>
> In xlnx_dp_write, when the offset is 0x3A8 and 0x3AC, the virtual device
> will write s->core_registers[0x3A4
> >> 2]. That is to say, the maxize of s->core_registers could be ((0x3A4
> >> 2) + 1). However, the current size of s->core_registers is (0x3AF >>
> >> 2), that is ((0x3A4 >> 2) + 2), which is out of the range.
> In xlxn_dp_read, the access to offset 0x3A8 or 0x3AC will be directed to
> the offset 0x3A8 (incorrect functionality) or 0x3AC (out-of-bounds read)
> rather than 0x3A4.
>
> This patch adjusts the size of s->core_registers and enforces the read
> access to offset 0x3A* and 0x3AC to 0x3A4. BTW, because the size of this
> MMIO region is 0x3AF, this patch also removes the assertion in
> xlnx_dp_write.
>
> Fixes: 58ac482a66de ("introduce xlnx-dp")
> Signed-off-by: Qiang Liu <cyruscyliu@gmail.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/display/xlnx_dp.c | 7 ++++---
> include/hw/display/xlnx_dp.h | 2 +-
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
> index 7bcbb13..8903181 100644
> --- a/hw/display/xlnx_dp.c
> +++ b/hw/display/xlnx_dp.c
> @@ -713,8 +713,10 @@ static uint64_t xlnx_dp_read(void *opaque, hwaddr offset, unsigned size)
> ret = 0;
> break;
> default:
> - assert(offset <= (0x3AC >> 2));
> - ret = s->core_registers[offset];
> + if (offset == (0x3A8 >> 2) || offset == (0x3AC >> 2))
> + ret = s->core_registers[DP_INT_MASK];
> + else
> + ret = s->core_registers[offset];
> break;
> }
>
> @@ -876,7 +878,6 @@ static void xlnx_dp_write(void *opaque, hwaddr offset, uint64_t value,
> xlnx_dp_update_irq(s);
> break;
> default:
> - assert(offset <= (0x504C >> 2));
> s->core_registers[offset] = value;
> break;
> }
> diff --git a/include/hw/display/xlnx_dp.h b/include/hw/display/xlnx_dp.h
> index e85e428..99a6d47 100644
> --- a/include/hw/display/xlnx_dp.h
> +++ b/include/hw/display/xlnx_dp.h
> @@ -39,7 +39,7 @@
> #define AUD_CHBUF_MAX_DEPTH (32 * KiB)
> #define MAX_QEMU_BUFFER_SIZE (4 * KiB)
>
> -#define DP_CORE_REG_ARRAY_SIZE (0x3AF >> 2)
> +#define DP_CORE_REG_ARRAY_SIZE (0x3A8 >> 2)
> #define DP_AVBUF_REG_ARRAY_SIZE (0x238 >> 2)
> #define DP_VBLEND_REG_ARRAY_SIZE (0x1DF >> 2)
> #define DP_AUDIO_REG_ARRAY_SIZE (0x50 >> 2)
> --
> 2.7.4
>
>
next prev parent reply other threads:[~2021-07-13 3:56 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-13 3:14 [PATCH] hw/display/xlnx_dp: fix an out-of-bounds read in xlnx_dp_read Qiang Liu
2021-07-13 3:54 ` Alistair Francis [this message]
2021-07-13 10:20 ` Philippe Mathieu-Daudé
2021-07-13 10:24 ` Philippe Mathieu-Daudé
2021-07-13 11:56 ` Qiang Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKmqyKO3vHYiS_fDqnQh66jjXDZUvxx2XYxKbwNzgHJWBc_gfw@mail.gmail.com \
--to=alistair23@gmail.com \
--cc=alistair@alistair23.me \
--cc=cyruscyliu@gmail.com \
--cc=edgar.iglesias@gmail.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.