| Message ID | 1497969886-17773-2-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | risu: Fix handling of ARM sigframe | expand |
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
On 20 June 2017 at 16:03, Alex Bennée <alex.bennee@linaro.org> wrote: > > 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? Probably not if it put them after the VFP registers (where you'd expect them to be), though if we supported SVE regs in risu we'd probably have found this bug in the process of getting that working ;-) thanks -- PMM
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; } }
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> --- risu_reginfo_arm.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) -- 2.7.4