On Wed, Feb 25, 2026 at 12:43:38PM +0100, 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 our case.
We fix the early returns by doing the same as in the happy path.
Actually the flaw here is that we should *NOT* have early returns that use 'ret' at all. An identity must only be returned if *all* data is fetched successfully. This is a regression accidentally introduced in commit 1280a631ef488aeaab905eb30a55899ef8ba97be Author: Daniel P. Berrangé <berrange@redhat.com> Date: Thu Aug 5 19:03:19 2021 +0100 src: stop checking virIdentityNew return value where I moved the 'virIdentityNew' call. Previously 'ret' would still be NULL in these places where we 'return ret', so it was effectively 'return NULL'. When I moved 'virIdentityNew' I broke this equivalence. So the fix here is to change 'return ret' to 'return NULL'
On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/util/viridentity.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/util/viridentity.c b/src/util/viridentity.c index b7b88056ac..10935fba60 100644 --- a/src/util/viridentity.c +++ b/src/util/viridentity.c @@ -327,15 +327,19 @@ virIdentity *virIdentityGetSystem(void) virIdentitySetProcessTime(ret, startTime) < 0) return NULL;
- if (!(username = virGetUserName(geteuid()))) - return ret; + if (!(username = virGetUserName(geteuid()))) { + VIR_WARN("virGetUserName failed, returning partial identity"); + return g_steal_pointer(&ret); + } if (virIdentitySetUserName(ret, username) < 0) return NULL; if (virIdentitySetUNIXUserID(ret, getuid()) < 0) return NULL;
- if (!(groupname = virGetGroupName(getegid()))) - return ret; + if (!(groupname = virGetGroupName(getegid()))) { + VIR_WARN("virGetGroupName failed, returning partial identity"); + return g_steal_pointer(&ret); + } if (virIdentitySetGroupName(ret, groupname) < 0) return NULL; if (virIdentitySetUNIXGroupID(ret, getgid()) < 0) -- 2.53.0
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 :|