Re: [PATCH] target/mips/mxu_translate.c: Fix array overrun for D16MIN/D16MAX

[Philippe Mathieu-Daudé]

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.