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@xxxxxxxxxxxx> > Link: https://lore.kernel.org/all/tencent_1647475C9618C390BEC601BE2CC1206D0C07@xxxxxx/ > Fixes: 2a31c54be097 ("RISC-V: Minimal parser for "riscv, isa" strings") > Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> > --- > 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
