Re: [PATCH v18 24/25] x86/cet/shstk: Add arch_prctl functions for shadow stack

[Dave Hansen]

On 1/29/21 10:56 AM, Yu, Yu-cheng wrote:
> On 1/29/2021 9:07 AM, Dave Hansen wrote:
>> On 1/27/21 1:25 PM, Yu-cheng Yu wrote:
>>> +    u64 buf[3] = {0, 0, 0};

Doesn't the compiler zero these if you initialize it to anything?  In
other words, doesn't this work?

	u64 buf[3] = {};

>>> +    if (cet->shstk_size) {
>>> +        buf[0] |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
>>> +        buf[1] = (u64)cet->shstk_base;
>>> +        buf[2] = (u64)cet->shstk_size;
>>
>> What's the casting for?
> 
> cet->shstk_base and cet->shstk_size are both 'unsigned long', not u64,
> so the cast.

Sure, but we don't put explicit casts at every implicit type conversion
in the kernel.  What function does this casting serve?

>>> +    cet = &current->thread.cet;
>>> +
>>> +    if (option == ARCH_X86_CET_STATUS)
>>> +        return copy_status_to_user(cet, arg2);
>>
>> What's the point of doing copy_status_to_user() if the processor doesn't
>> support CET?  In other words, shouldn't this be below the CPU feature
>> check?
> 
> The thought was to tell the difference between the kernel itself does
> not support CET and the system does not have CET.  And, if the kernel
> supports it, show CET status of the thread.

Why would that matter to userspace?

If they want to know if the processor has CET support there are existing
ways to do it.  I don't think this should be part of the ABI.