[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 3/3] xsm: properly handle error from XSM init
On 6/1/22 02:14, Jan Beulich wrote: > On 31.05.2022 17:08, Daniel P. Smith wrote: >> @@ -1690,7 +1691,7 @@ void __init noreturn __start_xen(unsigned long mbi_p) >> >> open_softirq(NEW_TLBFLUSH_CLOCK_PERIOD_SOFTIRQ, >> new_tlbflush_clock_period); >> >> - if ( opt_watchdog ) >> + if ( opt_watchdog ) >> nmi_watchdog = NMI_LOCAL_APIC; >> >> find_smp_config(); > > Please omit formatting changes to entirely unrelated pieces of code. Ack. this was in simliar vein of the other patches where I cleaned nearby trailing space. >> @@ -1700,7 +1701,11 @@ void __init noreturn __start_xen(unsigned long mbi_p) >> mmio_ro_ranges = rangeset_new(NULL, "r/o mmio ranges", >> RANGESETF_prettyprint_hex); >> >> - xsm_multiboot_init(module_map, mbi); >> + if ( xsm_multiboot_init(module_map, mbi) ) >> + warning_add("WARNING: XSM failed to initialize.\n" >> + "This has implications on the security of the system,\n" >> + "as uncontrolled communications between trusted and\n" >> + "untrusted domains may occur.\n"); > > Uncontrolled communication isn't the only thing that could occur, aiui. > So at the very least "e.g." or some such would want adding imo. Agreed, this was a reuse of the existing message and honestly I would like to believe this was the original author's attempt at brevity versus writing a detailed message of every implication to the security of the system. > Now that return values are checked, I think that in addition to what > you already do the two function declarations may want decorating with > __must_check. Understood but likely not necessary based on Andy's review suggestion to move these functions to void. v/r, dps
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |