From nobody Mon Feb 9 15:41:03 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=1686675173; cv=none; d=zohomail.com; s=zohoarc; b=KUPUB0XyyNoL0KO2EKhyuh6SzSnDfaAJMU1eE0FhzzO3s2oN171O5eWecmi2phfGANpLm7sJpKGxowllbKSoWjhyHL2Jqr+s2f2o3K/hX/I1ZKBc8phDk5oh8vFaRr/dcPb28INwZT+s+XZpz/Onvrt5yYFbQ2hFi7LyGTjBLJc= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1686675173; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=GKdk7/nxO9INGO+pOACUMmvq1LknMh+J6LivyxFJ/YM=; b=ZEsB5P3alFYHeTjzrAeXZAj4gXusvUSKm+ajsUi4Ckr3MfIWgZka2tI6DVotACjHzjNWaGz4MxtOuKVAYeCowjiuhyrkjHMCCMiRwJuouIUjBqUUvA8uLFns6HpJb3jPTir/twRRxn0EVQPt73T02AYkZ1tIiyitdKi7nQfDYls= 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) Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1686675173011127.40830811035664; Tue, 13 Jun 2023 09:52:53 -0700 (PDT) Received: from list by lists.xenproject.org with outflank-mailman.548310.856195 (Exim 4.92) (envelope-from ) id 1q97Fk-0000tE-VF; Tue, 13 Jun 2023 16:52:20 +0000 Received: by outflank-mailman (output) from mailman id 548310.856195; Tue, 13 Jun 2023 16:52: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 1q97Fk-0000t7-SR; Tue, 13 Jun 2023 16:52:20 +0000 Received: by outflank-mailman (input) for mailman id 548310; Tue, 13 Jun 2023 16:52:19 +0000 Received: from se1-gles-sth1-in.inumbo.com ([159.253.27.254] helo=se1-gles-sth1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1q97Fi-0000sz-SS for xen-devel@lists.xenproject.org; Tue, 13 Jun 2023 16:52:19 +0000 Received: from esa3.hc3370-68.iphmx.com (esa3.hc3370-68.iphmx.com [216.71.145.155]) by se1-gles-sth1.inumbo.com (Halon) with ESMTPS id a604f721-0a0a-11ee-b232-6b7b168915f2; Tue, 13 Jun 2023 18:52:16 +0200 (CEST) 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: a604f721-0a0a-11ee-b232-6b7b168915f2 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1686675136; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=IwpceYJKNzh/rUXf2UUdvMyT5o9XieuSGGNl5Y9TzsM=; b=VIFVtMks4L1/LjoQpj8wXspVVnDN7+g2O9Z8fV7kSgRpPgms2sSb6gnb RiHWch+yde0iXNFiUeDGKuPxFGQZA83f84yCuKBdoWEbGImQvTsitJpgI /Cq80wIAUQ46F1M+Y8h8KJQOnId/waSwqbTV/nx8A4J44AHBuHMLrN6Y6 4=; Authentication-Results: esa3.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none X-SBRS: 4.0 X-MesageID: 112658514 X-Ironport-Server: esa3.hc3370-68.iphmx.com X-Remote-IP: 162.221.156.123 X-Policy: $RELAYED IronPort-Data: A9a23:A88ViqupBxWaMJNcddCnrEaZrOfnVLBeMUV32f8akzHdYApBsoF/q tZmKW2BM6yDMDPxLdxxYIrg8RlVuMfTxtNmSlQ9ry5hFSsQ+JbJXdiXEBz9bniYRiHhoOCLz O1FM4Wdc5pkJpP4jk3wWlQ0hSAkjclkfpKlVKiffHg3HVQ+IMsYoUoLs/YjhYJ1isSODQqIu Nfjy+XSI1bg0DNvWo4uw/vrRChH4rKq4Fv0gnRkPaoQ5AGHzCFPZH4iDfrZw0XQE9E88tGSH 44v/JnhlkvF8hEkDM+Sk7qTWiXmlZaLYGBiIlIPM0STqkAqSh4ai87XB9JFAatjsB2bnsgZ9 Tl4ncfYpTHFnEH7sL91vxFwS0mSNEDdkVPNCSDXXce7lyUqf5ZwqhnH4Y5f0YAwo45K7W9yG fMwMCFXUz24le6K/LupZOg9qOQ6EtLoBdZK0p1g5Wmx4fcORJnCR+PB5MNC3Sd2jcdLdRrcT 5NHM3w1Nk2GOkARfA5NU/rSn8/x7pX7WxRepEiYuuwc5G/LwRYq+LPsLMDUapqBQsA9ckOw/ zucoTmpX0hGXDCZ4RCD8kCFi+7zpHPicZM4H6OCqsR6mmTGkwT/DzVJDADm8JFVkHWWS99Zb kAZ5Ccqhawz71CwCMnwWQWip3yJtQJaXMBfe8UYwgyQzqvf4y6CG3MJCDVGbbQOr9QqTDYn0 luImdLBBjF1trCRD3WH+d+8sjeaKSUTa2gYakcsTxYB4tTliJE+iFTIVNkLOJCyitr5CDTh2 QegpSI1h6gQpcMT3qD99lfC6xqmq4LVVAcz6kPSV3i88wJiTIe/Ysqj7l2z0BpbBN/HFB/b5 iFCwpXAqrlUVvlhiRBhXs0VDeuUys2XPAHgiExBHsgP1y2T4H2aKNU4DC5FGKt5DioVUWa3M BGN418Ju8M70GiCNvEuPd/oYyg+5e25TIm+CKiJBjZbSsIpHDJr6h2CcqJ5M4rFtEE32Z8yN p6AGSpHJSZLUP83pNZaqgp07FPK+szd7TmJLXwD5077uYdynVbMIVv/DHOAb/oi8ISPqxjP/ tBUOqOikksPDrCvPnaMoNJDdDjmyETX47is86S7kcbZe2Jb9JwJUaeNkdvNhaQ490iqqgs41 i7kARIJoLYOrXbGNR+LehhehEDHBP5CQYYAFXV0Zz6AgiFzCbtDGY9DL/Pbi5F7rr08pRO1J tFZE/i97gNnE2yXoW5BPMes8eSPtn2D3GqzAsZsWxBnF7YIeuAD0oSMktfHnMXWMheKiA== IronPort-HdrOrdr: A9a23:8pIMd6NmHqLCfMBcT4T155DYdb4zR+YMi2TDtnoBPCC9F/by+f xG88566faKskdsZJhNo7G90cq7MADhHOBOkOss1N6ZNWGNhILCFvAA0WKN+UyEJ8X0ntQtqp uJG8JFZOEZZjJB4voTL2ODfuoI8Z2/1OSNuM+b9nFqSGhRGtNdB8USMHfkLqWzLjM2dabQ0f Cnl7t6TkGbCBAqR/X+PGABQ+/A4/XTjfvdEGc7Li9i0hCKkTSrrJXnEx2Uty1uLg9n8PMZ6G 3YlA68wa2mv5iAu3jh/l6W1Y1ShNzijv1cA8CW4/JlTAnEu0KTfYF8XL/HhhAZydvfkGoCoZ 33uhI9OMY20X/LYW2vhhPo12DboU0Twk6n80acnXzg5fP0Xyg7Dc0pv/MiTifk X-Talos-CUID: =?us-ascii?q?9a23=3AbpolF2m44VtbHZ42Q0oUbBlGZiHXOWXMlkr9GF2?= =?us-ascii?q?0NVpgWpOUGHGw4Z1/kPM7zg=3D=3D?= X-Talos-MUID: 9a23:4Lbx5wkl6AuTcJGUbpakdnpIMsdayqmvNHldiLwXkcKaD3BKKy2k2WE= X-IronPort-AV: E=Sophos;i="6.00,240,1681185600"; d="scan'208";a="112658514" From: Andrew Cooper To: Xen-devel CC: Andrew Cooper , Jan Beulich , =?UTF-8?q?Roger=20Pau=20Monn=C3=A9?= , Wei Liu , Stefano Stabellini , Julien Grall , Volodymyr Babchuk , Bertrand Marquis Subject: [PATCH] xen/gnttab: Purge PIN_FAIL() Date: Tue, 13 Jun 2023 17:52:09 +0100 Message-ID: <20230613165209.3121038-1-andrew.cooper3@citrix.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @citrix.com) X-ZM-MESSAGEID: 1686675176130100001 This is disliked specifically by MISRA, but it also interferes with code legibility by hiding control flow. Expand and drop it. * Drop redundant "rc =3D rc" assignment * Rework gnttab_copy_buf() to be simpler by dropping the rc variable No functional change. Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich Reviewed-by: Julien Grall --- CC: Jan Beulich CC: Roger Pau Monn=C3=A9 CC: Wei Liu CC: Stefano Stabellini CC: Julien Grall CC: Volodymyr Babchuk CC: Bertrand Marquis --- xen/common/grant_table.c | 154 ++++++++++++++++++++++++++++----------- 1 file changed, 111 insertions(+), 43 deletions(-) diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index d87e58a53d86..b62f40f2543c 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -270,13 +270,6 @@ struct gnttab_unmap_common { #define GNTTAB_UNMAP_BATCH_SIZE 32 =20 =20 -#define PIN_FAIL(_lbl, _rc, _f, _a...) \ - do { \ - gdprintk(XENLOG_WARNING, _f, ## _a ); \ - rc =3D (_rc); \ - goto _lbl; \ - } while ( 0 ) - /* * Tracks a mapping of another domain's grant reference. Each domain has a * table of these, indexes into which are returned as a 'mapping handle'. @@ -785,9 +778,13 @@ static int _set_status_v1(const grant_entry_header_t *= shah, /* If not already pinned, check the grant domid and type. */ if ( !act->pin && (((scombo.flags & mask) !=3D GTF_permit_access) = || (scombo.domid !=3D ldomid)) ) - PIN_FAIL(done, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "Bad flags (%x) or dom (%d); expected d%d\n", scombo.flags, scombo.domid, ldomid); + rc =3D GNTST_general_error; + goto done; + } =20 new =3D scombo; new.flags |=3D GTF_reading; @@ -796,8 +793,12 @@ static int _set_status_v1(const grant_entry_header_t *= shah, { new.flags |=3D GTF_writing; if ( unlikely(scombo.flags & GTF_readonly) ) - PIN_FAIL(done, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "Attempt to write-pin a r/o grant entry\n"); + rc =3D GNTST_general_error; + goto done; + } } =20 prev.raw =3D guest_cmpxchg(rd, raw_shah, scombo.raw, new.raw); @@ -805,8 +806,11 @@ static int _set_status_v1(const grant_entry_header_t *= shah, break; =20 if ( retries++ =3D=3D 4 ) - PIN_FAIL(done, GNTST_general_error, - "Shared grant entry is unstable\n"); + { + gdprintk(XENLOG_WARNING, "Shared grant entry is unstable\n"); + rc =3D GNTST_general_error; + goto done; + } =20 scombo =3D prev; } @@ -840,9 +844,13 @@ static int _set_status_v2(const grant_entry_header_t *= shah, ((((scombo.flags & mask) !=3D GTF_permit_access) && (mapflag || ((scombo.flags & mask) !=3D GTF_transitive))) || (scombo.domid !=3D ldomid)) ) - PIN_FAIL(done, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "Bad flags (%x) or dom (%d); expected d%d, flags %x\n", scombo.flags, scombo.domid, ldomid, mask); + rc =3D GNTST_general_error; + goto done; + } =20 if ( readonly ) { @@ -851,8 +859,12 @@ static int _set_status_v2(const grant_entry_header_t *= shah, else { if ( unlikely(scombo.flags & GTF_readonly) ) - PIN_FAIL(done, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "Attempt to write-pin a r/o grant entry\n"); + rc =3D GNTST_general_error; + goto done; + } *status |=3D GTF_reading | GTF_writing; } =20 @@ -870,9 +882,11 @@ static int _set_status_v2(const grant_entry_header_t *= shah, (!readonly && (scombo.flags & GTF_readonly)) ) { gnttab_clear_flags(rd, GTF_writing | GTF_reading, status); - PIN_FAIL(done, GNTST_general_error, + gdprintk(XENLOG_WARNING, "Unstable flags (%x) or dom (%d); expected d%d (r/w: = %d)\n", scombo.flags, scombo.domid, ldomid, !readonly); + rc =3D GNTST_general_error; + goto done; } } else @@ -880,8 +894,9 @@ static int _set_status_v2(const grant_entry_header_t *s= hah, if ( unlikely(scombo.flags & GTF_readonly) ) { gnttab_clear_flags(rd, GTF_writing, status); - PIN_FAIL(done, GNTST_general_error, - "Unstable grant readonly flag\n"); + gdprintk(XENLOG_WARNING, "Unstable grant readonly flag\n"); + rc =3D GNTST_general_error; + goto done; } } =20 @@ -1050,8 +1065,12 @@ map_grant_ref( /* Bounds check on the grant ref */ ref =3D op->ref; if ( unlikely(ref >=3D nr_grant_entries(rgt))) - PIN_FAIL(unlock_out, GNTST_bad_gntref, "Bad ref %#x for d%d\n", + { + gdprintk(XENLOG_WARNING, "Bad ref %#x for d%d\n", ref, rgt->domain->domain_id); + rc =3D GNTST_bad_gntref; + goto unlock_out; + } =20 /* This call also ensures the above check cannot be passed speculative= ly */ shah =3D shared_entry_header(rgt, ref); @@ -1062,9 +1081,13 @@ map_grant_ref( ((act->domid !=3D ld->domain_id) || (act->pin & GNTPIN_incr2oflow_mask(pin_incr)) || (act->is_sub_page)) ) - PIN_FAIL(act_release_out, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "Bad domain (%d !=3D %d), or risk of counter overflow %08= x, or subpage %d\n", act->domid, ld->domain_id, act->pin, act->is_sub_page); + rc =3D GNTST_general_error; + goto act_release_out; + } =20 /* Make sure we do not access memory speculatively */ status =3D evaluate_nospec(rgt->gt_version =3D=3D 1) ? &shah->flags @@ -1465,9 +1488,13 @@ unmap_common( =20 if ( op->dev_bus_addr && (flags & GNTMAP_device_map) && unlikely(op->dev_bus_addr !=3D mfn_to_maddr(act->mfn)) ) - PIN_FAIL(act_release_out, GNTST_bad_dev_addr, + { + gdprintk(XENLOG_WARNING, "Bus address doesn't match gntref (%"PRIx64" !=3D %"PRIpa= ddr")\n", op->dev_bus_addr, mfn_to_maddr(act->mfn)); + rc =3D GNTST_bad_dev_addr; + goto act_release_out; + } =20 if ( op->host_addr && (flags & GNTMAP_host_map) ) { @@ -2560,8 +2587,11 @@ acquire_grant_for_copy( grant_read_lock(rgt); =20 if ( unlikely(gref >=3D nr_grant_entries(rgt)) ) - PIN_FAIL(gt_unlock_out, GNTST_bad_gntref, - "Bad grant reference %#x\n", gref); + { + gdprintk(XENLOG_WARNING, "Bad grant reference %#x\n", gref); + rc =3D GNTST_bad_gntref; + goto gt_unlock_out; + } =20 /* This call also ensures the above check cannot be passed speculative= ly */ shah =3D shared_entry_header(rgt, gref); @@ -2571,9 +2601,13 @@ acquire_grant_for_copy( if ( act->pin && ((act->domid !=3D ldom) || (act->pin & GNTPIN_incr2oflow_mask(pin_incr))) ) - PIN_FAIL(unlock_out, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "Bad domain (%d !=3D %d), or risk of counter overflow %08= x\n", act->domid, ldom, act->pin); + rc =3D GNTST_general_error; + goto unlock_out; + } =20 if ( evaluate_nospec(rgt->gt_version =3D=3D 1) ) { @@ -2596,16 +2630,24 @@ acquire_grant_for_copy( goto unlock_out; =20 if ( !allow_transitive ) - PIN_FAIL(unlock_out_clear, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "transitive grant when transitivity not allowed\n"); + rc =3D GNTST_general_error; + goto unlock_out_clear; + } =20 trans_domid =3D sha2->transitive.trans_domid; trans_gref =3D sha2->transitive.gref; barrier(); /* Stop the compiler from re-loading trans_domid from shared memory */ if ( trans_domid =3D=3D rd->domain_id ) - PIN_FAIL(unlock_out_clear, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "transitive grants cannot be self-referential\n"); + rc =3D GNTST_general_error; + goto unlock_out_clear; + } =20 /* * We allow the trans_domid =3D=3D ldom case, which corresponds to= a @@ -2618,9 +2660,13 @@ acquire_grant_for_copy( /* We need to leave the rrd locked during the grant copy. */ td =3D rcu_lock_domain_by_id(trans_domid); if ( td =3D=3D NULL ) - PIN_FAIL(unlock_out_clear, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "transitive grant referenced bad domain %d\n", trans_domid); + rc =3D GNTST_general_error; + goto unlock_out_clear; + } =20 /* * acquire_grant_for_copy() will take the lock on the remote table, @@ -2928,8 +2974,11 @@ static int gnttab_copy_claim_buf(const struct gnttab= _copy *op, rc =3D get_paged_frame(ptr->u.gmfn, &buf->mfn, &buf->page, buf->read_only, buf->domain); if ( rc !=3D GNTST_okay ) - PIN_FAIL(out, rc, + { + gdprintk(XENLOG_WARNING, "source frame %"PRI_xen_pfn" invalid\n", ptr->u.gmfn); + goto out; + } =20 buf->ptr.u.gmfn =3D ptr->u.gmfn; buf->ptr.offset =3D 0; @@ -2972,25 +3021,29 @@ static int gnttab_copy_buf(const struct gnttab_copy= *op, struct gnttab_copy_buf *dest, const struct gnttab_copy_buf *src) { - int rc; - if ( ((op->source.offset + op->len) > PAGE_SIZE) || ((op->dest.offset + op->len) > PAGE_SIZE) ) - PIN_FAIL(out, GNTST_bad_copy_arg, "copy beyond page area\n"); + { + gdprintk(XENLOG_WARNING, "copy beyond page area\n"); + return GNTST_bad_copy_arg; + } =20 if ( op->source.offset < src->ptr.offset || op->source.offset + op->len > src->ptr.offset + src->len ) - PIN_FAIL(out, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "copy source out of bounds: %d < %d || %d > %d\n", - op->source.offset, src->ptr.offset, - op->len, src->len); + op->source.offset, src->ptr.offset, op->len, src->len); + return GNTST_general_error; + } =20 if ( op->dest.offset < dest->ptr.offset || op->dest.offset + op->len > dest->ptr.offset + dest->len ) - PIN_FAIL(out, GNTST_general_error, - "copy dest out of bounds: %d < %d || %d > %d\n", - op->dest.offset, dest->ptr.offset, - op->len, dest->len); + { + gdprintk(XENLOG_WARNING, "copy dest out of bounds: %d < %d || %d >= %d\n", + op->dest.offset, dest->ptr.offset, op->len, dest->len); + return GNTST_general_error; + } =20 /* Make sure the above checks are not bypassed speculatively */ block_speculation(); @@ -2998,9 +3051,8 @@ static int gnttab_copy_buf(const struct gnttab_copy *= op, memcpy(dest->virt + op->dest.offset, src->virt + op->source.offset, op->len); gnttab_mark_dirty(dest->domain, dest->mfn); - rc =3D GNTST_okay; - out: - return rc; + + return GNTST_okay; } =20 static int gnttab_copy_one(const struct gnttab_copy *op, @@ -3373,9 +3425,17 @@ swap_grant_ref(grant_ref_t ref_a, grant_ref_t ref_b) =20 /* Bounds check on the grant refs */ if ( unlikely(ref_a >=3D nr_grant_entries(d->grant_table))) - PIN_FAIL(out, GNTST_bad_gntref, "Bad ref-a %#x\n", ref_a); + { + gdprintk(XENLOG_WARNING, "Bad ref-a %#x\n", ref_a); + rc =3D GNTST_bad_gntref; + goto out; + } if ( unlikely(ref_b >=3D nr_grant_entries(d->grant_table))) - PIN_FAIL(out, GNTST_bad_gntref, "Bad ref-b %#x\n", ref_b); + { + gdprintk(XENLOG_WARNING, "Bad ref-b %#x\n", ref_b); + rc =3D GNTST_bad_gntref; + goto out; + } =20 /* Make sure the above checks are not bypassed speculatively */ block_speculation(); @@ -3386,11 +3446,19 @@ swap_grant_ref(grant_ref_t ref_a, grant_ref_t ref_b) =20 act_a =3D active_entry_acquire(gt, ref_a); if ( act_a->pin ) - PIN_FAIL(out, GNTST_eagain, "ref a %#x busy\n", ref_a); + { + gdprintk(XENLOG_WARNING, "ref a %#x busy\n", ref_a); + rc =3D GNTST_eagain; + goto out; + } =20 act_b =3D active_entry_acquire(gt, ref_b); if ( act_b->pin ) - PIN_FAIL(out, GNTST_eagain, "ref b %#x busy\n", ref_b); + { + gdprintk(XENLOG_WARNING, "ref b %#x busy\n", ref_b); + rc =3D GNTST_eagain; + goto out; + } =20 if ( evaluate_nospec(gt->gt_version =3D=3D 1) ) { --=20 2.30.2