* [PATCH] libxl: Don't segfault on soft-reset failure
@ 2022-03-30 18:17 Jason Andryuk
2022-03-31 14:10 ` Anthony PERARD
0 siblings, 1 reply; 3+ messages in thread
From: Jason Andryuk @ 2022-03-30 18:17 UTC (permalink / raw)
To: xen-devel; +Cc: Jason Andryuk, Wei Liu, Anthony PERARD, Juergen Gross
If domain_soft_reset_cb can't rename the save file, it doesn't call
initiate_domain_create() and calls domcreate_complete().
Skipping initiate_domain_create() means dcs->console_wait is
uninitialized and all 0s.
We have:
domcreate_complete()
libxl__xswait_stop()
libxl__ev_xswatch_deregister().
The uninitialized slotnum 0 is considered valid (-1 is the invalid
sentinel), so the NULL pointer path to passed to xs_unwatch() which
segfaults.
libxl__ev_xswatch_deregister:watch w=0x12bc250 wpath=(null) token=0/0: deregister slotnum=0
Ensure dcs->console_xswait is minimally initialized by calling
libxl__xswait_init() in do_domain_soft_reset().
Also add a check for dcs->console_xswait.path being NULL in
domcreate_complete() to avoid the segfault.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
The NULL check in domcreate_complete isn't needed when the xswait is
initialized, but it could catch other occurances.
tools/libs/light/libxl_create.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 15ed021f41..631caa416d 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -1970,7 +1970,8 @@ static void domcreate_complete(libxl__egc *egc,
libxl_domain_config *const d_config = dcs->guest_config;
libxl_domain_config *d_config_saved = &dcs->guest_config_saved;
- libxl__xswait_stop(gc, &dcs->console_xswait);
+ if (dcs->console_xswait.path)
+ libxl__xswait_stop(gc, &dcs->console_xswait);
libxl__domain_build_state_dispose(&dcs->build_state);
@@ -2176,6 +2177,10 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
aop_console_how);
cdcs->domid_out = &domid_out;
+ /* Initialize in case we end up in domcreate_complete without calling
+ * initiate_domain_create. */
+ libxl__xswait_init(&cdcs->dcs.console_xswait);
+
dom_path = libxl__xs_get_dompath(gc, domid);
if (!dom_path) {
LOGD(ERROR, domid, "failed to read domain path");
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] libxl: Don't segfault on soft-reset failure
2022-03-30 18:17 [PATCH] libxl: Don't segfault on soft-reset failure Jason Andryuk
@ 2022-03-31 14:10 ` Anthony PERARD
2022-03-31 16:35 ` Jason Andryuk
0 siblings, 1 reply; 3+ messages in thread
From: Anthony PERARD @ 2022-03-31 14:10 UTC (permalink / raw)
To: Jason Andryuk; +Cc: xen-devel, Wei Liu, Juergen Gross
On Wed, Mar 30, 2022 at 02:17:27PM -0400, Jason Andryuk wrote:
> diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
> index 15ed021f41..631caa416d 100644
> --- a/tools/libs/light/libxl_create.c
> +++ b/tools/libs/light/libxl_create.c
> @@ -1970,7 +1970,8 @@ static void domcreate_complete(libxl__egc *egc,
> libxl_domain_config *const d_config = dcs->guest_config;
> libxl_domain_config *d_config_saved = &dcs->guest_config_saved;
>
> - libxl__xswait_stop(gc, &dcs->console_xswait);
> + if (dcs->console_xswait.path)
> + libxl__xswait_stop(gc, &dcs->console_xswait);
libxl__xswait_stop() needs to be called here. It's a function that can
be called several time without ill effect, as long as `console_xswait`
is initialised properly (by calling libxl__xswait_init() like you did
below.
> libxl__domain_build_state_dispose(&dcs->build_state);
>
> @@ -2176,6 +2177,10 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
> aop_console_how);
> cdcs->domid_out = &domid_out;
>
> + /* Initialize in case we end up in domcreate_complete without calling
> + * initiate_domain_create. */
That comment isn't needed. This is just part of the initialisation of
`cdcs->dcs`.
> + libxl__xswait_init(&cdcs->dcs.console_xswait);
> +
> dom_path = libxl__xs_get_dompath(gc, domid);
> if (!dom_path) {
> LOGD(ERROR, domid, "failed to read domain path");
As part of the patch, could you also move the libxl__xswait_init() call
in initiate_domain_create() to do_domain_create()? That would avoid
`console_xswait` been initialised twice, and do_domain_create() is the
function that initialise `dcs` before calling initiate_domain_create().
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] libxl: Don't segfault on soft-reset failure
2022-03-31 14:10 ` Anthony PERARD
@ 2022-03-31 16:35 ` Jason Andryuk
0 siblings, 0 replies; 3+ messages in thread
From: Jason Andryuk @ 2022-03-31 16:35 UTC (permalink / raw)
To: Anthony PERARD; +Cc: xen-devel, Wei Liu, Juergen Gross
On Thu, Mar 31, 2022 at 10:10 AM Anthony PERARD
<anthony.perard@citrix.com> wrote:
>
> On Wed, Mar 30, 2022 at 02:17:27PM -0400, Jason Andryuk wrote:
> > diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
> > index 15ed021f41..631caa416d 100644
> > --- a/tools/libs/light/libxl_create.c
> > +++ b/tools/libs/light/libxl_create.c
> > @@ -1970,7 +1970,8 @@ static void domcreate_complete(libxl__egc *egc,
> > libxl_domain_config *const d_config = dcs->guest_config;
> > libxl_domain_config *d_config_saved = &dcs->guest_config_saved;
> >
> > - libxl__xswait_stop(gc, &dcs->console_xswait);
> > + if (dcs->console_xswait.path)
> > + libxl__xswait_stop(gc, &dcs->console_xswait);
>
> libxl__xswait_stop() needs to be called here. It's a function that can
> be called several time without ill effect, as long as `console_xswait`
> is initialised properly (by calling libxl__xswait_init() like you did
> below.
>
> > libxl__domain_build_state_dispose(&dcs->build_state);
> >
> > @@ -2176,6 +2177,10 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
> > aop_console_how);
> > cdcs->domid_out = &domid_out;
> >
> > + /* Initialize in case we end up in domcreate_complete without calling
> > + * initiate_domain_create. */
>
> That comment isn't needed. This is just part of the initialisation of
> `cdcs->dcs`.
>
> > + libxl__xswait_init(&cdcs->dcs.console_xswait);
> > +
> > dom_path = libxl__xs_get_dompath(gc, domid);
> > if (!dom_path) {
> > LOGD(ERROR, domid, "failed to read domain path");
>
>
> As part of the patch, could you also move the libxl__xswait_init() call
> in initiate_domain_create() to do_domain_create()? That would avoid
> `console_xswait` been initialised twice, and do_domain_create() is the
> function that initialise `dcs` before calling initiate_domain_create().
Sure, that all sounds good. Thanks!
Regards,
Jason
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-03-31 16:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-30 18:17 [PATCH] libxl: Don't segfault on soft-reset failure Jason Andryuk
2022-03-31 14:10 ` Anthony PERARD
2022-03-31 16:35 ` Jason Andryuk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.