[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] target/mips/mxu_translate.c: Fix array overrun for D16MIN/D1
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH] target/mips/mxu_translate.c: Fix array overrun for D16MIN/D16MAX |
Date: |
Mon, 22 Mar 2021 11:19:25 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 |
On 3/16/21 2:13 PM, Peter Maydell wrote:
> Coverity reported (CID 1450831) an array overrun in
> gen_mxu_D16MAX_D16MIN():
>
> 1103 } else if (unlikely((XRb == 0) || (XRa == 0))) {
> ....
> 1112 if (opc == OPC_MXU_D16MAX) {
> 1113 tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> 1114 } else {
> 1115 tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> 1116 }
>
>>>> Overrunning array "mxu_gpr" of 15 8-byte elements at element
> index 4294967295 (byte offset 34359738367) using index "XRa - 1U"
> (which evaluates to 4294967295).
>
> This happens because the code is confused about which of XRa, XRb and
> XRc is the output, and which are the inputs. XRa is the output, but
> most of the conditions separating out different special cases are
> written as if XRc is the output, with the result that we can end up
> in the code path that assumes XRa is non-0 even when it is zero.
>
> Fix the erroneous code, bringing it in to line with the structure
> used in functions like gen_mxu_S32MAX_S32MIN() and
> gen_mxu_Q8MAX_Q8MIN().
>
> Fixes: CID 1450831
> Fixes: bb84cbf38505bd1d8
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> NB: tested with 'make check' and 'make check-acceptance' only, which
> almost certainly don't exercise this code path.
>
> target/mips/mxu_translate.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
Thanks, applied to mips-fixes.