Re: [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64

[Kees Cook <keescook at chromium dot org>]

On Fri, Dec 2, 2016 at 2:49 PM, Yao Qi <qiyaoltc@gmail.com> wrote:
> On 16-12-02 13:46:13, Kees Cook wrote:
>> When running a 32-bit ARM inferior on a 64-bit ARM host, only the hardware
>> floating-point registers (NT_ARM_VFP) are available. If the inferior
>> uses hard-float, do not request soft-float registers (NT_PRFPREG) and
>> run the risk of failing with EINVAL. This is most noticeably exposed
>
> "soft-float" is not accurate.  FPA is coprocessor.  Both VFP and FPA is
> implemented in the combination of software and hardware.  I'd like to
> rewrite the commit log like this,
>
> "When running a 32-bit ARM inferior with a 32-bit ARM GDB on 64-bit
> AArch64 host, only VFP registers (NT_ARM_VFP) are available.  The FPA
> registers (NT_PRFPREG) is not available."

That would be fine by me. I was kind of scratching my head over the
naming of the types of floating-point registers. :) Whatever the case,
arm64 doesn't support FPA, so an inferior using FPA couldn't run there
to start with. :)

>> when running "generate-core-file":
>>
>> (gdb) generate-core-file myprog.core
>> Unable to fetch the floating point registers.: Invalid argument.
>>
>> ptrace(PTRACE_GETREGSET, 27642, NT_FPREGSET, 0xffcc67f0) = -1 EINVAL (Invalid argument)
>>
>> gdb/ChangeLog:
>>
>> 2016-12-02  Kees Cook  <keescook@google.com>
>
> You don't have FSF copyright assignment.

Oh, hm, I thought there might be a Google-contributions-to-gdb one I
was already covered under. What's the best approach for me to take to
fix this?

>>       * gdb/arm-linux-nat.c: Skip soft-float registers when using hard-float.
>>
>> ---
>>  gdb/arm-linux-nat.c | 14 +++++++++-----
>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
>> index d11bdc6..2126cd7 100644
>> --- a/gdb/arm-linux-nat.c
>> +++ b/gdb/arm-linux-nat.c
>> @@ -384,17 +384,19 @@ arm_linux_fetch_inferior_registers (struct target_ops *ops,
>>    if (-1 == regno)
>>      {
>>        fetch_regs (regcache);
>> -      fetch_fpregs (regcache);
>
> We should only call fetch_fpregs if tdep->have_fpa_registers is true.

I couldn't determine how this was handled. What actually sets
org.gnu.gdb.arm.fpa in tdesc? I found gdb/features/arm/arm-fpa.xml and
seems to imply it's always included with arm? I wasn't able to follow,
but it seemed like _having_ VFP was a indicator that FPA wasn't used.

>
>>        if (tdep->have_wmmx_registers)
>>       fetch_wmmx_regs (regcache);
>>        if (tdep->vfp_register_count > 0)
>>       fetch_vfp_regs (regcache);
>> +      else
>> +     fetch_fpregs (regcache);
>>      }
>> -  else
>> +  else
>>      {
>>        if (regno < ARM_F0_REGNUM || regno == ARM_PS_REGNUM)
>>       fetch_regs (regcache);
>> -      else if (regno >= ARM_F0_REGNUM && regno <= ARM_FPS_REGNUM)
>> +      else if (tdep->vfp_register_count == 0
>> +            && regno >= ARM_F0_REGNUM && regno <= ARM_FPS_REGNUM)
>>       fetch_fpregs (regcache);
>
> Do we really need this change?  If FPA registers are not available,
> REGNO can't fall in this range (ARM_F0_REGNUM, ARM_FPS_REGNUM).
>
> These two comments above also apply to store registers.

It seemed like a reasonable change to make, but I see your point.

-Kees

-- 
Kees Cook
Nexus Security