From nobody Mon Feb 9 11:33:06 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass(p=reject dis=none) header.from=citrix.com ARC-Seal: i=1; a=rsa-sha256; t=1613580218; cv=none; d=zohomail.com; s=zohoarc; b=iTb1luLKUowBDIQiN4AIgg5AbqUYvmKFRdhVw42CPW/HFOIfc4vBaTsXkaYw3DfW1elkLuR8iS6+pvf0ffmU9LFvut+wqd7VuagsTgO8GUA15yPBRlsLkR8a7cUY3eZV3xkVfEBWydSl9D85c4Zdu+KlJhfv4u7utivLs4/Rh6M= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1613580218; h=Content-Type:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=rMsI8ILeJUwUYaDE0z8BQNsfcspk2tTJmQUrIo9Z1J8=; b=VUlzKGZqJAwotS5WX3mF7nKWxshuSnvw6GIZg+RWpmpY5jZQ8pu2SkOL/R2ca8F7iRT0IE0TekVJJ5ALCLXtUDqTYeOR/RGuUYjTw7fQDq6EjAYbwpGaq8Applp9LjViVtvrXSg7g27SWGSS/y8uuquuOJVQd19Dg23dIKPDwdQ= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass header.from= (p=reject dis=none) header.from= Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1613580218464255.71196282082883; Wed, 17 Feb 2021 08:43:38 -0800 (PST) Received: from list by lists.xenproject.org with outflank-mailman.86352.162084 (Exim 4.92) (envelope-from ) id 1lCPv6-0003Ye-4h; Wed, 17 Feb 2021 16:43:20 +0000 Received: by outflank-mailman (output) from mailman id 86352.162084; Wed, 17 Feb 2021 16:43:20 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1lCPv5-0003YQ-TC; Wed, 17 Feb 2021 16:43:19 +0000 Received: by outflank-mailman (input) for mailman id 86352; Wed, 17 Feb 2021 16:43:18 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1lCPv4-0003Xs-2T for xen-devel@lists.xenproject.org; Wed, 17 Feb 2021 16:43:18 +0000 Received: from esa6.hc3370-68.iphmx.com (unknown [216.71.155.175]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id b3dac973-a7df-4fe8-9898-4f22d82209cc; Wed, 17 Feb 2021 16:43:16 +0000 (UTC) X-Outflank-Mailman: Message body and most headers restored to incoming version X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: b3dac973-a7df-4fe8-9898-4f22d82209cc DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1613580196; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version; bh=Rf44n99j0ALTC6ho3Q828btTMXx7uCUyMKUasv55xao=; b=Fm8Iy5htOtH/gcUefKdml5qdYcNV8k9l/qjm3MDjfUKJshHew+YRkOX+ gedntByLm4r+IzJjy475ltCrRSRX66Jx05RcwnU/JDvbtxbY3z7kNraJO 5X0KTvGOX2M2zv7PQDruaNsj3nJR7ppc7uN4esXJuc+AC+Slg8JNcfm79 E=; Authentication-Results: esa6.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none IronPort-SDR: SiPIjv4hMhCbTbF9Li2GzodEYaWtYzebbOnjDJq1FDMG9tEThZ2R7a9YCB48cgX6uEHaBITRT4 S1MU13tEemk7tW/ioIcGjF3UMYuDCTRiOekD4tbY4hplH0tDOpfIQGN5Z4bTP+ON3xjQ9CxNWJ ODJhNaW4yjyv66kmcCP2HZvWNq19LrStlzhADTAWsT/S3N9ZtcMBm6aBTGHb2M/SVhz2gxeKPm 3hvAURFkEalFszMdzDLQXKkxRIrBj6DrabDr3wPY/AFrgYtcJAGoiiLmwrJFj0vt5GsYNMxRap C/w= X-SBRS: 5.1 X-MesageID: 37627938 X-Ironport-Server: esa6.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.81,184,1610427600"; d="scan'208";a="37627938" From: Andrew Cooper To: Xen-devel CC: Andrew Cooper , Ian Jackson , Wei Liu , Anthony PERARD Subject: [PATCH 3/3] tools/libxl: Rework invariants in libxl__domain_get_device_model_uid() Date: Wed, 17 Feb 2021 16:42:51 +0000 Message-ID: <20210217164251.11005-4-andrew.cooper3@citrix.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20210217164251.11005-1-andrew.cooper3@citrix.com> References: <20210217164251.11005-1-andrew.cooper3@citrix.com> MIME-Version: 1.0 X-ZohoMail-DKIM: pass (identity @citrix.com) Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Various version of gcc, when compiling with -Og, complain: libxl_dm.c: In function 'libxl__domain_get_device_model_uid': libxl_dm.c:256:12: error: 'kill_by_uid' may be used uninitialized in this= function [-Werror=3Dmaybe-uninitialized] 256 | if (kill_by_uid) | ^ The logic is very tangled. Two paths unconditionally set user before checking for associated errors. This interferes with the expected use of uninitialised-variable heuristics = to force compile failures for ill-formed exit paths. Use b_info->device_model_user and LIBXL_QEMU_USER_SHARED as appliable, and only set user on the goto out paths. All goto out paths now are comprised of the form: user =3D NULL; rc =3D 0; or: user =3D non-NULL; indended_uid =3D ...; kill_by_uid =3D ...; rc =3D 0; As a consequence, indended_uid can drop its default of -1, the dm_restrict = can drop its now-stale "just in case" comment and the redundant setting of kill_by_uid to work around this issue at other optimisation levels. Finally, rewrite the comment about invariants, indicating the split between the out and err lables, and associated rc values. Additionally, reword the "is {not,} set" terminology to "is {non-,}NULL" to be more precise. No funcational change. Signed-off-by: Andrew Cooper --- CC: Ian Jackson CC: Wei Liu CC: Anthony PERARD --- tools/libs/light/libxl_dm.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c index 7843c283ca..8a7e084d89 100644 --- a/tools/libs/light/libxl_dm.c +++ b/tools/libs/light/libxl_dm.c @@ -127,7 +127,7 @@ static int libxl__domain_get_device_model_uid(libxl__gc= *gc, struct passwd *user_base, user_pwbuf; int rc; char *user; - uid_t intended_uid =3D -1; + uid_t intended_uid; bool kill_by_uid; =20 /* Only qemu-upstream can run as a different uid */ @@ -135,33 +135,34 @@ static int libxl__domain_get_device_model_uid(libxl__= gc *gc, return 0; =20 /* - * From this point onward, all paths should go through the `out` - * label. The invariants should be: + * From this point onward, all paths should go through the `out` (succ= ess, + * rc =3D 0) or `err` (failure, rc !=3D 0) labels. The invariants sho= uld be: * - rc may be 0, or an error code. - * - if rc is an error code, user and intended_uid are ignored. - * - if rc is 0, user may be set or not set. - * - if user is set, then intended_uid must be set to a UID matching + * - if rc is an error code, all settings are ignored. + * - if rc is 0, user may be NULL or non-NULL. + * - if user is non-NULL, then intended_uid must be set to a UID match= ing * the username `user`, and kill_by_uid must be set to the appropria= te * value. intended_uid will be checked for root (0). */ - =20 + /* * If device_model_user is present, set `-runas` even if * dm_restrict isn't in use */ - user =3D b_info->device_model_user; - if (user) { - rc =3D userlookup_helper_getpwnam(gc, user, &user_pwbuf, &user_bas= e); + if (b_info->device_model_user) { + rc =3D userlookup_helper_getpwnam(gc, b_info->device_model_user, + &user_pwbuf, &user_base); if (rc) goto err; =20 if (!user_base) { LOGD(ERROR, guest_domid, "Couldn't find device_model_user %s", - user); + b_info->device_model_user); rc =3D ERROR_INVAL; goto err; } =20 + user =3D b_info->device_model_user; intended_uid =3D user_base->pw_uid; kill_by_uid =3D true; rc =3D 0; @@ -175,8 +176,7 @@ static int libxl__domain_get_device_model_uid(libxl__gc= *gc, if (!libxl_defbool_val(b_info->dm_restrict)) { LOGD(DEBUG, guest_domid, "dm_restrict disabled, starting QEMU as root"); - user =3D NULL; /* Should already be null, but just in case */ - kill_by_uid =3D false; /* Keep older versions of gcc happy */ + user =3D NULL; rc =3D 0; goto out; } @@ -219,13 +219,14 @@ static int libxl__domain_get_device_model_uid(libxl__= gc *gc, * QEMU_USER_SHARED. NB for QEMU_USER_SHARED, all QEMU will run * as the same UID, we can't kill by uid; therefore don't set uid. */ - user =3D LIBXL_QEMU_USER_SHARED; - rc =3D userlookup_helper_getpwnam(gc, user, &user_pwbuf, &user_base); + rc =3D userlookup_helper_getpwnam(gc, LIBXL_QEMU_USER_SHARED, + &user_pwbuf, &user_base); if (rc) goto err; if (user_base) { LOGD(WARN, guest_domid, "Could not find user %s, falling back to %= s", LIBXL_QEMU_USER_RANGE_BASE, LIBXL_QEMU_USER_SHARED); + user =3D LIBXL_QEMU_USER_SHARED; intended_uid =3D user_base->pw_uid; kill_by_uid =3D false; rc =3D 0; --=20 2.11.0