linux-kernel - Re: [PATCH v6 07/14] riscv: prevent compressed instructions in alternatives

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <mhng-edc0da56-7fc8-4287-8856-9aac0f7465c1@palmer-ri-x1c9>
Date:   Mon, 07 Mar 2022 16:47:21 -0800 (PST)
From:   Palmer Dabbelt <palmer@...belt.com>
To:     heiko@...ech.de
CC:     Paul Walmsley <paul.walmsley@...ive.com>, aou@...s.berkeley.edu,
        linux-riscv@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, robh+dt@...nel.org, wefu@...hat.com,
        liush@...winnertech.com, guoren@...nel.org, atishp@...shpatra.org,
        anup@...infault.org, drew@...gleboard.org,
        Christoph Hellwig <hch@....de>, Arnd Bergmann <arnd@...db.de>,
        wens@...e.org, maxime@...no.tech, gfavor@...tanamicro.com,
        andrea.mondelli@...wei.com, behrensj@....edu, xinhaoqu@...wei.com,
        huffman@...ence.com, mick@....forth.gr,
        allen.baum@...erantotech.com, jscheid@...tanamicro.com,
        rtrauben@...il.com, samuel@...lland.org, cmuellner@...ux.com,
        philipp.tomsich@...ll.eu, heiko@...ech.de
Subject:     Re: [PATCH v6 07/14] riscv: prevent compressed instructions in alternatives

On Wed, 09 Feb 2022 04:37:53 PST (-0800), heiko@...ech.de wrote:
> Instructions are opportunistically compressed by the RISC-V assembler
> when possible, but in alternatives-blocks both the old and new content
> need to be the same size, so having the toolchain do somewhat random
> optimizations will cause strange side-effects like
> "attempt to move .org backwards" compile-time errors.
>
> Already a simple "and" used in alternatives assembly will cause these
> mismatched code sizes.

There should probably be a ".option norelax" in here as well, as 
relaxation will trigger exactly the same issues.  That, or we could just 
remove the constraint that these must be the same size (ie, 
automatically pad the smaller one with NOP/jump-to-end).

> So prevent compressed instructions to be generated in alternatives-
> code and use option-push and -pop to only limit this to the relevant
> code blocks
>
> Signed-off-by: Heiko Stuebner <heiko@...ech.de>
> ---
>  arch/riscv/include/asm/alternative-macros.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
> index c0fb11fad631..3a52884bf23d 100644
> --- a/arch/riscv/include/asm/alternative-macros.h
> +++ b/arch/riscv/include/asm/alternative-macros.h
> @@ -19,7 +19,10 @@
>  	.popsection
>  	.subsection 1
>  888 :
> +	.option push
> +	.option norvc
>  	\new_c
> +	.option pop
>  889 :
>  	.previous
>  	.org    . - (889b - 888b) + (887b - 886b)
> @@ -29,7 +32,10 @@
>
>  .macro __ALTERNATIVE_CFG old_c, new_c, vendor_id, errata_id, enable
>  886 :
> +	.option push
> +	.option norvc
>  	\old_c
> +	.option pop
>  887 :
>  	ALT_NEW_CONTENT \vendor_id, \errata_id, \enable, \new_c
>  .endm
> @@ -40,7 +46,10 @@
>  .macro __ALTERNATIVE_CFG_2 old_c, new_c_1, vendor_id_1, errata_id_1, enable_1, \
>  				  new_c_2, vendor_id_2, errata_id_2, enable_2
>  886 :
> +	.option push
> +	.option norvc
>  	\old_c
> +	.option pop
>  887 :
>  	ALT_NEW_CONTENT \vendor_id_1, \errata_id_1, \enable_1, \new_c_1
>  	ALT_NEW_CONTENT \vendor_id_2, \errata_id_2, \enable_2, \new_c_2
> @@ -70,7 +79,10 @@
>  	".popsection\n"							\
>  	".subsection 1\n"						\
>  	"888 :\n"							\
> +	".option push\n"						\
> +	".option norvc\n"						\
>  	new_c "\n"							\
> +	".option pop\n"							\
>  	"889 :\n"							\
>  	".previous\n"							\
>  	".org	. - (887b - 886b) + (889b - 888b)\n"			\
> @@ -79,7 +91,10 @@
>
>  #define __ALTERNATIVE_CFG(old_c, new_c, vendor_id, errata_id, enable)	\
>  	"886 :\n"							\
> +	".option push\n"						\
> +	".option norvc\n"						\
>  	old_c "\n"							\
> +	".option pop\n"							\
>  	"887 :\n"							\
>  	ALT_NEW_CONTENT(vendor_id, errata_id, enable, new_c)
>
> @@ -89,7 +104,10 @@
>  #define __ALTERNATIVE_CFG_2(old_c, new_c_1, vendor_id_1, errata_id_1, enable_1, \
>  				  new_c_2, vendor_id_2, errata_id_2, enable_2) \
>  	"886 :\n"							\
> +	".option push\n"						\
> +	".option norvc\n"						\
>  	old_c "\n"							\
> +	".option pop\n"							\
>  	"887 :\n"							\
>  	ALT_NEW_CONTENT(vendor_id_1, errata_id_1, enable_1, new_c_1)	\
>  	ALT_NEW_CONTENT(vendor_id_2, errata_id_2, enable_2, new_c_2)