Re: [libvirt] [PATCH 06/14] driver: tighten check for whether loadable module exists or not

[Michal Privoznik]

On 04/19/2018 07:09 PM, Daniel P. Berrangé wrote:
> Currently we do a access(R_OK) check to see whether a loadable module
> exists, treating failure as non-fatal. This is unreasonably loose, as a
> module which exists but has had incorrect permissions set will turn into
> a silent skip. We only want to skip loading if the module genuinely does
> not exist on disk, due to the optional package not being installed.
> 
> Furthermore, checking the return value of virDriverLoadModuleFile() is
> not a suitable witness that the module does not exist. This method can
> return NULL if dlopen() fails, for example due to being unable to
> resolve symbols in the library. This is should always be reported as an
> error because it is a sign of the bad installation where either the
> module build doesn't match the libvirtd build, or where some 3rd party
> libraries are missing or broken.
> 
> Both these problems can be fixed by using virFileExists in the caller
> instead.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/driver.c | 14 ++
>  1 file changed, 6 insertions(+), 8 deletions(-)

ACK

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 06/14] driver: tighten check for whether loadable module exists or not

[Daniel P . Berrangé]

Currently we do a access(R_OK) check to see whether a loadable module
exists, treating failure as non-fatal. This is unreasonably loose, as a
module which exists but has had incorrect permissions set will turn into
a silent skip. We only want to skip loading if the module genuinely does
not exist on disk, due to the optional package not being installed.

Furthermore, checking the return value of virDriverLoadModuleFile() is
not a suitable witness that the module does not exist. This method can
return NULL if dlopen() fails, for example due to being unable to
resolve symbols in the library. This is should always be reported as an
error because it is a sign of the bad installation where either the
module build doesn't match the libvirtd build, or where some 3rd party
libraries are missing or broken.

Both these problems can be fixed by using virFileExists in the caller
instead.

Signed-off-by: Daniel P. Berrangé 
---
 src/driver.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/driver.c b/src/driver.c
index ddda1e71f7..e02efe2615 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -53,11 +53,6 @@ virDriverLoadModuleFile(const char *file)
 
 VIR_DEBUG("Load module file '%s'", file);
 
-if (access(file, R_OK) < 0) {
-VIR_INFO("Module %s not accessible", file);
-return NULL;
-}
-
 virUpdateSelfLastChanged(file);
 
 if (!(handle = dlopen(file, flags)))
@@ -105,11 +100,14 @@ virDriverLoadModuleFull(const char *path,
 int (*regsym)(void);
 int ret = -1;
 
-if (!(rethandle = virDriverLoadModuleFile(path))) {
-ret = 1;
-goto cleanup;
+if (!virFileExists(path)) {
+VIR_INFO("Module '%s' does not exists", path);
+return 1;
 }
 
+if (!(rethandle = virDriverLoadModuleFile(path)))
+goto cleanup;
+
 if (!(regsym = virDriverLoadModuleFunc(rethandle, regfunc)))
 goto cleanup;
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list