Re: [PULL 06/15] target/s390x: vxeh2: vector string search

[Peter Maydell]

On Wed, 4 May 2022 at 12:55, Thomas Huth <thuth@redhat.com> wrote:
>
> From: David Miller <dmiller423@gmail.com>

Hi; Coverity thinks there might be an unintentional overflow
in a shift in this function (CID 1488734). This particular
Coverity heuristic seems to be very prone to false positives, so
if you think it's wrong I'll just mark it off as an FP in the web UI.

> +static int vstrs(S390Vector *v1, const S390Vector *v2, const S390Vector *v3,
> +                 const S390Vector *v4, uint8_t es, bool zs)
> +{
> +    int substr_elen, substr_0, str_elen, i, j, k, cc;
> +    int nelem = 16 >> es;
> +    bool eos = false;
> +
> +    substr_elen = s390_vec_read_element8(v4, 7) >> es;
> +
> +    /* If ZS, bound substr length by min(nelem, strlen(v3)). */

[..]

> + done:
> +    s390_vec_write_element64(v1, 0, k << es);

Specifically here, because k is 32 bit but s390_vec_write_element64()
takes a uint64_t argument, we will do the shift as a signed 32 bit
value before widening to 64 bits, so if the values of 'k' and 'es'
are such that we might shift beyond bit 32 we'll get the wrong
value. It looks like 'es' is one of the MO_* values, so generally
small, but the upper bound on 'k' is a bit less obvious to me.
Is the overflow-of-32-bits case impossible?

> +    s390_vec_write_element64(v1, 1, 0);
> +    return cc;
> +}

thanks
-- PMM