From patchwork Thu Jul 27 07:38:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jan Beulich X-Patchwork-Id: 13329090 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 996C1C001DC for ; Thu, 27 Jul 2023 07:38:48 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.570806.892979 (Exim 4.92) (envelope-from ) id 1qOvZz-0000rr-BU; Thu, 27 Jul 2023 07:38:35 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 570806.892979; Thu, 27 Jul 2023 07:38:35 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qOvZz-0000ra-7B; Thu, 27 Jul 2023 07:38:35 +0000 Received: by outflank-mailman (input) for mailman id 570806; Thu, 27 Jul 2023 07:38:33 +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 1qOvZx-0000oV-Il for xen-devel@lists.xenproject.org; Thu, 27 Jul 2023 07:38:33 +0000 Received: from EUR05-AM6-obe.outbound.protection.outlook.com (mail-am6eur05on20627.outbound.protection.outlook.com [2a01:111:f400:7e1b::627]) by se1-gles-sth1.inumbo.com (Halon) with ESMTPS id 9636c91a-2c50-11ee-b247-6b7b168915f2; Thu, 27 Jul 2023 09:38:32 +0200 (CEST) Received: from DU2PR04MB8790.eurprd04.prod.outlook.com (2603:10a6:10:2e1::23) by DBBPR04MB8026.eurprd04.prod.outlook.com (2603:10a6:10:1ed::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6631.29; Thu, 27 Jul 2023 07:38:31 +0000 Received: from DU2PR04MB8790.eurprd04.prod.outlook.com ([fe80::e5cf:5743:ab60:b14e]) by DU2PR04MB8790.eurprd04.prod.outlook.com ([fe80::e5cf:5743:ab60:b14e%5]) with mapi id 15.20.6631.026; Thu, 27 Jul 2023 07:38:31 +0000 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: 9636c91a-2c50-11ee-b247-6b7b168915f2 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=S36Ixr4Z+Ln7tN/dSjpT2baYv+w+4WIJiQR4Y7v84Fm+jtYj2MFZjWvXX9jfOYlzeb3//uzQVAU9LPIVp5Y0KMOmFavrmOw2MXoNZrSXTonHXQ7MAbCbFLkP8VsfDVHV9Vo0cKC4TJYM6snAho+ZqwvE7jxptQ1ZyIpDiboUpLvB2WSzEwTlhvA6q5JkrWAHv90IUL++jVckFvUIvgKBHvuoItsuF4V89wPg9Pvu7UJSCwM16rlQ4jILKfUr31WP9HfhWoAYtLlAq9QJOoBQqibggntGDOo4SXRJ0dCPU9LgT/a5/BnuZ0mRfl/AYlNiKtnFKwxQkhk1BdUBiIZBYg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=42G1q1cbtqLAMoOILaHON+bdi1vm2gokigDbkHTcLas=; b=im54/SKoplx/5mwC6VfvATT4eOkoJI7tyUjARqxjVN5ipGGcNV7SU+Dd/F61wI29u2vLS/cmzjv6E6ePCZKhD3UeT61rrCdr8omA9YMCHdhGvRBdxDhCYVWEYREY3zqNNrSKEtNS+w3vbAyGOwk7KI6f+eXt15leHVQ7j616GCcNwDyYSqjLSDJwYxzeCEOVXoV7pl8bvO+1MldyQpRkl8w65N6G9/jodYJc6lBD0o5Tx7nH3a8s8T9mTZvd4sbWTih2BcnfvnWFZ113V/05pM5baLH2pxekg4fymNa8f236e9Zj1NehNvyXRybWeuvjAspx/dIM3FvU3RaW88erIg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=42G1q1cbtqLAMoOILaHON+bdi1vm2gokigDbkHTcLas=; b=C0f1ociMyS9vc9x+0AzwYlBDRBJ2/UxqacI/na6zD+u+AdonkZvChNia62dtt/tz4YlDTSVK+0VeH7AHSdoCUU27RfR0WiRVLvv/MHXJLheKlYh3lgemwBXVKSPXGlAbElpXgRvSYpvD222Jh7BlHM7v0kPCm2zhmHboZu+p4ima15joL+SvgPwYl79uPTfkUTSj8scy6J16rKFoU2v3fXZSQD1Syf0E9PIbedDnhhYoglz8mvXlMKqdx68eZXoLOx2v6t7bfGyiyH4TZVUQU59xGdjB5vDiOBLtst9/Ev5vvtDJIh7Qs1BtNUKfv/nZIKcpdDLPJx8CfHLNVjN0Wg== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com; Message-ID: <5641f8eb-5736-8ccc-122b-b3b47c1bac28@suse.com> Date: Thu, 27 Jul 2023 09:38:29 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: [PATCH v3 2/3] pirq_cleanup_check() leaks Content-Language: en-US From: Jan Beulich To: "xen-devel@lists.xenproject.org" Cc: Andrew Cooper , George Dunlap , Julien Grall , Stefano Stabellini , Wei Liu , =?utf-8?q?Roger_Pau_Monn=C3=A9?= References: <7cb0d24e-2bda-dcda-4874-2c01ba179947@suse.com> In-Reply-To: <7cb0d24e-2bda-dcda-4874-2c01ba179947@suse.com> X-ClientProxiedBy: FR0P281CA0245.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:af::6) To DU2PR04MB8790.eurprd04.prod.outlook.com (2603:10a6:10:2e1::23) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DU2PR04MB8790:EE_|DBBPR04MB8026:EE_ X-MS-Office365-Filtering-Correlation-Id: d0e47f9d-840e-464a-5cba-08db8e747992 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: SQg/baH/L+3HTi5t7EeuF7Jhyc36KxHdDA/186cQTCKdqBaQBR3kCTRqmRPvkVptJPolJL1kbmkVqYYh8GnB4SEI/zmLASS4+c9P0ebAKY3+hho5p/NBEczDvJbofFnkcs0+hk8n1Q29UsRNvTnT3Ewk9LoHl6OGbIXuwj2DxBqLrFaC/cciePAI/8tQshKOpThh5CVZuShAyCFocY2jiCmCtx4KOejW1mPs5X2ugqiPGRtt3xosfHJWmlJFYVj4TWw1Hhmi/Zn1tMuG317a9omN/zATFCbLygIeluSyd0Us5T/we8vh2uE9+keft3L3Zj/HjlJOAWwmq0hw6xLn5hLGchSoOBBUy53nKJNl8FCrwvEDCPDXXSkR32aonyAKH03J8n8YBbZOFcAQFJFPdd6nemJvv5rQKTAvIj/kaFQ7rMnCy72yh5TT5Pipd1gyKqoX9MDxok9wirtrNGMl0PnVPl/2+LRoL9Ii3VtS+KswnwXM9273upk5UxtemDCu5Me0pp3qxDmtG4QTUD00AgZpESeSNPYyp3lpL9emORIr50h5AxxE1VaAlnyPtwsgFiJbk6eKQeFp5xWFZJW53wsFVZT+O3LtLv5Zuab2jJM9vqR5qxOD5ej9TARvV6dGt4s4MREPYYRnIAJ+5ZH/UA== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DU2PR04MB8790.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(346002)(396003)(39860400002)(136003)(366004)(376002)(451199021)(6486002)(478600001)(83380400001)(6506007)(26005)(6512007)(6916009)(4326008)(54906003)(66556008)(66476007)(66946007)(186003)(2616005)(38100700002)(31686004)(8936002)(8676002)(5660300002)(2906002)(316002)(41300700001)(86362001)(31696002)(36756003)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?q?EPaCki54CgDVES2fcrJdYHSleuof?= =?utf-8?q?8sl/tfa6NVnZcMhn1LylLAlDiObWywuz9melaop2CJ7MEHWwgrO6cEJThNXY88Vxb?= =?utf-8?q?VA5h2tDe3FpfjppxBxTWAXFa7yFoCdzEK137zZ2CydD3wzYrsgAAB08kK+6yT7B+5?= =?utf-8?q?JIPmi3Iif/wolwWEBQH9f19titPF4Q+N4Qm9w6t9jNL+X5Itac1w/pa9esWWZu9hi?= =?utf-8?q?1yu18DaZWVQGN/XPVIGNJi862SzgaFDnsqxeUBOtScCy/AlG0bMeOVb8Q4ttb/XSS?= =?utf-8?q?NmtgYLN28zYrXtpK/3gpkm1g8oFcEtcPSpajxwbZo+PfavIwC9kM7at6jL28efuS0?= =?utf-8?q?/n2wf4wdNAA+TG36Q5UjiKc+cuVvll6u52YziaGU80YyZMRvZ04FG5ZOvq302X0CU?= =?utf-8?q?ag8cXFJRW2enc7QCo172NSRB7vOXesgsjqxWaC8tpDAlWBVSHzPWre054pMi3Ix2Y?= =?utf-8?q?JHPBHLMP0ToEuHN8VS/NxrEyUYvVWj9Yh6o4E2kJN/Gn/hsZa1RJ7yvVQO1gyRqoj?= =?utf-8?q?SB58KmRwgflUseLqJ1lGiyF5cROHY59eDGYj1Ie5MJasGOsQAYmf+GcswiXapEknV?= =?utf-8?q?m9T5VUBV0a0oFDDpAoQxbU6kcOCmlcRdW0X9ypCDwce6asyrP1G2zWFj0jCwX2jtQ?= =?utf-8?q?3rOANEM3TK18MUKf0kclnfZPdk7zb16dqW+99G6bdXxHkXZFFmFTmW0tr2OzVDahQ?= =?utf-8?q?HoWnUn26mU9cNfpG4vmnkGimfmM6KxUFcixMh1ny/tCDbXkAew5jBXV5j7+IQrWP+?= =?utf-8?q?ohuEGFeLZcpLINbS2gzA3x5Ac9587WcesiqFAgRflSdWN6Z2iQLuEaYVp5Clhgkcy?= =?utf-8?q?gpB3MkTv9hW//ITGaadZ8uITrOtSK/0BY9w2KalNA/e3gn4Rxi/g6+Tms8DOgaLZk?= =?utf-8?q?IEkXrprWVeLFW7+zgKAZcPYOsEZVa0WeVsOtOtxavRXFhtK5sjrvJj8F/Iy9Ju+fI?= =?utf-8?q?o+9gNdhe8UWth2guGY/mVGct86En6pagChe+huuoabIIHL9sXq4GAKybqLoP6Sujz?= =?utf-8?q?EOxMODuSPRK+6A71U8Bqw0Kj0O9+6KlYcNCqjboqzYcMCyqgRXIvTArT4TvyxnA+G?= =?utf-8?q?5ni13ImPB3DUHFbcCZFDU+XGDKbOSnqq2Z2hjdJQcrQLP2jwMtmkjeruUCMeJr0cH?= =?utf-8?q?3yhqft/TUIS9wY3l6Lf9R/3oZq8SXA8uPRHLM1roF91Xbhp1ZfKhk2p7a3Rtle8C/?= =?utf-8?q?TAh/AVYP3FSyqOVaaPU+bXJvkSkDAOHJIIWo219ZN1EhOMlcUlHJk0qSC0G+DKzx2?= =?utf-8?q?6nUu8da33bTUn1JWU+Mfgwfxw5mWFHDGKq9xY0OSDUpz3gXKZUF5UHYF8Sad0XGgC?= =?utf-8?q?4H/h+K8YUQAeihFjuNuB2Ev/prJaFfY5/vot6y6DnAo2Vm5rEHB6UYfc9H6oDlhlA?= =?utf-8?q?5fn9jUsxAn0bctWn/y63uFwKgQeX/bJNkVSyUHaYNsMT4NW4PpoxUZlWGdpxN76JN?= =?utf-8?q?5MWk9hodGiPYT6zhG1nH5G4BMI5t4zYhWaBnXmB9pGL72RWTEs18g2FLT96rF7IwM?= =?utf-8?q?L60nqmP49Nua?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: d0e47f9d-840e-464a-5cba-08db8e747992 X-MS-Exchange-CrossTenant-AuthSource: DU2PR04MB8790.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jul 2023 07:38:31.1934 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: swjeH4W5T0YnuLfNMSpgIRAZZEoeAnVaSz4EnZ2iiuAz2brzEmkuQnZvDJCIVMWWVXrRcfdhquAaPDKU6TIz+Q== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DBBPR04MB8026 Its original introduction had two issues: For one the "common" part of the checks (carried out in the macro) was inverted. And then after removal from the radix tree the structure wasn't scheduled for freeing. (All structures still left in the radix tree would be freed upon domain destruction, though.) For the freeing to be safe even if it didn't use RCU (i.e. to avoid use- after-free), re-arrange checks/operations in evtchn_close(), such that the pointer wouldn't be used anymore after calling pirq_cleanup_check() (noting that unmap_domain_pirq_emuirq() itself calls the function in the success case). Fixes: c24536b636f2 ("replace d->nr_pirqs sized arrays with radix tree") Fixes: 79858fee307c ("xen: fix hvm_domain_use_pirq's behavior") Signed-off-by: Jan Beulich Reviewed-by: Roger Pau Monné --- This is fallout from me looking into whether the function and macro of the same name could be suitably split, to please Misra rule 5.5. The idea was apparently that the check done in the macro is the "common" part, and the actual function would be per-architecture. Pretty clearly this, if need be, could also be achieved by naming the actual function e.g. arch_pirq_cleanup_check(). Despite my testing of this (to a certain degree), I'm wary of the change, since the code has been the way it was for about 12 years. It feels like I'm overlooking something crucial ... The wrong check is also what explains why Arm got away without implementing the function (prior to "restrict concept of pIRQ to x86"): The compiler simply eliminated the two calls from event_channel.c. --- v3: New. --- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -1349,6 +1349,7 @@ void (pirq_cleanup_check)(struct pirq *p if ( radix_tree_delete(&d->pirq_tree, pirq->pirq) != pirq ) BUG(); + free_pirq_struct(pirq); } /* Flush all ready EOIs from the top of this CPU's pending-EOI stack. */ --- a/xen/common/event_channel.c +++ b/xen/common/event_channel.c @@ -711,9 +711,10 @@ int evtchn_close(struct domain *d1, int if ( !is_hvm_domain(d1) ) pirq_guest_unbind(d1, pirq); pirq->evtchn = 0; - pirq_cleanup_check(pirq, d1); - if ( is_hvm_domain(d1) && domain_pirq_to_irq(d1, pirq->pirq) > 0 ) - unmap_domain_pirq_emuirq(d1, pirq->pirq); + if ( !is_hvm_domain(d1) || + domain_pirq_to_irq(d1, pirq->pirq) <= 0 || + unmap_domain_pirq_emuirq(d1, pirq->pirq) < 0 ) + pirq_cleanup_check(pirq, d1); } unlink_pirq_port(chn1, d1->vcpu[chn1->notify_vcpu_id]); break; --- a/xen/include/xen/irq.h +++ b/xen/include/xen/irq.h @@ -158,7 +158,7 @@ extern struct pirq *pirq_get_info(struct void pirq_cleanup_check(struct pirq *, struct domain *); #define pirq_cleanup_check(pirq, d) \ - ((pirq)->evtchn ? pirq_cleanup_check(pirq, d) : (void)0) + (!(pirq)->evtchn ? pirq_cleanup_check(pirq, d) : (void)0) extern void pirq_guest_eoi(struct pirq *); extern void desc_guest_eoi(struct irq_desc *, struct pirq *);