Re: [PATCH v11 6/9] x86/cet: Add PTRACE interface for CET

[Dave Hansen <dave.hansen@intel.com>]

On 9/2/20 9:35 PM, Andy Lutomirski wrote:
>>>>>> +       fpu__prepare_read(fpu);
>>>>>> +       cetregs = get_xsave_addr(&fpu->state.xsave, XFEATURE_CET_USER);
>>>>>> +       if (!cetregs)
>>>>>> +               return -EFAULT;
>>>>> Can this branch ever be hit without a kernel bug? If yes, I think
>>>>> -EFAULT is probably a weird error code to choose here. If no, this
>>>>> should probably use WARN_ON(). Same thing in cetregs_set().
>>>> When a thread is not CET-enabled, its CET state does not exist.  I looked at EFAULT, and it means "Bad address".  Maybe this can be ENODEV, which means "No such device"?
> Having read the code, I’m unconvinced. It looks like a get_xsave_addr() failure means “state not saved; task sees INIT state”.  So *maybe* it’s reasonable -ENODEV this, but I’m not really convinced. I tend to think we should return the actual INIT state and that we should permit writes and handle them correctly.

PTRACE is asking for access to the values in the *registers*, not for
the value in the kernel XSAVE buffer.  We just happen to only have the
kernel XSAVE buffer around.

If we want to really support PTRACE we have to allow the registers to be
get/set, regardless of what state they are in, INIT state or not.  So,
yeah I agree with Andy.