Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 27 Aug 2022 20:23:17 +0200
From:   Heiko Carstens <hca@linux.ibm.com>
To:     Josh Poimboeuf <jpoimboe@kernel.org>
Cc:     Vasily Gorbik <gor@linux.ibm.com>,
        Alexander Gordeev <agordeev@linux.ibm.com>,
        linux-s390@vger.kernel.org,
        Christian Borntraeger <borntraeger@linux.ibm.com>,
        Sven Schnelle <svens@linux.ibm.com>,
        linux-kernel@vger.kernel.org,
        Sumanth Korikkar <sumanthk@linux.ibm.com>
Subject: Re: [PATCH RFC] s390: Fix nospec table alignments
Message-ID: <YwphFXr+pHxf4PGT@osiris>
References: <8719bf1ce4a72ebdeb575200290094e9ce047bcc.1661557333.git.jpoimboe@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <8719bf1ce4a72ebdeb575200290094e9ce047bcc.1661557333.git.jpoimboe@kernel.org>
Precedence: bulk

On Fri, Aug 26, 2022 at 04:55:44PM -0700, Josh Poimboeuf wrote:
> Add proper alignment for .nospec_call_table and .nospec_return_table in
> vmlinux.
> 
> Fixes: f19fbd5ed642 ("s390: introduce execute-trampolines for branches")
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
> This is RFC because I don't know anything about s390 behavior for
> unaligned data accesses, but this seemed to fix an issue for me.
> 
> While working on another s390 issue, I was getting intermittent boot
> failures in __nospec_revert() when it tried to access 'instr[0]'.  I
> noticed the __nospec_call_start address ended in 'ff'.  This patch
> seemed to fix it.  I have no idea why it was (only sometimes) failing in
> the first place.
> 
> The intermittent part of it is probably at least partially explained by
> CONFIG_RANDOMIZE_BASE.  Except now I can no longer recreate it :-/
> 
> Regardless, this patch seems correct.  I just can't explain what I saw.
> Any ideas?
> 
>  arch/s390/kernel/vmlinux.lds.S | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/s390/kernel/vmlinux.lds.S b/arch/s390/kernel/vmlinux.lds.S
> index 2e526f11b91e..5ea3830af0cc 100644
> --- a/arch/s390/kernel/vmlinux.lds.S
> +++ b/arch/s390/kernel/vmlinux.lds.S
> @@ -131,6 +131,7 @@ SECTIONS
>  	/*
>  	 * Table with the patch locations to undo expolines
>  	*/
> +	. = ALIGN(4);
>  	.nospec_call_table : {
>  		__nospec_call_start = . ;
>  		*(.s390_indirect*)

On s390 labels must be at an even address due to instructions like
"larl" (load address relative long), which generate a pc-relative
address adding the number of words encoded into the instruction to the
current IP.

With commit e6ed91fd0768 ("s390/alternatives: remove padding
generation code") I managed to reduce the size of struct alt_instr by
one byte, so it is now only 11 bytes in size. So depending on the size
/ number of members of the __alt_instructions array nospec_call_table
starts at an uneven address, which would explain this.

Unfortunately I was unable to let any compiler generate code, that
would use the larl instruction. Instead the address of
nospec_call_table was loaded indirectly via the GOT, which again works
always, regardless if the table starts at an even or uneven address.

This needs to be fixed anyway, and your patch certainly is correct.

Could you maybe share your kernel config + compiler version, if you
are still able to reproduce this?