On Mon, Feb 05, 2024 at 04:48:44PM +0100, Heiko Carstens wrote:
> Code sections in s390 specific kernel code which use floating point or
> vector registers all come with a 520 byte stack variable to save already in
> use registers, if required.
>
> With INIT_STACK_ALL_PATTERN or INIT_STACK_ALL_ZERO enabled this variable
> will always be initialized on function entry in addition to saving register
> contents, which contradicts the intend (performance improvement) of such
> code sections.
>
> Therefore provide a DECLARE_KERNEL_FPU_ONSTACK() macro which provides
> struct kernel_fpu variables with an __uninitialized attribute, and convert
> all existing code to use this.
>
> This way only this specific type of stack variable will not be initialized,
> regardless of config options.
>
> Signed-off-by: Heiko Carstens <hca@xxxxxxxxxxxxx>
> ---
> arch/s390/crypto/chacha-glue.c | 2 +-
> arch/s390/crypto/crc32-vx.c | 2 +-
> arch/s390/include/asm/fpu/types.h | 3 +++
> arch/s390/kernel/sysinfo.c | 2 +-
> lib/raid6/s390vx.uc | 4 ++--
> 5 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/arch/s390/crypto/chacha-glue.c b/arch/s390/crypto/chacha-glue.c
> index ed9959e6f714..a823132fc563 100644
> --- a/arch/s390/crypto/chacha-glue.c
> +++ b/arch/s390/crypto/chacha-glue.c
> @@ -22,7 +22,7 @@ static void chacha20_crypt_s390(u32 *state, u8 *dst, const u8 *src,
> unsigned int nbytes, const u32 *key,
> u32 *counter)
> {
> - struct kernel_fpu vxstate;
> + DECLARE_KERNEL_FPU_ONSTACK(vxstate);
>
> kernel_fpu_begin(&vxstate, KERNEL_VXR);
> chacha20_vx(dst, src, nbytes, key, counter);
> diff --git a/arch/s390/crypto/crc32-vx.c b/arch/s390/crypto/crc32-vx.c
> index 017143e9cef7..6ae3e3ff5b0a 100644
> --- a/arch/s390/crypto/crc32-vx.c
> +++ b/arch/s390/crypto/crc32-vx.c
> @@ -49,8 +49,8 @@ u32 crc32c_le_vgfm_16(u32 crc, unsigned char const *buf, size_t size);
> static u32 __pure ___fname(u32 crc, \
> unsigned char const *data, size_t datalen) \
> { \
> - struct kernel_fpu vxstate; \
> unsigned long prealign, aligned, remaining; \
> + DECLARE_KERNEL_FPU_ONSTACK(vxstate); \
> \
> if (datalen < VX_MIN_LEN + VX_ALIGN_MASK) \
> return ___crc32_sw(crc, data, datalen); \
> diff --git a/arch/s390/include/asm/fpu/types.h b/arch/s390/include/asm/fpu/types.h
> index d889e9436865..b1afa13c07b7 100644
> --- a/arch/s390/include/asm/fpu/types.h
> +++ b/arch/s390/include/asm/fpu/types.h
> @@ -35,4 +35,7 @@ struct kernel_fpu {
> };
> };
>
> +#define DECLARE_KERNEL_FPU_ONSTACK(name) \
> + struct kernel_fpu name __uninitialized
Are there cases when struct kernel_fpu should be initialized? e.g.
should the attribute just be added to the struct definition instead of
marking each use?
-Kees
> +
> #endif /* _ASM_S390_FPU_TYPES_H */
> diff --git a/arch/s390/kernel/sysinfo.c b/arch/s390/kernel/sysinfo.c
> index f6f8f498c9be..b439f17516eb 100644
> --- a/arch/s390/kernel/sysinfo.c
> +++ b/arch/s390/kernel/sysinfo.c
> @@ -426,9 +426,9 @@ subsys_initcall(create_proc_service_level);
> */
> void s390_adjust_jiffies(void)
> {
> + DECLARE_KERNEL_FPU_ONSTACK(fpu);
> struct sysinfo_1_2_2 *info;
> unsigned long capability;
> - struct kernel_fpu fpu;
>
> info = (void *) get_zeroed_page(GFP_KERNEL);
> if (!info)
> diff --git a/lib/raid6/s390vx.uc b/lib/raid6/s390vx.uc
> index cd0b9e95f499..7b0b355e1a26 100644
> --- a/lib/raid6/s390vx.uc
> +++ b/lib/raid6/s390vx.uc
> @@ -81,7 +81,7 @@ static inline void COPY_VEC(int x, int y)
>
> static void raid6_s390vx$#_gen_syndrome(int disks, size_t bytes, void **ptrs)
> {
> - struct kernel_fpu vxstate;
> + DECLARE_KERNEL_FPU_ONSTACK(vxstate);
> u8 **dptr, *p, *q;
> int d, z, z0;
>
> @@ -114,7 +114,7 @@ static void raid6_s390vx$#_gen_syndrome(int disks, size_t bytes, void **ptrs)
> static void raid6_s390vx$#_xor_syndrome(int disks, int start, int stop,
> size_t bytes, void **ptrs)
> {
> - struct kernel_fpu vxstate;
> + DECLARE_KERNEL_FPU_ONSTACK(vxstate);
> u8 **dptr, *p, *q;
> int d, z, z0;
>
> --
> 2.40.1
>
--
Kees Cook
