[PATCH v1 1/2] RISC-V: skip parsing multi-letter extensions starting with caps

[Andrew Jones]

On Wed, Apr 26, 2023 at 11:43:24AM +0100, Conor Dooley wrote:
> Yangyu Chen reported that if an multi-letter extension begins with a
> capital letter the parser will treat the remainder of that multi-letter
> extension as single-letter extensions.

I think the problem is that the parser doesn't completely abort when
it sees something it doesn't understand. Continuing is risky since
it may be possible to compose an invalid string that gets the parser
to run off the rails.

How about completely aborting, noisily, when the string doesn't match
expectations, falling back to a default string such as rv64ima instead.
That also ought to get faster corrections of device trees.

> 
> Certain versions of rocket-chip will export devicetree containing
> rv64ima_Zifencei, which is parsed by the kernel as rv64imafc.
> 
> While capital letters in riscv,isa are invalid and the validation of
> devicetree's isn't the kernel's job, we should behave more gracefully
> here. Rather than abort parsing on meeting a capital letter, mark the
> extension as an error & allow the parser to skip ahead to the next
> extension.
> 
> Reported-by: Yangyu Chen <cyy at cyyself.name>
> Link: https://lore.kernel.org/all/tencent_1647475C9618C390BEC601BE2CC1206D0C07@qq.com/
> Fixes: 2a31c54be097 ("RISC-V: Minimal parser for "riscv, isa" strings")
> Signed-off-by: Conor Dooley <conor.dooley at microchip.com>
> ---
>  arch/riscv/kernel/cpufeature.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 52585e088873..93850540b0b4 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -142,6 +142,10 @@ void __init riscv_fill_hwcap(void)
>  			const char *ext_end = isa;
>  			bool ext_long = false, ext_err = false;
>  
> +			if (unlikely(!islower(*ext))) {
> +				ext_err = true;
> +			}

Can drop the {}

> +
>  			switch (*ext) {
>  			case 's':
>  				/**
> @@ -156,6 +160,15 @@ void __init riscv_fill_hwcap(void)
>  					break;
>  				}
>  				fallthrough;
> +			case 'S':
> +			case 'X':
> +			case 'Z':
> +				/*
> +				 * As the riscv,isa string must be lower-case,
> +				 * S, X and Z are not valid characters. Parse
> +				 * the invalid extension anyway, to skip ahead
> +				 * to the next valid one.
> +				 */
>  			case 'x':
>  			case 'z':
>  				ext_long = true;
> @@ -185,10 +198,8 @@ void __init riscv_fill_hwcap(void)
>  				++ext_end;
>  				break;
>  			default:
> -				if (unlikely(!islower(*ext))) {
> -					ext_err = true;
> +				if (unlikely(ext_err))
>  					break;
> -				}
>  				/* Find next extension */
>  				if (!isdigit(*isa))
>  					break;
> -- 
> 2.39.2
>

Thanks,
drew