[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/2] xen/livepatch: Add a return value to load hooks
On Tue, Nov 05, 2019 at 07:43:16PM +0000, Andrew Cooper wrote: > One use of load hooks is to perform a safety check of the system in its > quiesced state. Any non-zero return value from a load hook will abort the > apply attempt. > Shouldn't you also update the documentation (design doc?) Thanks. > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > --- > CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> > CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx> > CC: Juergen Gross <jgross@xxxxxxxx> > > For several years, the following patch in the series has been shipped in every > XenServer livepatch, implemented as a void load hook which modifies its return > address to skip to the end of apply_payload(). This method is distinctly less > hacky. > --- > xen/common/livepatch.c | 30 +++++++++++++++++++----------- > xen/include/xen/livepatch_payload.h | 2 +- > xen/test/livepatch/xen_hello_world.c | 12 +++++++++--- > 3 files changed, 29 insertions(+), 15 deletions(-) > > diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c > index 7caa30c202..962647616a 100644 > --- a/xen/common/livepatch.c > +++ b/xen/common/livepatch.c > @@ -1076,25 +1076,33 @@ static int apply_payload(struct payload *data) > * temporarily disable the spin locks IRQ state checks. > */ > spin_debug_disable(); > - for ( i = 0; i < data->n_load_funcs; i++ ) > - data->load_funcs[i](); > + for ( i = 0; !rc && i < data->n_load_funcs; i++ ) > + rc = data->load_funcs[i](); > spin_debug_enable(); > > + if ( rc ) > + printk(XENLOG_ERR LIVEPATCH "%s: load_funcs[%u] failed: %d\n", > + data->name, i, rc); > + > ASSERT(!local_irq_is_enabled()); > > - for ( i = 0; i < data->nfuncs; i++ ) > - arch_livepatch_apply(&data->funcs[i]); > + if ( !rc ) > + for ( i = 0; i < data->nfuncs; i++ ) > + arch_livepatch_apply(&data->funcs[i]); > > arch_livepatch_revive(); > > - /* > - * We need RCU variant (which has barriers) in case we crash here. > - * The applied_list is iterated by the trap code. > - */ > - list_add_tail_rcu(&data->applied_list, &applied_list); > - register_virtual_region(&data->region); > + if ( !rc ) > + { > + /* > + * We need RCU variant (which has barriers) in case we crash here. > + * The applied_list is iterated by the trap code. > + */ > + list_add_tail_rcu(&data->applied_list, &applied_list); > + register_virtual_region(&data->region); > + } > > - return 0; > + return rc; > } > > static int revert_payload(struct payload *data) > diff --git a/xen/include/xen/livepatch_payload.h > b/xen/include/xen/livepatch_payload.h > index 4a1a96d054..3788b52ddf 100644 > --- a/xen/include/xen/livepatch_payload.h > +++ b/xen/include/xen/livepatch_payload.h > @@ -9,7 +9,7 @@ > * The following definitions are to be used in patches. They are taken > * from kpatch. > */ > -typedef void livepatch_loadcall_t(void); > +typedef int livepatch_loadcall_t(void); > typedef void livepatch_unloadcall_t(void); > > /* > diff --git a/xen/test/livepatch/xen_hello_world.c > b/xen/test/livepatch/xen_hello_world.c > index 02f3f85dc0..d720001f07 100644 > --- a/xen/test/livepatch/xen_hello_world.c > +++ b/xen/test/livepatch/xen_hello_world.c > @@ -16,9 +16,11 @@ static const char hello_world_patch_this_fnc[] = > "xen_extra_version"; > extern const char *xen_hello_world(void); > static unsigned int cnt; > > -static void apply_hook(void) > +static int apply_hook(void) > { > printk(KERN_DEBUG "Hook executing.\n"); > + > + return 0; > } > > static void revert_hook(void) > @@ -26,10 +28,14 @@ static void revert_hook(void) > printk(KERN_DEBUG "Hook unloaded.\n"); > } > > -static void hi_func(void) > +static int hi_func(void) > { > printk(KERN_DEBUG "%s: Hi! (called %u times)\n", __func__, ++cnt); > + > + return 0; > }; > +/* Safe to cast away the return value for this trivial case. */ > +void (void_hi_func)(void) __attribute__((alias("hi_func"))); > > static void check_fnc(void) > { > @@ -43,7 +49,7 @@ LIVEPATCH_UNLOAD_HOOK(revert_hook); > /* Imbalance here. Two load and three unload. */ > > LIVEPATCH_LOAD_HOOK(hi_func); > -LIVEPATCH_UNLOAD_HOOK(hi_func); > +LIVEPATCH_UNLOAD_HOOK(void_hi_func); > > LIVEPATCH_UNLOAD_HOOK(check_fnc); > > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |