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 :|