[PATCH] util: Avoid double free in virProcessSetAffinity

Martin Kletzander posted 1 patch 3 years, 5 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/ad3395ba741b798f6a32800ccd051d31dbb9258e.1603803557.git.mkletzan@redhat.com
src/util/virprocess.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
[PATCH] util: Avoid double free in virProcessSetAffinity
Posted by Martin Kletzander 3 years, 5 months ago
The cpu mask was free()'d immediately on any error and at the end of the
function, where it was expected that it would either error out and return or
goto another allocation if the code was to fail.  However since commit
9514e24984ee the error path did not return in one new case which caused
double-free in such situation.  In order to make the code more straightforward
just free the mask after it's been used even before checking the return code of
the call.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1819801

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/util/virprocess.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 9366f0630c42..37413796b3f6 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -447,6 +447,7 @@ int virProcessSetAffinity(pid_t pid, virBitmapPtr map, bool quiet)
     int numcpus = 1024;
     size_t masklen;
     cpu_set_t *mask;
+    int rv = -1;
 
     VIR_DEBUG("Set process affinity on %lld", (long long)pid);
 
@@ -472,8 +473,10 @@ int virProcessSetAffinity(pid_t pid, virBitmapPtr map, bool quiet)
             CPU_SET_S(i, masklen, mask);
     }
 
-    if (sched_setaffinity(pid, masklen, mask) < 0) {
-        CPU_FREE(mask);
+    rv = sched_setaffinity(pid, masklen, mask);
+    CPU_FREE(mask);
+
+    if (rv < 0) {
         if (errno == EINVAL &&
             numcpus < (1024 << 8)) { /* 262144 cpus ought to be enough for anyone */
             numcpus = numcpus << 2;
@@ -488,7 +491,6 @@ int virProcessSetAffinity(pid_t pid, virBitmapPtr map, bool quiet)
             return -1;
         }
     }
-    CPU_FREE(mask);
 
     return 0;
 }
-- 
2.28.0

Re: [PATCH] util: Avoid double free in virProcessSetAffinity
Posted by Peter Krempa 3 years, 5 months ago
On Tue, Oct 27, 2020 at 13:59:17 +0100, Martin Kletzander wrote:
> The cpu mask was free()'d immediately on any error and at the end of the
> function, where it was expected that it would either error out and return or
> goto another allocation if the code was to fail.  However since commit
> 9514e24984ee the error path did not return in one new case which caused
> double-free in such situation.  In order to make the code more straightforward
> just free the mask after it's been used even before checking the return code of
> the call.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1819801
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/util/virprocess.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Reviewed-by: Peter Krempa <pkrempa@redhat.com>