Re: [PATCH 2/4] x86/signal: Use switch_to_permissive_pkey_reg() helper

[Dave Hansen]

...
> -/*
> - * Enable all pkeys temporarily, so as to ensure that both the current
> - * execution stack as well as the alternate signal stack are writeable.
> - * The application can use any of the available pkeys to protect the
> - * alternate signal stack, and we don't know which one it is, so enable
> - * all. The PKRU register will be reset to init_pkru later in the flow,
> - * in fpu__clear_user_states(), and it is the application's responsibility
> - * to enable the appropriate pkey as the first step in the signal handler
> - * so that the handler does not segfault.
> - */
> -static inline u32 sig_prepare_pkru(void)
> -{
> -	u32 orig_pkru = read_pkru();
> -
> -	write_pkru(0);
> -	return orig_pkru;
> -}
> -
>  /*
>   * Set up a signal frame.
>   */
> @@ -157,8 +140,18 @@ get_sigframe(struct ksignal *ksig, struct pt_regs *regs, size_t frame_size,
>  		return (void __user *)-1L;
>  	}
>  
> -	/* Update PKRU to enable access to the alternate signal stack. */
> -	pkru = sig_prepare_pkru();
> +	/*
> +	 * Enable all pkeys temporarily, so as to ensure that both the current
> +	 * execution stack as well as the alternate signal stack are
> +	 * writeable. The application can use any of the available pkeys to
> +	 * protect the alternate signal stack, and we don't know which one it
> +	 * is, so enable all. The PKRU register will be reset to init_pkru
> +	 * later in the flow, in fpu__clear_user_states(), and it is the
> +	 * application's responsibility to enable the appropriate pkey as the
> +	 * first step in the signal handler so that the handler does not
> +	 * segfault.
> +	 */
> +	pkru = switch_to_permissive_pkey_reg();
I think this hurts readability too much in the get_sigframe() code. On
some level, it's silly to have a basically empty helper. But in this
case, it does help keep the signal code readable.

In other words, this would be preferred:

/*
 * Keep existing big comment
 */
static inline u32 sig_prepare_pkru(void)
{
	return switch_to_permissive_pkey_reg();
}