[PATCH] libxl: fix crash when initializing driver

Jim Fehlig posted 1 patch 4 years ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200403220153.22064-1-jfehlig@suse.com
src/libxl/libxl_driver.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH] libxl: fix crash when initializing driver
Posted by Jim Fehlig 4 years ago
Commit 54a401af478 split out DriverConfigInit from DriverConfigNew, but
then called it a bit late from libxlStateInitialize. The cfg is used in
libxlDriverConfigLoadFile and when uninitialized results in a crash.
Calling DriverConfigInit immediately after DriverConfigNew fixes the
crash.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_driver.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7ec4fcc3d1..980984b199 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -702,14 +702,14 @@ libxlStateInitialize(bool privileged,
     if (!(cfg = libxlDriverConfigNew()))
         goto error;
 
+    if (libxlDriverConfigInit(cfg) < 0)
+        goto error;
+
     driverConf = g_strdup_printf("%s/libxl.conf", cfg->configBaseDir);
 
     if (libxlDriverConfigLoadFile(cfg, driverConf) < 0)
         goto error;
 
-    if (libxlDriverConfigInit(cfg) < 0)
-        goto error;
-
     /* Register the callbacks providing access to libvirt's event loop */
     libxl_osevent_register_hooks(cfg->ctx, &libxl_osevent_callbacks, cfg->ctx);
 
-- 
2.26.0


Re: [PATCH] libxl: fix crash when initializing driver
Posted by Erik Skultety 4 years ago
On Fri, Apr 03, 2020 at 04:01:53PM -0600, Jim Fehlig wrote:
> Commit 54a401af478 split out DriverConfigInit from DriverConfigNew, but
> then called it a bit late from libxlStateInitialize. The cfg is used in
> libxlDriverConfigLoadFile and when uninitialized results in a crash.
> Calling DriverConfigInit immediately after DriverConfigNew fixes the
> crash.
>
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
Reviewed-by: Erik Skultety <eskultet@redhat.com>

Re: [PATCH] libxl: fix crash when initializing driver
Posted by Ján Tomko 4 years ago
On a Friday in 2020, Jim Fehlig wrote:
>Commit 54a401af478 split out DriverConfigInit from DriverConfigNew, but
>then called it a bit late from libxlStateInitialize. The cfg is used in
>libxlDriverConfigLoadFile and when uninitialized results in a crash.
>Calling DriverConfigInit immediately after DriverConfigNew fixes the
>crash.
>

More specifically it's libxlGetAutoballoonConf which depends on
cfg->verInfo being filled if autoballoon is not set in the config file.
I don't have an idea how to neatly split it to maintain the separation
I intended to do in that commit.

>Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>---
> src/libxl/libxl_driver.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
Re: [PATCH] libxl: fix crash when initializing driver
Posted by Jim Fehlig 4 years ago
On 4/6/20 3:19 AM, Ján Tomko wrote:
> On a Friday in 2020, Jim Fehlig wrote:
>> Commit 54a401af478 split out DriverConfigInit from DriverConfigNew, but
>> then called it a bit late from libxlStateInitialize. The cfg is used in
>> libxlDriverConfigLoadFile and when uninitialized results in a crash.
>> Calling DriverConfigInit immediately after DriverConfigNew fixes the
>> crash.
>>
> 
> More specifically it's libxlGetAutoballoonConf which depends on
> cfg->verInfo being filled if autoballoon is not set in the config file.

And I was hoping no one would notice that ugly little detail :-).

> I don't have an idea how to neatly split it to maintain the separation
> I intended to do in that commit.

Before sending this patch I thought about splitting the check for dom0_mem out 
of libxlGetAutoballoonConf, but I couldn't think of a better place to put it. 
Although it is lumped with parsing libxl.conf it does keep the autoballoon logic 
in one small function, so in the end I decided to just leave it.

Regards,
Jim