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 X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2F382C433DF for ; Thu, 25 Jun 2020 19:58:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 07B4E2076E for ; Thu, 25 Jun 2020 19:58:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1593115104; bh=dyLST9MuMknNIoJ/bqqmhoGtr5xNyhepxCilwqcM8SA=; h=Date:From:To:Cc:Subject:In-Reply-To:List-ID:From; b=Ku5azNLdUcGkUrI/oDZawfN4yg6b5kx/rtENrZfTrbvUofMr+tRnKB57NW7+hkW6Q YLzmYBHyB8ZXx3N9Kdp5F9uAMn5YfdeX5M52YjivcsMPDifxrpL3Wnn6qjWX6FS9WV iEHmfB8xGzh4IACmpjjzBP1R2P56BYlIogA6hKFw= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2406936AbgFYT6X (ORCPT ); Thu, 25 Jun 2020 15:58:23 -0400 Received: from mail.kernel.org ([198.145.29.99]:45960 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2406569AbgFYT6X (ORCPT ); Thu, 25 Jun 2020 15:58:23 -0400 Received: from localhost (mobile-166-170-222-206.mycingular.net [166.170.222.206]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 5FDD7206A5; Thu, 25 Jun 2020 19:58:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1593115102; bh=dyLST9MuMknNIoJ/bqqmhoGtr5xNyhepxCilwqcM8SA=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=ci/khozqAkODT0b/ot6jPxeLDGfu4ewXRK7AQ5RfkDCNt9yCc1pYARcThxjciXm0c FwmoZc6DO1oSrMA/grTLbu24uAZGIkrxgw56+OKb2cBKbKc2t9TpVzpGfwGrwUtfLV ZR12ilC829DG03iSM7DJRQyMI8tYh/asmFzCNx2c= Date: Thu, 25 Jun 2020 14:58:20 -0500 From: Bjorn Helgaas To: Jon Derrick Cc: Lorenzo Pieralisi , linux-pci@vger.kernel.org, Sushma Kalakota , Andy Shevchenko , Thomas Gleixner Subject: Re: [PATCH] PCI: vmd: Keep fwnode allocated through VMD irqdomain life Message-ID: <20200625195820.GA2701690@bjorn-Precision-5520> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200625162450.5419-1-jonathan.derrick@intel.com> Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org [+cc Thomas] On Thu, Jun 25, 2020 at 12:24:49PM -0400, Jon Derrick wrote: > From: Andy Shevchenko > > The VMD domain does not subscribe to ACPI, and so does not operate on > it's irqdomain fwnode. It was freeing the handle after allocation of the > domain. As of 181e9d4efaf6a (irqdomain: Make __irq_domain_add() less > OF-dependent), the fwnode is put during irq_domain_remove causing a page > fault. This patch keeps VMD's fwnode allocated through the lifetime of > the VMD irqdomain. > > Fixes: ae904cafd59d ("PCI/vmd: Create named irq domain") > Signed-off-by: Andy Shevchenko > Co-developed-by: Andy Shevchenko > Signed-off-by: Jon Derrick > --- > Hi Lorenzo, Bjorn, > > Please take this patch for v5.8 fixes. It fixes an issue during VMD > unload. I tentatively applied this to for-linus for v5.8. But I would like to clarify the commit log. It says this fixes Thomas' ae904cafd59d ("PCI/vmd: Create named irq domain"). That appeared in v4.13, which suggests that this patch should be backported to v4.13 and later. But it's not clear to me that ae904cafd59d is actually broken, since the log also says the problem appeared after 181e9d4efaf6 ("irqdomain: Make __irq_domain_add() less OF-dependent"), which we just merged for v5.8-rc1. And obviously, freeing the fwnode doesn't *cause* a page fault. A use-after-free might cause a fault, but it's not clear where that happens. I guess fwnode is used in the interval between: vmd_enable_domain pci_msi_create_irq_domain ... <-- fwnode used here somewhere vmd_remove vmd_cleanup_srcu irq_domain_free_fwnode But I can't tell how 181e9d4efaf6a and/or ae904cafd59d are related to that. > drivers/pci/controller/vmd.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c > index e386d4eac407..ebec0a6e77ed 100644 > --- a/drivers/pci/controller/vmd.c > +++ b/drivers/pci/controller/vmd.c > @@ -546,9 +546,10 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) > > vmd->irq_domain = pci_msi_create_irq_domain(fn, &vmd_msi_domain_info, > x86_vector_domain); > - irq_domain_free_fwnode(fn); > - if (!vmd->irq_domain) > + if (!vmd->irq_domain) { > + irq_domain_free_fwnode(fn); > return -ENODEV; > + } > > pci_add_resource(&resources, &vmd->resources[0]); > pci_add_resource_offset(&resources, &vmd->resources[1], offset[0]); > @@ -559,6 +560,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) > if (!vmd->bus) { > pci_free_resource_list(&resources); > irq_domain_remove(vmd->irq_domain); > + irq_domain_free_fwnode(fn); > return -ENODEV; > } > > @@ -672,6 +674,7 @@ static void vmd_cleanup_srcu(struct vmd_dev *vmd) > static void vmd_remove(struct pci_dev *dev) > { > struct vmd_dev *vmd = pci_get_drvdata(dev); > + struct fwnode_handle *fn = vmd->irq_domain->fwnode; > > sysfs_remove_link(&vmd->dev->dev.kobj, "domain"); > pci_stop_root_bus(vmd->bus); > @@ -679,6 +682,7 @@ static void vmd_remove(struct pci_dev *dev) > vmd_cleanup_srcu(vmd); > vmd_detach_resources(vmd); > irq_domain_remove(vmd->irq_domain); > + irq_domain_free_fwnode(fn); > } > > #ifdef CONFIG_PM_SLEEP > -- > 2.18.1 >