On Tue, Aug 01, 2023 at 01:43:36PM +0200, [email protected] 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
>
> [email protected]:/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.
>
> 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
