From mboxrd@z Thu Jan 1 00:00:00 1970 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 007C3E8180C for ; Tue, 26 Sep 2023 02:22:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Message-ID:MIME-Version:References: In-Reply-To:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=3aPh7XmL0T7kUjXBIccfQ7oMspEDqo2jhvVbQOjwwlY=; b=4eNdL4CjM/Cb+R LQr19GYBqQkyUJZA3n5agx4kR9prWxJ19MImoBeVimChhVsErX4oFtAykZQn3E78IN0RN/Anj6Ypp LTcTptLJPjV+1xSkic2z68/NA2rzDgF9OnelcYqzHmRxrJ+ne0w+YiOWULfdfHC2cNKbB5zJRqFoR mXfvU6ncA0flkdg/fJt5u7VP/IAjhfEaL6mzEWavHPKfKKSB7o14I8NusUrcVhCFDz9NyBTE4OGrF Ii2mjLDNs3PZ4NC1ptqeXLHqIuWdUngZny9a0skB7g931Nzo+soSLdeVaQ7X+6hxUCqUl7yqZmKsI N9Hta6ad82qsJaKIbn5w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qkxhi-00FOgG-0t; Tue, 26 Sep 2023 02:21:38 +0000 Received: from mail.zju.edu.cn ([61.164.42.155] helo=zju.edu.cn) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qkxhf-00FOfk-0e for linux-arm-kernel@lists.infradead.org; Tue, 26 Sep 2023 02:21:37 +0000 Received: from dinghao.liu$zju.edu.cn ( [10.192.76.118] ) by ajax-webmail-mail-app3 (Coremail) ; Tue, 26 Sep 2023 10:21:13 +0800 (GMT+08:00) X-Originating-IP: [10.192.76.118] Date: Tue, 26 Sep 2023 10:21:13 +0800 (GMT+08:00) X-CM-HeaderCharset: UTF-8 From: dinghao.liu@zju.edu.cn To: "Marc Zyngier" Cc: "Toan Le" , "Lorenzo Pieralisi" , =?UTF-8?Q?Krzysztof_Wilczy=C5=84ski?= , "Rob Herring" , "Bjorn Helgaas" , "Duc Dang" , "Tanmay Inamdar" , linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] PCI: xgene-msi: Fix a potential UAF in xgene_msi_probe X-Priority: 3 X-Mailer: Coremail Webmail Server Version 2023.2-cmXT5 build 20230825(e13b6a3b) Copyright (c) 2002-2023 www.mailtech.cn mispb-4df6dc2c-e274-4d1c-b502-72c5c3dfa9ce-zj.edu.cn In-Reply-To: <87o7hqmvz3.wl-maz@kernel.org> References: <20230925062133.14170-1-dinghao.liu@zju.edu.cn> <87o7hqmvz3.wl-maz@kernel.org> MIME-Version: 1.0 Message-ID: <1c50637d.3e6d5.18acf4a638c.Coremail.dinghao.liu@zju.edu.cn> X-Coremail-Locale: zh_CN X-CM-TRANSID: cC_KCgBXWRwaQBJlmv3eAA--.28221W X-CM-SenderInfo: qrrzjiaqtzq6lmxovvfxof0/1tbiAgEJBmUQRiAzPQAGsX X-Coremail-Antispam: 1Ur529EdanIXcx71UUUUU7IcSsGvfJ3iIAIbVAYjsxI4VWxJw CS07vEb4IE77IF4wCS07vE1I0E4x80FVAKz4kxMIAIbVAFxVCaYxvI4VCIwcAKzIAtYxBI daVFxhVjvjDU= X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230925_192135_572499_90E08433 X-CRM114-Status: GOOD ( 17.14 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org > On Mon, 25 Sep 2023 07:21:32 +0100, > Dinghao Liu wrote: > > > > xgene_allocate_domains() will call irq_domain_remove() to free > > msi->inner_domain on failure. However, its caller, xgene_msi_probe(), > > will also call irq_domain_remove() through xgene_msi_remove() on the > > same failure, which may lead to a use-after-free. Set the freed pointer > > to NULL to fix this issue. > > > > Fixes: dcd19de36775 ("PCI: xgene: Add APM X-Gene v1 PCIe MSI/MSIX termination driver") > > Signed-off-by: Dinghao Liu > > --- > > drivers/pci/controller/pci-xgene-msi.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/pci/controller/pci-xgene-msi.c b/drivers/pci/controller/pci-xgene-msi.c > > index 3ce38dfd0d29..c0192c5ff0f3 100644 > > --- a/drivers/pci/controller/pci-xgene-msi.c > > +++ b/drivers/pci/controller/pci-xgene-msi.c > > @@ -253,6 +253,7 @@ static int xgene_allocate_domains(struct xgene_msi *msi) > > > > if (!msi->msi_domain) { > > irq_domain_remove(msi->inner_domain); > > + msi->inner_domain = NULL; > > return -ENOMEM; > > } > > Why can't we just drop the irq_domain_remove() call here instead, and > simply rely on xgene_msi_remove() to do the right thing? Something > like the untested patch below. > > Thanks, > > M. > > diff --git a/drivers/pci/controller/pci-xgene-msi.c b/drivers/pci/controller/pci-xgene-msi.c > index 0234e528b9a5..f98c9eb7bebf 100644 > --- a/drivers/pci/controller/pci-xgene-msi.c > +++ b/drivers/pci/controller/pci-xgene-msi.c > @@ -251,10 +251,8 @@ static int xgene_allocate_domains(struct xgene_msi *msi) > &xgene_msi_domain_info, > msi->inner_domain); > > - if (!msi->msi_domain) { > - irq_domain_remove(msi->inner_domain); > + if (!msi->msi_domain) > return -ENOMEM; > - } > > return 0; > } Thanks for your advice! This patch is more concise. I will resend a new patch soon. Regards, Dinghao _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel