[libvirt] [PATCH] Avoid wild securityManager pointer in tests
For some reason we are not setting the driver with memset() to zeros. But since commit 74abc3deac6e14ffa9151e425c6e6cd2b075aac5 driver->securityManager is being accessed and qemuagenttest started crashing due to that. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- I chose to just clear the pointer instead of clearing the whole driver with memset() and opening another can of worms. But I would rather do memset() to zeros. however, that can be done after a discussion. This needs to be pushed under the build-breaker rule... Done! tests/testutilsqemu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 8a4f567b0b2a..f2eacdded53d 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -558,6 +558,8 @@ int qemuTestDriverInit(virQEMUDriver *driver) if (virMutexInit(&driver->lock) < 0) return -1; + driver->securityManager = NULL; + driver->config = virQEMUDriverConfigNew(false); if (!driver->config) goto error; -- 2.7.0
On 01/09/2016 12:36 PM, Martin Kletzander wrote:
For some reason we are not setting the driver with memset() to zeros. But since commit 74abc3deac6e14ffa9151e425c6e6cd2b075aac5 driver->securityManager is being accessed and qemuagenttest started crashing due to that.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- I chose to just clear the pointer instead of clearing the whole driver with memset() and opening another can of worms. But I would rather do memset() to zeros. however, that can be done after a discussion. This needs to be pushed under the build-breaker rule... Done!
tests/testutilsqemu.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 8a4f567b0b2a..f2eacdded53d 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -558,6 +558,8 @@ int qemuTestDriverInit(virQEMUDriver *driver) if (virMutexInit(&driver->lock) < 0) return -1;
+ driver->securityManager = NULL; + driver->config = virQEMUDriverConfigNew(false); if (!driver->config) goto error;
Hmm, I wonder why I wasn't hitting this... - Cole
On Sat, Jan 09, 2016 at 05:32:46PM -0500, Cole Robinson wrote:
On 01/09/2016 12:36 PM, Martin Kletzander wrote:
For some reason we are not setting the driver with memset() to zeros. But since commit 74abc3deac6e14ffa9151e425c6e6cd2b075aac5 driver->securityManager is being accessed and qemuagenttest started crashing due to that.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- I chose to just clear the pointer instead of clearing the whole driver with memset() and opening another can of worms. But I would rather do memset() to zeros. however, that can be done after a discussion. This needs to be pushed under the build-breaker rule... Done!
tests/testutilsqemu.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 8a4f567b0b2a..f2eacdded53d 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -558,6 +558,8 @@ int qemuTestDriverInit(virQEMUDriver *driver) if (virMutexInit(&driver->lock) < 0) return -1;
+ driver->securityManager = NULL; + driver->config = virQEMUDriverConfigNew(false); if (!driver->config) goto error;
Hmm, I wonder why I wasn't hitting this...
I don't think it's because I am building without SELinux, but it will probably be one of the compiler options. It shouldn't be caused just by using gcc 5.3.0, but rather one of: lv_cv_static_analysis=no CFLAGS='-O0 -ggdb -fno-profile-arcs -std=gnu11 -ftest-coverage' or maybe just a coincidence =) Martin
On 09.01.2016 18:36, Martin Kletzander wrote:
For some reason we are not setting the driver with memset() to zeros. But since commit 74abc3deac6e14ffa9151e425c6e6cd2b075aac5 driver->securityManager is being accessed and qemuagenttest started crashing due to that.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- I chose to just clear the pointer instead of clearing the whole driver with memset() and opening another can of worms. But I would rather do memset() to zeros. however, that can be done after a discussion. This needs to be pushed under the build-breaker rule... Done!
tests/testutilsqemu.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 8a4f567b0b2a..f2eacdded53d 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -558,6 +558,8 @@ int qemuTestDriverInit(virQEMUDriver *driver) if (virMutexInit(&driver->lock) < 0) return -1;
+ driver->securityManager = NULL; + driver->config = virQEMUDriverConfigNew(false); if (!driver->config) goto error;
I know you already pushed this, but how about: memset(driver, 0, sizeof(*driver)); Michal
On Fri, Jan 15, 2016 at 01:01:46PM +0100, Michal Privoznik wrote:
On 09.01.2016 18:36, Martin Kletzander wrote:
For some reason we are not setting the driver with memset() to zeros. But since commit 74abc3deac6e14ffa9151e425c6e6cd2b075aac5 driver->securityManager is being accessed and qemuagenttest started crashing due to that.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- I chose to just clear the pointer instead of clearing the whole driver with memset() and opening another can of worms. But I would rather do memset() to zeros. however, that can be done after a discussion. This needs to be pushed under the build-breaker rule... Done!
tests/testutilsqemu.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 8a4f567b0b2a..f2eacdded53d 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -558,6 +558,8 @@ int qemuTestDriverInit(virQEMUDriver *driver) if (virMutexInit(&driver->lock) < 0) return -1;
+ driver->securityManager = NULL; + driver->config = virQEMUDriverConfigNew(false); if (!driver->config) goto error;
I know you already pushed this, but how about:
memset(driver, 0, sizeof(*driver));
That was what I suggested and wanted to do as well, but at that point in time I wanted to just fix that one wild pointer. Feel free to send the memset() patch, I believe we will all agree that it's something to have in.
Michal
participants (3)
-
Cole Robinson -
Martin Kletzander -
Michal Privoznik