V2 [PATCH] x86/CET: Document glibc.tune.x86_ibt and glibc.tune.x86_shstk

  On Tue, Jul 17, 2018 at 10:22 PM, Rical Jasan <rj@2c3t.io> wrote:
> On 07/17/2018 08:04 PM, H.J. Lu wrote:
>> diff --git a/manual/tunables.texi b/manual/tunables.texi
>> index be33c9fc79..7998b3b7e6 100644
>> --- a/manual/tunables.texi
>> +++ b/manual/tunables.texi
>> @@ -356,3 +356,26 @@ to set threshold in bytes for non temporal store.
>>
>>  This tunable is specific to i386 and x86-64.
>>  @end deftp
>> +
>> +@deftp Tunable glibc.tune.x86_ibt
>> +The @code{glibc.tune.x86_ibt=[on|off|permissive]} tunable allows the user> +to control how indirect branch tracking (IBT) should be enabled.
>
> It seems out of place to list the available options like that in the
> textual context here (for reasons similar to why we don't write function
> calls within paragraphs; e.g., @code{foo(x, y)}), but I do see some
> precedent with other tunables currently in the manual.  Instead, I would
> follow up the above sentence with, "Accepted values are @code{on},
> @code{off}, and @code{permissive}."

Fixed.

>> +@code{on} always turns on IBT regardless of whether IBT is enabled in the
>> +executable and its dependent shared libraries.  @code{off} always turns
>> +off IBT regardless of whether IBT is enabled in the executable and its
>> +dependent shared libraries.  @code{permissive} is the same as the default.
>
> Which is the default and what does it do?

Fixed.

>> +This tunable is specific to i386 and x86-64.
>> +@end deftp
>> +
>> +@deftp Tunable glibc.tune.x86_shstk
>> +The @code{glibc.tune.x86_shstk=[on|off|permissive]} tunable allows the
>> +user to control how shadow stack (SHSTK) should be enabled.  @code{on}
>
> Should "shadow stack" be prefixed with a definite article ("the shadow
> stack")?  Similarly for SHSTK below.

I don't think SHSTK needs a "the" prefix as we don't use the SSE.

>> +always turns on SHSTK regardless of whether SHSTK is enabled in the
>> +executable and its dependent shared libraries.  @code{off} always turns
>> +off SHSTK regardless of whether SHSTK is enabled in the executable and
>> +its dependent shared libraries.  @code{permissive} turns off SHSTK when
>> +dlopening a legacy shared library, instead of returns an error.
>
> "instead of returning"?

Fixed.

> Also, what does "legacy" mean in this context?

Fixed/

>> +This tunable is specific to i386 and x86-64.
>> +@end deftp
>

Here is the updated patch.  OK for master?

Thanks.
  

On 07/18/2018 09:44 AM, H.J. Lu wrote:
...
> diff --git a/manual/tunables.texi b/manual/tunables.texi
> index be33c9fc79..13426ce238 100644
> --- a/manual/tunables.texi
> +++ b/manual/tunables.texi
> @@ -356,3 +356,31 @@ to set threshold in bytes for non temporal store.
>  
>  This tunable is specific to i386 and x86-64.
>  @end deftp
> +
> +@deftp Tunable glibc.tune.x86_ibt
> +The @code{glibc.tune.x86_ibt=[on|off|permissive]} tunable allows the user

I meant to just use @code{glibc.tune.x86_ibt} here, and then list the
options after, like you did below.

> +to control how indirect branch tracking (IBT) should be enabled.  Accepted
> +values are @code{on}, @code{off}, and @code{permissive}.  @code{on} always
> +turns on IBT regardless of whether IBT is enabled in the executable and
> +its dependent shared libraries.  @code{off} always turns off IBT regardless
> +of whether IBT is enabled in the executable and its dependent shared
> +libraries.  @code{permissive} is the same as the default which disables
> +IBT on non-CET executables and shared libraries.
> +
> +This tunable is specific to i386 and x86-64.
> +@end deftp
> +
> +@deftp Tunable glibc.tune.x86_shstk
> +The @code{glibc.tune.x86_shstk=[on|off|permissive]} tunable allows the

And here.

> +user to control how the shadow stack (SHSTK) should be enabled.  Accepted
> +values are @code{on}, @code{off}, and @code{permissive}.  @code{on}
> +always turns on SHSTK regardless of whether SHSTK is enabled in the
> +executable and its dependent shared libraries.  @code{off} always turns
> +off SHSTK regardless of whether SHSTK is enabled in the executable and
> +its dependent shared libraries.  @code{permissive} changes how dlopen
> +works on non-CET shared libraries.  By default, when SHSTK is enabled,
> +dlopening a non-CET shared library returns an error.  With
> +@code{permissive}, it turns off SHSTK instead.
> +
> +This tunable is specific to i386 and x86-64.
> +@end deftp
> -- 2.17.1

The permissive explanations are much better, thank you.

Rical