> -----Original Message----- > From: Andrew Cooper <[email protected]> > Sent: 12 June 2020 10:54 > To: [email protected]; 'Xen-devel' <[email protected]> > Cc: 'Ian Jackson' <[email protected]>; 'Wei Liu' <[email protected]>; > 'Juergen Gross' <[email protected]> > Subject: Re: [PATCH for-4.14] tools: fix error path of xendevicemodel_open() > > On 12/06/2020 08:22, Paul Durrant wrote: > >> -----Original Message----- > >> From: Andrew Cooper <[email protected]> > >> Sent: 10 June 2020 12:40 > >> To: Xen-devel <[email protected]> > >> Cc: Andrew Cooper <[email protected]>; Ian Jackson > >> <[email protected]>; Wei Liu > >> <[email protected]>; Juergen Gross <[email protected]>; Paul Durrant > >> <[email protected]> > >> Subject: [PATCH for-4.14] tools: fix error path of xendevicemodel_open() > >> > >> c/s 6902cb00e03 "tools/libxendevicemodel: extract functions and add a > >> compat > >> layer" introduced calls to both xencall_open() and > >> osdep_xendevicemodel_open() > >> but failed to fix up the error path. > >> > >> c/s f68c7c618a3 "libs/devicemodel: free xencall handle in error path in > >> _open()" fixed up the xencall_open() aspect of the error path (missing the > >> osdep_xendevicemodel_open() aspect), but positioned the xencall_close() > >> incorrectly, creating the same pattern proved to be problematic by c/s > >> 30a72f02870 "tools: fix error path of xenhypfs_open()". > >> > >> Reposition xtl_logger_destroy(), and introduce the missing > >> osdep_xendevicemodel_close(). > >> > >> Fixes: 6902cb00e03 ("tools/libxendevicemodel: extract functions and add a > >> compat layer") > >> Fixes: f68c7c618a3 ("libs/devicemodel: free xencall handle in error path > >> in _open()") > >> Backport: 4.9+ > >> Signed-off-by: Andrew Cooper <[email protected]> > >> --- > >> CC: Ian Jackson <[email protected]> > >> CC: Wei Liu <[email protected]> > >> CC: Juergen Gross <[email protected]> > >> CC: Paul Durrant <[email protected]> > >> > >> RFC - this is still broken. > >> > > I'm slightly confused. Do you want this in 4.14 in this form or are you > > expecting to update it? > > In this form, it is an improvement over before. > > There is still the crash described below which needs some form of > figuring out and fixing. >
Ok, in which case consider it... Release-acked-by: Paul Durrant <[email protected]> > ~Andrew > > > > > Paul > > > >> Failure to create the logger will still hit the NULL deference, in all of > >> the > >> stable libs, not just devicemodel. > >> > >> Also, unless I'd triple checked the history, I was about to reintroduce the > >> deadlock from c/s 9976f3874d4, because it totally counterintuitive wrong to > >> expect setup and teardown in opposite orders. > >> --- > >> tools/libs/devicemodel/core.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/tools/libs/devicemodel/core.c b/tools/libs/devicemodel/core.c > >> index db501d9e80..4d4063956d 100644 > >> --- a/tools/libs/devicemodel/core.c > >> +++ b/tools/libs/devicemodel/core.c > >> @@ -67,9 +67,10 @@ xendevicemodel_handle > >> *xendevicemodel_open(xentoollog_logger *logger, > >> return dmod; > >> > >> err: > >> - xtl_logger_destroy(dmod->logger_tofree); > >> + osdep_xendevicemodel_close(dmod); > >> xentoolcore__deregister_active_handle(&dmod->tc_ah); > >> xencall_close(dmod->xcall); > >> + xtl_logger_destroy(dmod->logger_tofree); > >> free(dmod); > >> return NULL; > >> } > >> -- > >> 2.11.0 > >
