Re: buffer overprint in riscv64/cpu.c
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 - 1.14
> > +++ cpu.c 1 Aug 2023 11:35:28 -
> > @@ -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 - 1.14
> +++ cpu.c 4 Aug 2023 11:15:16 -
> @@ -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;
> + }
> + /*
> +
Re: buffer overprint in riscv64/cpu.c
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.
>
> 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 - 1.14
> +++ cpu.c 1 Aug 2023 11:35:28 -
> @@ -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 - 1.14
+++ cpu.c 4 Aug 2023 11:15:16 -
@@ -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';
+
buffer overprint in riscv64/cpu.c
>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.
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 - 1.14
+++ cpu.c 1 Aug 2023 11:35:28 -
@@ -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;
dmesg:
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
real mem = 2147483648 (2048MB)
avail mem = 2027982848 (1934MB)
SBI: OpenSBI v1.2, SBI Specification Version 1.0
random: good seed from bootblocks
mainbus0 at root: riscv-virtio,qemu
cpu0 at mainbus0: vendor 0 arch 70200 imp 70200
rv64imafdch_zicsr_zifencei_zihin\M-n\M-#7
intc0 at cpu0
cpu1 at mainbus0: vendor 0 arch 70200 imp 70200
rv64imafdch_zicsr_zifencei_zihin\M-n\M-#7
syscon0 at mainbus0: "poweroff"
syscon1 at mainbus0: "reboot"
"fw-cfg" at mainbus0 not configured
"flash" at mainbus0 not configured
simplebus0 at mainbus0: "platform-bus"
simplebus1 at mainbus0: "soc"
syscon2 at simplebus1: "test"
plic0 at simplebus1
"pmu" at simplebus1 not configured
gfrtc0 at simplebus1
com0 at simplebus1: ns16550, no working fifo
com0: console
pciecam0 at simplebus1
pci0 at pciecam0
"Red Hat Host" rev 0x00 at pci0 dev 0 function 0 not configured
virtio0 at simplebus1: Virtio Network Device
vio0 at virtio0: address 52:54:00:12:34:56
virtio1 at simplebus1: Virtio Block Device
vioblk0 at virtio1
scsibus0 at vioblk0: 1 targets
sd0 at scsibus0 targ 0 lun 0:
sd0: 40960MB, 512 bytes/sector, 83886080 sectors
virtio2 at simplebus1: Virtio Unknown (0) Device
virtio3 at simplebus1: Virtio Unknown (0) Device
virtio4 at simplebus1: Virtio Unknown (0) Device
virtio5 at simplebus1: Virtio Unknown (0) Device
virtio6 at simplebus1: Virtio Unknown (0) Device
virtio7 at simplebus1: Virtio Unknown (0) Device
"clint" at simplebus1 not configured
vscsi0 at root
scsibus1 at vscsi0: 256 targets
softraid0 at root
scsibus2 at softraid0: 256 targets
root on sd0a (a44107957d8bed73.a) swap on sd0b dump on sd0b
usbdevs:
usbdevs: no USB controllers found
pcidump:
everything past here is not provided.
