[PATCH v3] virt-aa-helper: Fix unchecked return value

Dmitry Lopatin posted 1 patch 2 days, 8 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20260204192916.350664-1-dmitry.lopatin@flant.com
There is a newer version of this series
src/security/virt-aa-helper.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH v3] virt-aa-helper: Fix unchecked return value
Posted by Dmitry Lopatin 2 days, 8 hours ago
Add missing return value checks to fix the following issues reported
by the static analyzer:

 - virDriverLoadModule() call when loading the storage driver (line 908)
   was not checked, while there are examples with return code check
   throughout the code.
---
 src/security/virt-aa-helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index f4ec6b7826..2de1b31b5a 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -892,7 +892,8 @@ get_files(vahControl * ctl)
 
     /* load the storage driver so that backing store can be accessed */
 #ifdef WITH_STORAGE
-    virDriverLoadModule("storage", "storageRegister", false);
+    if (virDriverLoadModule("storage", "storageRegister", false) < 0)
+        goto cleanup;("storage", "storageRegister", false);
 #endif
 
     for (i = 0; i < ctl->def->ndisks; i++) {
-- 
2.34.1
Re: [PATCH v3] virt-aa-helper: Fix unchecked return value
Posted by Peter Krempa via Devel 18 hours ago
On Wed, Feb 04, 2026 at 22:29:16 +0300, Dmitry Lopatin wrote:
> Add missing return value checks to fix the following issues reported
> by the static analyzer:
> 
>  - virDriverLoadModule() call when loading the storage driver (line 908)
>    was not checked, while there are examples with return code check
>    throughout the code.

Your patch is missing declaration that it conforms to the Developer
Certificate of Origin, which is required for all our submissions:

https://www.libvirt.org/hacking.html#developer-certificate-of-origin

Although now I see that your v1 had this.

> ---
>  src/security/virt-aa-helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index f4ec6b7826..2de1b31b5a 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -892,7 +892,8 @@ get_files(vahControl * ctl)
>  
>      /* load the storage driver so that backing store can be accessed */
>  #ifdef WITH_STORAGE
> -    virDriverLoadModule("storage", "storageRegister", false);
> +    if (virDriverLoadModule("storage", "storageRegister", false) < 0)
> +        goto cleanup;("storage", "storageRegister", false);

                        ^^^^^^^^^^ This looks very broken


>  #endif
>  
>      for (i = 0; i < ctl->def->ndisks; i++) {
> -- 
> 2.34.1
>
[PATCH v4] virt-aa-helper: Fix unchecked return value
Posted by Dmitry Lopatin 17 hours ago
Add missing return value checks to fix the following issues reported
by the static analyzer:

 - virDriverLoadModule() call when loading the storage driver (line 908)
   was not checked, while there are examples with return code check
   throughout the code.

Signed-off-by: Dmitry Lopatin <dmitry.lopatin@flant.com>
---
 src/security/virt-aa-helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index f4ec6b7826..624da61a0f 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -892,7 +892,8 @@ get_files(vahControl * ctl)
 
     /* load the storage driver so that backing store can be accessed */
 #ifdef WITH_STORAGE
-    virDriverLoadModule("storage", "storageRegister", false);
+    if (virDriverLoadModule("storage", "storageRegister", false) < 0)
+        goto cleanup;
 #endif
 
     for (i = 0; i < ctl->def->ndisks; i++) {
-- 
2.34.1
Re: [PATCH v4] virt-aa-helper: Fix unchecked return value
Posted by Peter Krempa via Devel 17 hours ago
On Fri, Feb 06, 2026 at 13:03:04 +0300, Dmitry Lopatin wrote:
> Add missing return value checks to fix the following issues reported
> by the static analyzer:
> 
>  - virDriverLoadModule() call when loading the storage driver (line 908)
>    was not checked, while there are examples with return code check
>    throughout the code.
> 
> Signed-off-by: Dmitry Lopatin <dmitry.lopatin@flant.com>
> ---
>  src/security/virt-aa-helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index f4ec6b7826..624da61a0f 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -892,7 +892,8 @@ get_files(vahControl * ctl)
>  
>      /* load the storage driver so that backing store can be accessed */
>  #ifdef WITH_STORAGE
> -    virDriverLoadModule("storage", "storageRegister", false);
> +    if (virDriverLoadModule("storage", "storageRegister", false) < 0)
> +        goto cleanup;

There's no 'cleanup' label in this function:

FAILED: src/virt-aa-helper.p/security_virt-aa-helper.c.o
ccache cc -Isrc/virt-aa-helper.p -Isrc -I../src -Isrc/conf -I../src/conf -Isrc/hypervisor -I../src/hypervisor -Isrc/security -I../src/security -Isrc/storage_file -I../src/storage_file -Isrc/util -I../src/util -Iinclude -I../include -I. -I.. -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/gio-unix-2.0 -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/libxml2 -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu99 -O2 -g @/builds/pipo.sk/libvirt/build/c-warnings.txt -fPIE -pthread -MD -MQ src/virt-aa-helper.p/security_virt-aa-helper.c.o -MF src/virt-aa-helper.p/security_virt-aa-helper.c.o.d -o src/virt-aa-helper.p/security_virt-aa-helper.c.o -c ../src/security/virt-aa-helper.c
../src/security/virt-aa-helper.c: In function ‘get_files’:
../src/security/virt-aa-helper.c:896:9: error: label ‘cleanup’ used but not defined
  896 |         goto cleanup;
      |         ^~~~

https://gitlab.com/pipo.sk/libvirt/-/jobs/13015050448#L1767

[PATCH v5] virt-aa-helper: Fix unchecked return value
Posted by Dmitry Lopatin 16 hours ago
Add missing return value checks to fix the following issues reported
by the static analyzer:

 - virDriverLoadModule() call when loading the storage driver (line 908)
   was not checked, while there are examples with return code check
   throughout the code.

Signed-off-by: Dmitry Lopatin <dmitry.lopatin@flant.com>
---
 src/security/virt-aa-helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index f4ec6b7826..38625f3721 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -892,7 +892,8 @@ get_files(vahControl * ctl)
 
     /* load the storage driver so that backing store can be accessed */
 #ifdef WITH_STORAGE
-    virDriverLoadModule("storage", "storageRegister", false);
+    if (virDriverLoadModule("storage", "storageRegister", false) < 0)
+        return -1;
 #endif
 
     for (i = 0; i < ctl->def->ndisks; i++) {
-- 
2.34.1