On Fri, Aug 04, 2023 at 01:26:08PM +0200, Peter J. Philipp wrote: > On Tue, Aug 01, 2023 at 01:43:36PM +0200, p...@delphinusdns.org wrote: > > >Synopsis: non-terminated strings buffer in riscv64/cpu.c > > >Category: kernel > > >Environment: > > System : OpenBSD 7.3 > > Details : OpenBSD 7.3-current (GENERIC.MP) #376: Thu Jul 13 > > 03:59:40 MDT 2023 > > > > dera...@riscv64.openbsd.org:/usr/src/sys/arch/riscv64/compile/GENERIC.MP > > > > Architecture: OpenBSD.riscv64 > > Machine : riscv64 > > >Description: > > The cpu detect output is not NUL terminated, this causes *puke* to be > > displayed on serial terminals. > > >How-To-Repeat: > > Using Qemu for riscv64 arch. > > > > from a eeprom -p | grep isa output: > > > > riscv,isa: > > 'rv64imafdch_zicsr_zifencei_zihintpause_zba_zbb_zbc_zbs_sstc' > > riscv,isa: > > 'rv64imafdch_zicsr_zifencei_zihintpause_zba_zbb_zbc_zbs_sstc' > > > > I counted this as 60 bytes long. > > >Fix: > > > > There is two approaches. One is to explicitly NUL terminate the 32 byte > > buffer or make it bigger. I give an untested patch of the latter.
riscv,isa is going away: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=aeb71e42caae2031ec849a858080d81462cacca9 I see no point in trying to parse it. > > > > Index: cpu.c > > =================================================================== > > RCS file: /cvs/src/sys/arch/riscv64/riscv64/cpu.c,v > > retrieving revision 1.14 > > diff -u -p -u -r1.14 cpu.c > > --- cpu.c 15 Jun 2023 22:18:08 -0000 1.14 > > +++ cpu.c 1 Aug 2023 11:35:28 -0000 > > @@ -87,7 +87,7 @@ int cpu_errata_sifive_cip_1200; > > void > > cpu_identify(struct cpu_info *ci) > > { > > - char isa[32]; > > + char isa[64]; > > uint64_t marchid, mimpid; > > uint32_t mvendorid; > > const char *vendor_name = NULL; > > > > > > [tying in tech@] > > This wasn't effective I just saw. On another QEMU host the cpu ISA string is > larger than 80 characters. So I've made another patch. > > With this patch it looks like so: > > oceans$ dmesg|grep cpu > cpu0 at mainbus0: vendor 0 arch 0 imp 0 rv64imafdch Zicbom Zicboz Zicsr > Zifencei Zihintpause Zawrs Zfa Zca Zcd Zba Zbb Zbc Zbs Sstc Svadu > intc0 at cpu0 > cpu1 at mainbus0: vendor 0 arch 0 imp 0 rv64imafdch Zicbom Zicboz Zicsr > Zifencei Zihintpause Zawrs Zfa Zca Zcd Zba Zbb Zbc Zbs Sstc Svadu > oceans# grep zicboz /root/eeprom-p.out > > riscv,isa: > 'rv64imafdch_zicbom_zicboz_zicsr_zifencei_zihintpause_zawrs_zfa_zca_zcd_zba_zbb_zbc_zbs_sstc_svadu' > riscv,isa: > 'rv64imafdch_zicbom_zicboz_zicsr_zifencei_zihintpause_zawrs_zfa_zca_zcd_zba_zbb_zbc_zbs_sstc_svadu' > > This is in convention with the cpu.c found in qemu: > > https://gitlab.com/qemu-project/qemu/-/blob/master/target/riscv/cpu.c > > lines 64 through 84 is the description of it. > > While I have no OpenBSD/riscv64 on true hardware it works on QEMU, and I > googled for a dmesg online and the Hifive Unmatched should work as well. > > patch follows: > > Index: cpu.c > =================================================================== > RCS file: /cvs/src/sys/arch/riscv64/riscv64/cpu.c,v > retrieving revision 1.14 > diff -u -p -u -r1.14 cpu.c > --- cpu.c 15 Jun 2023 22:18:08 -0000 1.14 > +++ cpu.c 4 Aug 2023 11:15:16 -0000 > @@ -84,15 +84,19 @@ struct cfdriver cpu_cd = { > > int cpu_errata_sifive_cip_1200; > > + > void > cpu_identify(struct cpu_info *ci) > { > - char isa[32]; > + char isa[255]; > + char szx_ext[255]; /* S, Z and X extension buffer */ > + char *extensions = "imafdqlcbkjtpvh"; > uint64_t marchid, mimpid; > uint32_t mvendorid; > const char *vendor_name = NULL; > const char *arch_name = NULL; > struct arch *archlist = cpu_arch_none; > + char *p, *pe, *end; > int i, len; > > mvendorid = sbi_get_mvendorid(); > @@ -126,8 +130,70 @@ cpu_identify(struct cpu_info *ci) > > len = OF_getprop(ci->ci_node, "riscv,isa", isa, sizeof(isa)); > if (len != -1) { > + /* terminate it, it could be non-terminated */ > + isa[sizeof(isa) - 1] = '\0'; > + > + /* PARSE the IMAFDQ... extensions */ > + pe = extensions; > + if ((p = strchr(isa, 'i')) != NULL || > + (p = strchr(isa, 'I')) != NULL) { > + for (; *pe != '\0'; pe++) { > + if (((*p)|0x20) == *pe) { > + if (p[1]) { > + p++; > + i++; > + } else > + break; > + } > + /* > + * we've hit an underscore what follows > + * may be an S or Z extension > + */ > + if (*p == '_') > + break; > + } > + > + szx_ext[0] = '\0'; > + if (*p == '_') { > + *p++ = '\0'; > + for (; *p ;) { > + end = strchr(p, '_'); > + if (end != NULL) > + *end++ = '\0'; > + > + switch (*p) { > + case 'Z': > + case 'z': > + strlcat(szx_ext, "Z", > sizeof(szx_ext)); > + break; > + case 'S': > + case 's': > + strlcat(szx_ext, "S", > sizeof(szx_ext)); > + break; > + case 'X': > + case 'x': > + default: > + strlcat(szx_ext, "?", > sizeof(szx_ext)); > + break; > + } > + p++; > + i++; > + strlcat(szx_ext, p, sizeof(szx_ext)); > + if (end) { > + strlcat(szx_ext, " ", > sizeof(szx_ext)); > + p = end; > + } else > + break; > + } > + } else > + *p = '\0'; > + } > + > printf(" %s", isa); > strlcpy(cpu_model, isa, sizeof(cpu_model)); > + > + printf(" %s", szx_ext); > + > } > printf("\n"); > > > Best Regards, > -peter > >
