[PATCH 0/2] virlockspace: Fix two more crashes of virtlockd on the exec-restart code path
Peter Krempa (2): virLockSpacePreExecRestart: Avoid use-after-free virLockSpaceNewPostExecRestart: Fix out-of-bounds array access src/util/virlockspace.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) -- 2.29.2
Recent refactor marked 'object' which is returned from the function as autofree but forgot to use g_steal_pointer in the return statement to prevent freeing it. Fixes: 9a1651f64d7 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virlockspace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index f253091f39..9e80db6a0c 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -472,7 +472,7 @@ virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr lockspace) goto error; virMutexUnlock(&lockspace->lock); - return object; + return g_steal_pointer(&object); error: virMutexUnlock(&lockspace->lock); -- 2.29.2
'res->owners' is allocated to 'res->nOwners' elements, but unfortunately 'res->nOwners' doesn't contain the proper value until after the allocation so 0 elements are allocated. The following loop which assumes that the array has the right number of elements then accesses the pointer out of bounds. The bug was also faithfully converted from VIR_ALLOC_N to g_new0. Fixes: 4a3d6ed5ee0 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virlockspace.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index 9e80db6a0c..0d6cff3707 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -324,7 +324,6 @@ virLockSpacePtr virLockSpaceNewPostExecRestart(virJSONValuePtr object) const char *tmp; virJSONValuePtr owners; size_t j; - size_t m; res = g_new0(virLockSpaceResource, 1); res->fd = -1; @@ -384,9 +383,8 @@ virLockSpacePtr virLockSpaceNewPostExecRestart(virJSONValuePtr object) goto error; } - m = virJSONValueArraySize(owners); + res->nOwners = virJSONValueArraySize(owners); res->owners = g_new0(pid_t, res->nOwners); - res->nOwners = m; for (j = 0; j < res->nOwners; j++) { unsigned long long int owner; -- 2.29.2
On 3/12/21 10:23 AM, Peter Krempa wrote:
Peter Krempa (2): virLockSpacePreExecRestart: Avoid use-after-free virLockSpaceNewPostExecRestart: Fix out-of-bounds array access
src/util/virlockspace.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Privoznik -
Peter Krempa