target/riscv/kvm: Handle SBI console backend failures
PATCH 1/1 target/riscv/kvm: Handle SBI console backend failures
The serial backend is optional, for example when QEMU is started with -serial none. The legacy SBI console and DBCN handlers nevertheless dereference serial_hd(0)->fe without checking whether the chardev or its frontend exists, causing QEMU to crash on a guest console call. DBCN also terminates QEMU when an existing backend reports an I/O error.
Resolve the frontend once per SBI exit and check it before use. Discard legacy output and return -1 for legacy input when no console is available. Explicitly return zero from legacy console putchar as required by SBI.
For DBCN, return SBI_ERR_DENIED with a zero value when the frontend is missing, before allocating a buffer or accessing guest memory. Convert backend I/O errors to SBI_ERR_FAILED instead of exiting QEMU, and do not copy a failed read into guest memory. Treat an incomplete single-byte write as a failure too, since that call must write the specified byte.
Preserve successful zero-length reads and writes, partial bulk transfers, and the unsupported-function response.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4430
--- target/riscv/kvm/kvm-cpu.c | 53 ++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 17 deletions(-)
target/riscv/kvm/kvm-cpu.c +36 -17
@@ -1686,7 +1686,8 @@ bool kvm_arch_stop_on_emulation_error(CPUState *cs)return true; } -static void kvm_riscv_handle_sbi_dbcn (CPUState *cs,struct kvm_run *run) +static void kvm_riscv_handle_sbi_dbcn (CPUState *cs,struct kvm_run *run, +CharFrontend *fe) {const MemTxAttrs attrs =MEMTXATTRS_UNSPECIFIED ; g_autofree uint8_t *buf = NULL;
@@ -1707,6 +1708,12 @@ static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run)break ; } +if (!fe) { + run->riscv_sbi.ret[0 ] =SBI_ERR_DENIED ; + run->riscv_sbi.ret[1 ] =0 ; +break ; + } + addr = run->riscv_sbi.args[1 ];/*
@@ -1720,22 +1727,22 @@ static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run) buf =g_malloc0 (num_bytes);if (run->riscv_sbi.function_id ==SBI_EXT_DBCN_CONSOLE_READ ) { - ret =qemu_chr_fe_read_all (serial_hd (0 )->fe, buf, num_bytes); + ret =qemu_chr_fe_read_all (fe, buf, num_bytes);if (ret <0 ) { -error_report ("SBI_EXT_DBCN_CONSOLE_READ: error when " -"reading chardev" ); -exit (1 ); + run->riscv_sbi.ret[0 ] =SBI_ERR_FAILED ; + run->riscv_sbi.ret[1 ] =0 ; +break ; }address_space_write (cs->as, addr, attrs, buf, ret); }else {address_space_read (cs->as, addr, attrs, buf, num_bytes); - ret =qemu_chr_fe_write_all (serial_hd (0 )->fe, buf, num_bytes); + ret =qemu_chr_fe_write_all (fe, buf, num_bytes);if (ret <0 ) { -error_report ("SBI_EXT_DBCN_CONSOLE_WRITE: error when " -"writing chardev" ); -exit (1 ); + run->riscv_sbi.ret[0 ] =SBI_ERR_FAILED ; + run->riscv_sbi.ret[1 ] =0 ; +break ; } }
@@ -1743,13 +1750,19 @@ static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run) run->riscv_sbi.ret[1 ] = ret;break ;case SBI_EXT_DBCN_CONSOLE_WRITE_BYTE : +if (!fe) { + run->riscv_sbi.ret[0 ] =SBI_ERR_DENIED ; + run->riscv_sbi.ret[1 ] =0 ; +break ; + } + ch = run->riscv_sbi.args[0 ]; - ret =qemu_chr_fe_write_all (serial_hd (0 )->fe, &ch,sizeof (ch)); + ret =qemu_chr_fe_write_all (fe, &ch,sizeof (ch)); -if (ret <0 ) { -error_report ("SBI_EXT_DBCN_CONSOLE_WRITE_BYTE: error when " -"writing chardev" ); -exit (1 ); +if (ret !=sizeof (ch)) { + run->riscv_sbi.ret[0 ] =SBI_ERR_FAILED ; + run->riscv_sbi.ret[1 ] =0 ; +break ; } run->riscv_sbi.ret[0 ] =SBI_SUCCESS ;
@@ -1762,15 +1775,21 @@ static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run)static int kvm_riscv_handle_sbi (CPUState *cs,struct kvm_run *run) { +Chardev *chr =serial_hd (0 ); +CharFrontend *fe = chr ? chr->fe :NULL ;int ret =0 ;unsigned char ch; +switch (run->riscv_sbi.extension_id) {case SBI_EXT_0_1_CONSOLE_PUTCHAR : ch = run->riscv_sbi.args[0 ]; -qemu_chr_fe_write (serial_hd (0 )->fe, &ch,sizeof (ch)); +if (fe) { +qemu_chr_fe_write (fe, &ch,sizeof (ch)); + } + run->riscv_sbi.ret[0 ] =0 ;break ;case SBI_EXT_0_1_CONSOLE_GETCHAR : - ret =qemu_chr_fe_read_all (serial_hd (0 )->fe, &ch,sizeof (ch)); + ret = fe ?qemu_chr_fe_read_all (fe, &ch,sizeof (ch)) :0 ;if (ret ==sizeof (ch)) { run->riscv_sbi.ret[0 ] = ch; }else {
@@ -1779,7 +1798,7 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct kvm_run *run) ret =0 ;break ;case SBI_EXT_DBCN : -kvm_riscv_handle_sbi_dbcn (cs, run); +kvm_riscv_handle_sbi_dbcn (cs, run, fe);break ;default : qemu_log_mask(LOG_UNIMP,
Signature · 1 line
2.43.0
Quoted 28 lines · Zephyr Li
The serial backend is optional, for example when QEMU is started with -serial none. The legacy SBI console and DBCN handlers nevertheless dereference serial_hd(0)->fe without checking whether the chardev or its frontend exists, causing QEMU to crash on a guest console call. DBCN also terminates QEMU when an existing backend reports an I/O error.
Resolve the frontend once per SBI exit and check it before use. Discard
legacy output and return -1 for legacy input when no console is available.
Explicitly return zero from legacy console putchar as required by SBI.For DBCN, return
SBI_ERR_DENIEDwith a zero value when the frontend is
missing, before allocating a buffer or accessing guest memory. Convert
backend I/O errors toSBI_ERR_FAILEDinstead of exiting QEMU, and do not
copy a failed read into guest memory. Treat an incomplete single-byte write as a failure too, since that call must write the specified byte.Preserve successful zero-length reads and writes, partial bulk transfers,
and the unsupported-function response.
Quoted 5 lines
Fixes 4eb471258bd0 ("target/riscv: Handle KVM_EXIT_RISCV_SBI exit")Fixes a6b53378f537 ("target/riscv/kvm: implement SBI debug console(DBCN) calls")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4430Signed-off-by Zephyr Li <[email protected]>
Alistair
Quoted 137 lines
---
target/riscv/kvm/kvm-cpu.c | 53 ++++++++++++++++++++++++++---------- --1 file changed, 36 insertions(+), 17 deletions(-)target/riscv/kvm/kvm-cpu.c +0
-0@@ -1686,7 +1686,8 @@ bool kvm_arch_stop_on_emulation_error(CPUState *cs) return true; }
-static voidkvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run)
+static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, structkvm_run*run,
+ CharFrontend *fe) {
const MemTxAttrs attrs =MEMTXATTRS_UNSPECIFIED;g_autofreeuint8_t*buf = NULL;target/riscv/kvm/kvm-cpu.c +0
-0@@ -1707,6 +1708,12 @@ static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run) break; }
+ if (!fe) {
+ run->riscv_sbi.ret[0] =SBI_ERR_DENIED; + run->riscv_sbi.ret[1] = 0;
+ break;
+ }
+
addr = run->riscv_sbi.args[1];
/*target/riscv/kvm/kvm-cpu.c +0
-0@@ -1720,22 +1727,22 @@ static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run) buf = g_malloc0(num_bytes);
if (run->riscv_sbi.function_id==SBI_EXT_DBCN_CONSOLE_READ) {
- ret =qemu_chr_fe_read_all(serial_hd(0)->fe, buf, num_bytes);
+ ret =qemu_chr_fe_read_all(fe, buf, num_bytes); if (ret < 0) {
- error_report("SBI_EXT_DBCN_CONSOLE_READ: error when "
- "reading chardev");
- exit(1);
+ run->riscv_sbi.ret[0] =SBI_ERR_FAILED;
+ run->riscv_sbi.ret[1] = 0;
+ break;
}
address_space_write(cs->as, addr, attrs, buf, ret); } else {
address_space_read(cs->as, addr, attrs, buf, num_bytes);
- ret =qemu_chr_fe_write_all(serial_hd(0)->fe, buf, num_bytes);
+ ret =qemu_chr_fe_write_all(fe, buf, num_bytes); if (ret < 0) {
- error_report("SBI_EXT_DBCN_CONSOLE_WRITE: error when "
- "writing chardev");
- exit(1);
+ run->riscv_sbi.ret[0] =SBI_ERR_FAILED;
+ run->riscv_sbi.ret[1] = 0;
+ break;
}
}target/riscv/kvm/kvm-cpu.c +0
-0@@ -1743,13 +1750,19 @@ static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run) run->riscv_sbi.ret[1] = ret; break;case
SBI_EXT_DBCN_CONSOLE_WRITE_BYTE:
+ if (!fe) {
+ run->riscv_sbi.ret[0] =SBI_ERR_DENIED;
+ run->riscv_sbi.ret[1] = 0;
+ break;
+ }
+
ch = run->riscv_sbi.args[0];
- ret =qemu_chr_fe_write_all(serial_hd(0)->fe, &ch, sizeof(ch));
+ ret =qemu_chr_fe_write_all(fe, &ch, sizeof(ch));
- if (ret < 0) {
- error_report("SBI_EXT_DBCN_CONSOLE_WRITE_BYTE: error when "
- "writing chardev");
- exit(1);
+ if (ret != sizeof(ch)) {
+ run->riscv_sbi.ret[0] =SBI_ERR_FAILED;
+ run->riscv_sbi.ret[1] = 0;
+ break;
}
run->riscv_sbi.ret[0] =SBI_SUCCESS;target/riscv/kvm/kvm-cpu.c +0
-0@@ -1762,15 +1775,21 @@ static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run)
static intkvm_riscv_handle_sbi(CPUState *cs, struct kvm_run *run){
+ Chardev *chr =serial_hd(0);
+ CharFrontend *fe = chr ?chr->fe: NULL;
int ret = 0;
unsigned char ch;
+
switch (run->riscv_sbi.extension_id) {
caseSBI_EXT_0_1_CONSOLE_PUTCHAR:
ch = run->riscv_sbi.args[0];
-qemu_chr_fe_write(serial_hd(0)->fe, &ch, sizeof(ch)); + if (fe) {
+qemu_chr_fe_write(fe, &ch, sizeof(ch));
+ }
+ run->riscv_sbi.ret[0] = 0;
break;
caseSBI_EXT_0_1_CONSOLE_GETCHAR:
- ret =qemu_chr_fe_read_all(serial_hd(0)->fe, &ch, sizeof(ch));
+ ret = fe ?qemu_chr_fe_read_all(fe, &ch, sizeof(ch)): 0; if (ret == sizeof(ch)) {
run->riscv_sbi.ret[0] = ch;
} else {target/riscv/kvm/kvm-cpu.c +0
-0@@ -1779,7 +1798,7 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct kvm_run *run) ret = 0; break;case
SBI_EXT_DBCN:
-kvm_riscv_handle_sbi_dbcn(cs, run);
+kvm_riscv_handle_sbi_dbcn(cs, run, fe);
break;
default:
qemu_log_mask(LOG_UNIMP,
Alistair Francis
AppliedQuoted 28 lines · Zephyr Li
The serial backend is optional, for example when QEMU is started with -serial none. The legacy SBI console and DBCN handlers nevertheless dereference serial_hd(0)->fe without checking whether the chardev or its frontend exists, causing QEMU to crash on a guest console call. DBCN also terminates QEMU when an existing backend reports an I/O error.
Resolve the frontend once per SBI exit and check it before use. Discard
legacy output and return -1 for legacy input when no console is available.
Explicitly return zero from legacy console putchar as required by SBI.For DBCN, return
SBI_ERR_DENIEDwith a zero value when the frontend is
missing, before allocating a buffer or accessing guest memory. Convert
backend I/O errors toSBI_ERR_FAILEDinstead of exiting QEMU, and do not
copy a failed read into guest memory. Treat an incomplete single-byte write as a failure too, since that call must write the specified byte.Preserve successful zero-length reads and writes, partial bulk transfers,
and the unsupported-function response.
Quoted 5 lines
Fixes 4eb471258bd0 ("target/riscv: Handle KVM_EXIT_RISCV_SBI exit")Fixes a6b53378f537 ("target/riscv/kvm: implement SBI debug console(DBCN) calls")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4430Signed-off-by Zephyr Li <[email protected]>
Thanks!
Applied to riscv-to-apply.next
Alistair
Quoted 137 lines
---
target/riscv/kvm/kvm-cpu.c | 53 ++++++++++++++++++++++++++---------- --1 file changed, 36 insertions(+), 17 deletions(-)target/riscv/kvm/kvm-cpu.c +0
-0@@ -1686,7 +1686,8 @@ bool kvm_arch_stop_on_emulation_error(CPUState *cs) return true; }
-static voidkvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run)
+static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, structkvm_run*run,
+ CharFrontend *fe) {
const MemTxAttrs attrs =MEMTXATTRS_UNSPECIFIED;g_autofreeuint8_t*buf = NULL;target/riscv/kvm/kvm-cpu.c +0
-0@@ -1707,6 +1708,12 @@ static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run) break; }
+ if (!fe) {
+ run->riscv_sbi.ret[0] =SBI_ERR_DENIED; + run->riscv_sbi.ret[1] = 0;
+ break;
+ }
+
addr = run->riscv_sbi.args[1];
/*target/riscv/kvm/kvm-cpu.c +0
-0@@ -1720,22 +1727,22 @@ static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run) buf = g_malloc0(num_bytes);
if (run->riscv_sbi.function_id==SBI_EXT_DBCN_CONSOLE_READ) {
- ret =qemu_chr_fe_read_all(serial_hd(0)->fe, buf, num_bytes);
+ ret =qemu_chr_fe_read_all(fe, buf, num_bytes); if (ret < 0) {
- error_report("SBI_EXT_DBCN_CONSOLE_READ: error when "
- "reading chardev");
- exit(1);
+ run->riscv_sbi.ret[0] =SBI_ERR_FAILED;
+ run->riscv_sbi.ret[1] = 0;
+ break;
}
address_space_write(cs->as, addr, attrs, buf, ret); } else {
address_space_read(cs->as, addr, attrs, buf, num_bytes);
- ret =qemu_chr_fe_write_all(serial_hd(0)->fe, buf, num_bytes);
+ ret =qemu_chr_fe_write_all(fe, buf, num_bytes); if (ret < 0) {
- error_report("SBI_EXT_DBCN_CONSOLE_WRITE: error when "
- "writing chardev");
- exit(1);
+ run->riscv_sbi.ret[0] =SBI_ERR_FAILED;
+ run->riscv_sbi.ret[1] = 0;
+ break;
}
}target/riscv/kvm/kvm-cpu.c +0
-0@@ -1743,13 +1750,19 @@ static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run) run->riscv_sbi.ret[1] = ret; break;case
SBI_EXT_DBCN_CONSOLE_WRITE_BYTE:
+ if (!fe) {
+ run->riscv_sbi.ret[0] =SBI_ERR_DENIED;
+ run->riscv_sbi.ret[1] = 0;
+ break;
+ }
+
ch = run->riscv_sbi.args[0];
- ret =qemu_chr_fe_write_all(serial_hd(0)->fe, &ch, sizeof(ch));
+ ret =qemu_chr_fe_write_all(fe, &ch, sizeof(ch));
- if (ret < 0) {
- error_report("SBI_EXT_DBCN_CONSOLE_WRITE_BYTE: error when "
- "writing chardev");
- exit(1);
+ if (ret != sizeof(ch)) {
+ run->riscv_sbi.ret[0] =SBI_ERR_FAILED;
+ run->riscv_sbi.ret[1] = 0;
+ break;
}
run->riscv_sbi.ret[0] =SBI_SUCCESS;target/riscv/kvm/kvm-cpu.c +0
-0@@ -1762,15 +1775,21 @@ static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run)
static intkvm_riscv_handle_sbi(CPUState *cs, struct kvm_run *run){
+ Chardev *chr =serial_hd(0);
+ CharFrontend *fe = chr ?chr->fe: NULL;
int ret = 0;
unsigned char ch;
+
switch (run->riscv_sbi.extension_id) {
caseSBI_EXT_0_1_CONSOLE_PUTCHAR:
ch = run->riscv_sbi.args[0];
-qemu_chr_fe_write(serial_hd(0)->fe, &ch, sizeof(ch)); + if (fe) {
+qemu_chr_fe_write(fe, &ch, sizeof(ch));
+ }
+ run->riscv_sbi.ret[0] = 0;
break;
caseSBI_EXT_0_1_CONSOLE_GETCHAR:
- ret =qemu_chr_fe_read_all(serial_hd(0)->fe, &ch, sizeof(ch));
+ ret = fe ?qemu_chr_fe_read_all(fe, &ch, sizeof(ch)): 0; if (ret == sizeof(ch)) {
run->riscv_sbi.ret[0] = ch;
} else {target/riscv/kvm/kvm-cpu.c +0
-0@@ -1779,7 +1798,7 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct kvm_run *run) ret = 0; break;case
SBI_EXT_DBCN:
-kvm_riscv_handle_sbi_dbcn(cs, run);
+kvm_riscv_handle_sbi_dbcn(cs, run, fe);
break;
default:
qemu_log_mask(LOG_UNIMP,