[libvirt PATCH v2 0/1] Prevent expanding worker pool by zero
V1: https://listman.redhat.com/archives/libvir-list/2021-July/msg00217.html Changes since V1: * Fix this not in virThreadPoolExpand, but in virThreadPoolNewFull. * Expanded the commit message. Tim Wiederhake (1): virThreadPoolNewFull: Prevent expanding worker pool by zero src/util/virthreadpool.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.31.1
On libvirtd startup, the list of priority worker threads is uninitialized (`pool->prioWorkers` is NULL), and then "expanded" to zero (`prioWorkers`) entries. This causes `virThreadPoolExpand` to call `VIR_EXPAND_N` on a null pointer and an increment of zero. The zero increment triggers `virReallocN` to not actually allocate any memory and leave the pointer NULL, which, eventually, causes `memset(NULL, 0, 0)` to be called in `virExpandN`. `memset` is declared `__attribute__ ((__nonnull__ 1))`, which triggers the following warning when libvirt is compiled with address sanitizing enabled: $ meson -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address,undefined build && ninja -C build $ ./build/run build/src/libvirtd src/util/viralloc.c:82:5: runtime error: null pointer passed as argument 1, which is declared to never be null Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virthreadpool.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c index 9ddd86a679..92b7cac286 100644 --- a/src/util/virthreadpool.c +++ b/src/util/virthreadpool.c @@ -247,10 +247,10 @@ virThreadPoolNewFull(size_t minWorkers, pool->maxWorkers = maxWorkers; pool->maxPrioWorkers = prioWorkers; - if (virThreadPoolExpand(pool, minWorkers, false) < 0) + if ((minWorkers > 0) && virThreadPoolExpand(pool, minWorkers, false) < 0) goto error; - if (virThreadPoolExpand(pool, prioWorkers, true) < 0) + if ((prioWorkers > 0) && virThreadPoolExpand(pool, prioWorkers, true) < 0) goto error; return pool; -- 2.31.1
On 7/13/21 4:49 PM, Tim Wiederhake wrote:
On libvirtd startup, the list of priority worker threads is uninitialized (`pool->prioWorkers` is NULL), and then "expanded" to zero (`prioWorkers`) entries.
This causes `virThreadPoolExpand` to call `VIR_EXPAND_N` on a null pointer and an increment of zero. The zero increment triggers `virReallocN` to not actually allocate any memory and leave the pointer NULL, which, eventually, causes `memset(NULL, 0, 0)` to be called in `virExpandN`.
`memset` is declared `__attribute__ ((__nonnull__ 1))`, which triggers the following warning when libvirt is compiled with address sanitizing enabled:
$ meson -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address,undefined build && ninja -C build $ ./build/run build/src/libvirtd src/util/viralloc.c:82:5: runtime error: null pointer passed as argument 1, which is declared to never be null
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virthreadpool.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník -
Tim Wiederhake