1. Patchew
  2. Libvirt
  3. View series

[PATCH] Commit 9899c91515 introduced the potential for a NULL pointer dereference crash when the 'ctl' parameter to vshPrintStderr() is NULL. Below is an example backtrace of the crash when attempting to detach a non-existent disk with 'virsh detach-disk test vdz'

Jim Fehlig via Devel posted 1 patch 4 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20260804173218.217139-1-jfehlig@suse.com
tools/vsh.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] Commit 9899c91515 introduced the potential for a NULL pointer dereference crash when the 'ctl' parameter to vshPrintStderr() is NULL. Below is an example backtrace of the crash when attempting to detach a non-existent disk with 'virsh detach-disk test vdz'
Posted by Jim Fehlig via Devel 4 hours ago 
From: Jim Fehlig <jfehlig@suse.com>

 #0  vshPrintStderr (ctl=0x0, level=4, format=<optimized out>, ap=<optimized out>) at ../tools/vsh.c:2148
 #1  0x00005555555e6b2c in vshError (ctl=ctl@entry=0x0, format=<optimized out>) at ../tools/vsh.c:2170
 #2  0x00005555555afec7 in virshFindDisk
    (doc=doc@entry=0x555555696ee0 "<domain type='kvm' id='1'>\n  <name>test</name>\n  <uuid>9b27a21e-dfa2-4c1e-9d11-2de275aefb03</uuid>\n  <metadata>\n    <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1."..., path=0x555555651240 "vdz", type=type@entry=0) at ../tools/virsh-domain.c:13055
 #3  0x00005555555b0151 in cmdDetachDisk (ctl=0x7fffffffdac0, cmd=0x5555556914c0) at ../tools/virsh-domain.c:13231
 #4  0x00005555555e6ddb in vshCommandRun (ctl=ctl@entry=0x7fffffffdac0, cmd=0x5555556914c0) at ../tools/vsh.c:1383
 #5  0x000055555559347a in main (argc=<optimized out>, argv=0x7fffffffdeb8) at ../tools/virsh.c:908

Ensure 'ctl' is valid before dereferencing it.

