On Thu, 16 Jan 2020 16:34:06 +0100 Philippe Mathieu-Daudé <[email protected]> wrote:
> Hi Greg, > Hi Phil, > On 1/16/20 4:05 PM, Greg Kurz wrote: > > Most of the option vector helpers have assertions to check their > > arguments aren't null. The guest can provide an arbitrary address > > for the CAS structure that would result in such null arguments. > > Fail CAS with H_PARAMETER instead of aborting QEMU. > > > > Signed-off-by: Greg Kurz <[email protected]> > > --- > > hw/ppc/spapr_hcall.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > > index 84e1612595bb..051869ae20ec 100644 > > --- a/hw/ppc/spapr_hcall.c > > +++ b/hw/ppc/spapr_hcall.c > > @@ -1701,9 +1701,18 @@ static target_ulong > > h_client_architecture_support(PowerPCCPU *cpu, > > > > /* For the future use: here @ov_table points to the first option > > vector */ > > ov_table = addr; > > + if (!ov_table) { > > + return H_PARAMETER; > > + } > > This doesn't look right to check ov_table, I'd check addr directly instead: > I decided to check ov_table because this is what we pass to spapr_ovec_parse_vector() and that shouldn't be NULL. > -- >8 -- > @@ -1679,12 +1679,16 @@ static target_ulong > h_client_architecture_support(PowerPCCPU *cpu, > > cas_pvr = cas_check_pvr(spapr, cpu, &addr, &raw_mode_supported, > &local_err); > if (local_err) { > error_report_err(local_err); > return H_HARDWARE; > } > + if (!addr) { > + // error_report*() > + return H_PARAMETER; > + } > I don't really care one way or another, but adding an error_report() is a good idea since linux just print out the following in case of CAS failure: WARNING: ibm,client-architecture-support call FAILED! > /* Update CPUs */ > if (cpu->compat_pvr != cas_pvr) { > --- > > Still I'm not sure it makes sense, because the guest can also set other > invalid addresses such addr=0x69. > The point of this patch is just to avoid hitting the assertions. 0x69 is probably bullshit but it passes the g_assert() at least. > > > > ov1_guest = spapr_ovec_parse_vector(ov_table, 1); > > + if (!ov1_guest) { > > + return H_PARAMETER; > > + } > > This one is OK (unlikely case where vector 1 isn't present). > > > ov5_guest = spapr_ovec_parse_vector(ov_table, 5); > > + if (!ov5_guest) { > > + return H_PARAMETER; > > + } > > This one is OK too (unlikely case where vector 5 isn't present). > > > if (spapr_ovec_test(ov5_guest, OV5_MMU_BOTH)) { > > error_report("guest requested hash and radix MMU, which is > > invalid."); > > exit(EXIT_FAILURE); > > > > >
