Re: [Qemu-devel] [PATCH 1/2] risu_reginfo_arm.c: Fix handling of size values in sigframe

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> The code in reginfo_init_vfp() to parse the signal frame
> was mishandling the size counts:
>  * the size includes the bytes for the magic and size fields,
>    so the code to skip forward over unknown or undersize blocks
>    was adding 4 more than it should
>  * the size is in bytes but the "is this block too small"
>    test was checking against an expected size in words
>
> This didn't cause any problems because the kernel happens
> to generate signal frames with the VFP section first.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

I guess this would have tripped up once the kernel started dumping SVE
registers in the context?

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


> ---
>  risu_reginfo_arm.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/risu_reginfo_arm.c b/risu_reginfo_arm.c
> index 0cb9087..b0d5da7 100644
> --- a/risu_reginfo_arm.c
> +++ b/risu_reginfo_arm.c
> @@ -36,7 +36,12 @@ static void reginfo_init_vfp(struct reginfo *ri, ucontext_t *uc)
>      unsigned long *rs = uc->uc_regspace;
>
>      for (;;) {
> -        switch (*rs++) {
> +        unsigned long magic = *rs++;
> +        unsigned long size = *rs++;
> +
> +        size -= 8; /* Account for the magic/size fields */
> +
> +        switch (magic) {
>          case 0:
>          {
>              /* We didn't find any VFP at all (probably a no-VFP
> @@ -57,11 +62,11 @@ static void reginfo_init_vfp(struct reginfo *ri, ucontext_t *uc)
>               */
>              int i;
>              /* Skip if it's smaller than we expected (should never happen!) */
> -            if (*rs < ((32 * 2) + 1)) {
> -                rs += (*rs / 4);
> +            if (size < ((32 * 2) + 1) * 4) {
> +                rs += size / 4;
>                  break;
>              }
> -            rs++;
> +
>              for (i = 0; i < 32; i++) {
>                  ri->fpregs[i] = *rs++;
>                  ri->fpregs[i] |= (uint64_t) (*rs++) << 32;
> @@ -86,7 +91,7 @@ static void reginfo_init_vfp(struct reginfo *ri, ucontext_t *uc)
>          }
>          default:
>              /* Some other kind of block, ignore it */
> -            rs += (*rs / 4);
> +            rs += size / 4;
>              break;
>          }
>      }


--
Alex Bennée