[PATCH v2] util: fix use-after-free in virIdentityGetSystem
We have a g_autoptr ret in the virIdentityGetSystem function. In the happy path it is properly returned by doing: return g_steal_pointer(&ret); There are 2 early return paths, were we do the following: "return ret;" This leads to the g_autoptr being cleaned up after we leave the function, as we do not properly "steal" it. When later using the return value we have a use-after-free, which has led to segfaults in some cases. As this is a regression introduced in 1280a631ef488aeaab905eb30a55899ef8ba97be, we change the behavior to properly return NULL in those cases. On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/util/viridentity.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/viridentity.c b/src/util/viridentity.c index b7b88056ac..8e59179a20 100644 --- a/src/util/viridentity.c +++ b/src/util/viridentity.c @@ -328,14 +328,14 @@ virIdentity *virIdentityGetSystem(void) return NULL; if (!(username = virGetUserName(geteuid()))) - return ret; + return NULL; if (virIdentitySetUserName(ret, username) < 0) return NULL; if (virIdentitySetUNIXUserID(ret, getuid()) < 0) return NULL; if (!(groupname = virGetGroupName(getegid()))) - return ret; + return NULL; if (virIdentitySetGroupName(ret, groupname) < 0) return NULL; if (virIdentitySetUNIXGroupID(ret, getgid()) < 0) -- 2.53.0
On 2/25/26 13:50, Stefan Kober wrote:
We have a g_autoptr ret in the virIdentityGetSystem function. In the happy path it is properly returned by doing: return g_steal_pointer(&ret);
There are 2 early return paths, were we do the following: "return ret;"
This leads to the g_autoptr being cleaned up after we leave the function, as we do not properly "steal" it.
When later using the return value we have a use-after-free, which has led to segfaults in some cases.
As this is a regression introduced in 1280a631ef488aeaab905eb30a55899ef8ba97be, we change the behavior to properly return NULL in those cases.
In fact, it was introduced in c6825d88137cb8e4debdf4310e45ee23cb5698c0.
On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/util/viridentity.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and merged. Michal
On Thu, Feb 26, 2026 at 10:21:35AM +0100, Michal Prívozník via Devel wrote:
On 2/25/26 13:50, Stefan Kober wrote:
We have a g_autoptr ret in the virIdentityGetSystem function. In the happy path it is properly returned by doing: return g_steal_pointer(&ret);
There are 2 early return paths, were we do the following: "return ret;"
This leads to the g_autoptr being cleaned up after we leave the function, as we do not properly "steal" it.
When later using the return value we have a use-after-free, which has led to segfaults in some cases.
As this is a regression introduced in 1280a631ef488aeaab905eb30a55899ef8ba97be, we change the behavior to properly return NULL in those cases.
In fact, it was introduced in c6825d88137cb8e4debdf4310e45ee23cb5698c0.
In fact the root cause was introduced in 5282ed8d1cb015810154143697a12cc1d73f8b83 it just wasn't a double-free at that point - merely a return of an incompletely populated identity object :-(
On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/util/viridentity.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
and merged.
Michal
With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
participants (3)
-
Daniel P. Berrangé -
Michal Prívozník -
Stefan Kober