Fixes: 9899c91515d79595d19800ccc895df18eb7bb3f7
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 tools/vsh.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index ccf2f21d64..667cc1a0d2 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2145,7 +2145,7 @@ vshPrintStderr(vshControl *ctl,
     if (ctl)
         vshOutputLogFile(ctl, level, str);
 
-    if (ctl->stderr_closed)
+    if (ctl && ctl->stderr_closed)
         return;
 
     /* Most output is to stdout, but if someone ran virsh 2>&1, then
-- 
2.51.0
Re: [PATCH] Commit 9899c91515 introduced the potential for a NULL pointer dereference crash when the 'ctl' parameter to vshPrintStderr() is NULL. Below is an example backtrace of the crash when attempting to detach a non-existent disk with 'virsh detach-disk test vdz'
Posted by Daniel P. Berrangé via Devel 4 hours ago 
On Tue, Aug 04, 2026 at 11:32:06AM -0600, Jim Fehlig via Devel wrote:
> From: Jim Fehlig <jfehlig@suse.com>
> 
>  #0  vshPrintStderr (ctl=0x0, level=4, format=<optimized out>, ap=<optimized out>) at ../tools/vsh.c:2148
>  #1  0x00005555555e6b2c in vshError (ctl=ctl@entry=0x0, format=<optimized out>) at ../tools/vsh.c:2170
>  #2  0x00005555555afec7 in virshFindDisk
>     (doc=doc@entry=0x555555696ee0 "<domain type='kvm' id='1'>\n  <name>test</name>\n  <uuid>9b27a21e-dfa2-4c1e-9d11-2de275aefb03</uuid>\n  <metadata>\n    <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1."..., path=0x555555651240 "vdz", type=type@entry=0) at ../tools/virsh-domain.c:13055
>  #3  0x00005555555b0151 in cmdDetachDisk (ctl=0x7fffffffdac0, cmd=0x5555556914c0) at ../tools/virsh-domain.c:13231
>  #4  0x00005555555e6ddb in vshCommandRun (ctl=ctl@entry=0x7fffffffdac0, cmd=0x5555556914c0) at ../tools/vsh.c:1383
>  #5  0x000055555559347a in main (argc=<optimized out>, argv=0x7fffffffdeb8) at ../tools/virsh.c:908
> 
> Ensure 'ctl' is valid before dereferencing it.
> 
> Fixes: 9899c91515d79595d19800ccc895df18eb7bb3f7
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>  tools/vsh.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

> 
> diff --git a/tools/vsh.c b/tools/vsh.c
> index ccf2f21d64..667cc1a0d2 100644
> --- a/tools/vsh.c
> +++ b/tools/vsh.c
> @@ -2145,7 +2145,7 @@ vshPrintStderr(vshControl *ctl,
>      if (ctl)
>          vshOutputLogFile(ctl, level, str);

Indeed this shows we've expected 'ctl' to be NULL already.

>  
> -    if (ctl->stderr_closed)
> +    if (ctl && ctl->stderr_closed)
>          return;
>  
>      /* Most output is to stdout, but if someone ran virsh 2>&1, then
> -- 
> 2.51.0
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|
Re: [PATCH] Commit 9899c91515 introduced the potential for a NULL pointer dereference crash when the 'ctl' parameter to vshPrintStderr() is NULL. Below is an example backtrace of the crash when attempting to detach a non-existent disk with 'virsh detach-disk test vdz'
Posted by Jim Fehlig via Devel 3 hours ago 
On 8/4/26 11:36 AM, Daniel P. Berrangé wrote:
> On Tue, Aug 04, 2026 at 11:32:06AM -0600, Jim Fehlig via Devel wrote:
>> From: Jim Fehlig <jfehlig@suse.com>
>>
>>   #0  vshPrintStderr (ctl=0x0, level=4, format=<optimized out>, ap=<optimized out>) at ../tools/vsh.c:2148
>>   #1  0x00005555555e6b2c in vshError (ctl=ctl@entry=0x0, format=<optimized out>) at ../tools/vsh.c:2170
>>   #2  0x00005555555afec7 in virshFindDisk
>>      (doc=doc@entry=0x555555696ee0 "<domain type='kvm' id='1'>\n  <name>test</name>\n  <uuid>9b27a21e-dfa2-4c1e-9d11-2de275aefb03</uuid>\n  <metadata>\n    <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1."..., path=0x555555651240 "vdz", type=type@entry=0) at ../tools/virsh-domain.c:13055
>>   #3  0x00005555555b0151 in cmdDetachDisk (ctl=0x7fffffffdac0, cmd=0x5555556914c0) at ../tools/virsh-domain.c:13231
>>   #4  0x00005555555e6ddb in vshCommandRun (ctl=ctl@entry=0x7fffffffdac0, cmd=0x5555556914c0) at ../tools/vsh.c:1383
>>   #5  0x000055555559347a in main (argc=<optimized out>, argv=0x7fffffffdeb8) at ../tools/virsh.c:908
>>
>> Ensure 'ctl' is valid before dereferencing it.
>>
>> Fixes: 9899c91515d79595d19800ccc895df18eb7bb3f7
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>>   tools/vsh.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
>>
>> diff --git a/tools/vsh.c b/tools/vsh.c
>> index ccf2f21d64..667cc1a0d2 100644
>> --- a/tools/vsh.c
>> +++ b/tools/vsh.c
>> @@ -2145,7 +2145,7 @@ vshPrintStderr(vshControl *ctl,
>>       if (ctl)
>>           vshOutputLogFile(ctl, level, str);
> 
> Indeed this shows we've expected 'ctl' to be NULL already.

Nod. And there are several cases where NULL ctl is explicitly passed to 
vshError, which passes it along to vshPrintStderr()

libvirt-src>  grep -r -F -o 'vshError(NULL,' * 2>/dev/null | wc -l
8

Regards,
Jim
Re: [PATCH] Commit 9899c91515 introduced the potential for a NULL pointer dereference crash when the 'ctl' parameter to vshPrintStderr() is NULL. Below is an example backtrace of the crash when attempting to detach a non-existent disk with 'virsh detach-disk test vdz'
Posted by Jim Fehlig via Devel 3 hours ago 
Dammit, I pushed this without a proper commit summary :-(. Apologies for 
dirtying the git history.

Regards,
Jim

On 8/4/26 11:52 AM, Jim Fehlig wrote:
> On 8/4/26 11:36 AM, Daniel P. Berrangé wrote:
>> On Tue, Aug 04, 2026 at 11:32:06AM -0600, Jim Fehlig via Devel wrote:
>>> From: Jim Fehlig <jfehlig@suse.com>
>>>
>>>   #0  vshPrintStderr (ctl=0x0, level=4, format=<optimized out>, ap=<optimized 
>>> out>) at ../tools/vsh.c:2148
>>>   #1  0x00005555555e6b2c in vshError (ctl=ctl@entry=0x0, format=<optimized 
>>> out>) at ../tools/vsh.c:2170
>>>   #2  0x00005555555afec7 in virshFindDisk
>>>      (doc=doc@entry=0x555555696ee0 "<domain type='kvm' id='1'>\n  
>>> <name>test</name>\n  <uuid>9b27a21e-dfa2-4c1e-9d11-2de275aefb03</uuid>\n  
>>> <metadata>\n    <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/ 
>>> xmlns/libvirt/domain/1."..., path=0x555555651240 "vdz", type=type@entry=0) 
>>> at ../tools/virsh-domain.c:13055
>>>   #3  0x00005555555b0151 in cmdDetachDisk (ctl=0x7fffffffdac0, 
>>> cmd=0x5555556914c0) at ../tools/virsh-domain.c:13231
>>>   #4  0x00005555555e6ddb in vshCommandRun (ctl=ctl@entry=0x7fffffffdac0, 
>>> cmd=0x5555556914c0) at ../tools/vsh.c:1383
>>>   #5  0x000055555559347a in main (argc=<optimized out>, argv=0x7fffffffdeb8) 
>>> at ../tools/virsh.c:908
>>>
>>> Ensure 'ctl' is valid before dereferencing it.
>>>
>>> Fixes: 9899c91515d79595d19800ccc895df18eb7bb3f7
>>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>>> ---
>>>   tools/vsh.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>
>>>
>>> diff --git a/tools/vsh.c b/tools/vsh.c
>>> index ccf2f21d64..667cc1a0d2 100644
>>> --- a/tools/vsh.c
>>> +++ b/tools/vsh.c
>>> @@ -2145,7 +2145,7 @@ vshPrintStderr(vshControl *ctl,
>>>       if (ctl)
>>>           vshOutputLogFile(ctl, level, str);
>>
>> Indeed this shows we've expected 'ctl' to be NULL already.
> 
> Nod. And there are several cases where NULL ctl is explicitly passed to 
> vshError, which passes it along to vshPrintStderr()
> 
> libvirt-src>  grep -r -F -o 'vshError(NULL,' * 2>/dev/null | wc -l
> 8
> 
> Regards,
> Jim
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.