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=-6.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 E51C4C2D0A3 for ; Wed, 4 Nov 2020 20:12:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7520620782 for ; Wed, 4 Nov 2020 20:12:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=intel-com.20150623.gappssmtp.com header.i=@intel-com.20150623.gappssmtp.com header.b="DLnfeD9J" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731012AbgKDUM3 (ORCPT ); Wed, 4 Nov 2020 15:12:29 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43154 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727245AbgKDUM2 (ORCPT ); Wed, 4 Nov 2020 15:12:28 -0500 Received: from mail-qt1-x841.google.com (mail-qt1-x841.google.com [IPv6:2607:f8b0:4864:20::841]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7F53FC0613D4 for ; Wed, 4 Nov 2020 12:12:28 -0800 (PST) Received: by mail-qt1-x841.google.com with SMTP id n63so7597635qte.4 for ; Wed, 04 Nov 2020 12:12:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=rEEEXt1os4uvlr5FpRnmuBcc7L7msKpqjcsUXwjQF/c=; b=DLnfeD9J1ORdQ1bfs5+3AntXSfjpW/03OlgMKpfMczcb0Yo31gAQmfKmU/MxwQ17Tz p8ZagfXHFsC1dHS6FW5fSkhChMKNOPBoZuaf0P1ssilDHMYkmfYcvMK/N4bu0vQXK3wy c2tGk1nId5n4tetbAX3T+IoXKixnSiRJU2P+RgF0i9/MzM9vLJQC4pR4SHkQIDhJjOCm nI9uYdoL8DUDIOHzXzueS+XUGj/Xv/0E4OLEq6oqPKUHmkx/sfooqnKn5e06EiNpsCvu pIeS4Ja9kfYHVEpTmDFGZ6rRJqSaQZ+CUMbWPT9DrPeXAPhB8c08+nN3zrd9Xy424Dsr vK5Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=rEEEXt1os4uvlr5FpRnmuBcc7L7msKpqjcsUXwjQF/c=; b=VJYcjPkwPYdP+oXbyTO6IypFTVY1QA2Hs1wLvsJ9OL4w8/aGHtAsXVAufr7B6zkyEJ 4KvLgnuRayMfDwRolMO3579YZ+Ivh8VQP60StsVn2Y1HTKAJNl+SDMaByrY5tKebdeZK z7GrncyX5DgkbIOL+1XQQybxVgQVqH/1ZsGRMR2g90Gom9LYbsRD6ZPEPpveh6tV5d2M R0c4L0Ur1p4SSt932jdc7lku/UYosjlLTDwkh1k9+GT6WioDDZl9RiQPqlNDfE1xnW+N DppiObV9XxAESgNyRsi4d/4pUzwR2DYVud6kTrTxFoulTFqY01LdtcBgXj4xEwxQyvVI CeHg== X-Gm-Message-State: AOAM531iVJcubd6MtoRpB40FQKWJypqVtFCUS8NVUEdA56ydBXPD08IB cjWf4SF+UbzVPKZ3Zio12eLU+IvAFyAebZnNctExYA== X-Google-Smtp-Source: ABdhPJyImQ0VMkgJ13gDdAbq5pEtNIjA5jffxfB/P+jRlJioL0jIF2z8jDtJ1zF31+FQiHcdLBXSWIzDRip6n19HrNg= X-Received: by 2002:ac8:4b79:: with SMTP id g25mr21823130qts.19.1604520747497; Wed, 04 Nov 2020 12:12:27 -0800 (PST) MIME-Version: 1.0 References: <20201104165017.GA352206@bjorn-Precision-5520> In-Reply-To: <20201104165017.GA352206@bjorn-Precision-5520> From: Dan Williams Date: Wed, 4 Nov 2020 12:12:15 -0800 Message-ID: Subject: Re: [PATCH v5 11/15] PCI: Obey iomem restrictions for procfs mmap To: Bjorn Helgaas Cc: Daniel Vetter , DRI Development , LKML , KVM list , Linux MM , Linux ARM , linux-samsung-soc , "Linux-media@vger.kernel.org" , Daniel Vetter , Jason Gunthorpe , Kees Cook , Andrew Morton , John Hubbard , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , Jan Kara , Bjorn Helgaas , Linux PCI Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 4, 2020 at 8:50 AM Bjorn Helgaas wrote: > > On Wed, Nov 04, 2020 at 09:44:04AM +0100, Daniel Vetter wrote: > > On Tue, Nov 3, 2020 at 11:09 PM Dan Williams wrote: > > > On Tue, Nov 3, 2020 at 1:28 PM Bjorn Helgaas wrote: > > > > On Fri, Oct 30, 2020 at 11:08:11AM +0100, Daniel Vetter wrote: > > > > > There's three ways to access PCI BARs from userspace: /dev/mem, sysfs > > > > > files, and the old proc interface. Two check against > > > > > iomem_is_exclusive, proc never did. And with CONFIG_IO_STRICT_DEVMEM, > > > > > this starts to matter, since we don't want random userspace having > > > > > access to PCI BARs while a driver is loaded and using it. > > > > > > > > > > Fix this by adding the same iomem_is_exclusive() check we already have > > > > > on the sysfs side in pci_mmap_resource(). > > > > > > > > > > References: 90a545e98126 ("restrict /dev/mem to idle io memory ranges") > > > > > Signed-off-by: Daniel Vetter > > > > > > > > This is OK with me but it looks like IORESOURCE_EXCLUSIVE is currently > > > > only used in a few places: > > > > > > > > e1000_probe() calls pci_request_selected_regions_exclusive(), > > > > ne_pci_probe() calls pci_request_regions_exclusive(), > > > > vmbus_allocate_mmio() calls request_mem_region_exclusive() > > > > > > > > which raises the question of whether it's worth keeping > > > > IORESOURCE_EXCLUSIVE at all. I'm totally fine with removing it > > > > completely. > > > > > > Now that CONFIG_IO_STRICT_DEVMEM upgrades IORESOURCE_BUSY to > > > IORESOURCE_EXCLUSIVE semantics the latter has lost its meaning so I'd > > > be in favor of removing it as well. > > > > Still has some value since it enforces exclusive access even if the > > config isn't enabled, and iirc e1000 had some fun with userspace tools > > clobbering the firmware and bricking the chip. > > There's *some* value; I'm just skeptical since only three drivers use > it. > > IORESOURCE_EXCLUSIVE is from e8de1481fd71 ("resource: allow MMIO > exclusivity for device drivers"), and the commit message says this is > only active when CONFIG_STRICT_DEVMEM is set. I didn't check to see > whether that's still true. > > That commit adds a bunch of wrappers and "__"-prefixed functions to > pass the IORESOURCE_EXCLUSIVE flag around. That's a fair bit of > uglification for three drivers. > > > Another thing I kinda wondered, since pci maintainer is here: At least > > in drivers/gpu I see very few drivers explicitly requestion regions > > (this might be a historical artifact due to the shadow attach stuff > > before we had real modesetting drivers). And pci core doesn't do that > > either, even when a driver is bound. Is this intentional, or > > should/could we do better? Since drivers work happily without > > reserving regions I don't think "the drivers need to remember to do > > this" will ever really work out well. > > You're right, many drivers don't call pci_request_regions(). Maybe we > could do better, but I haven't looked into that recently. There is a > related note in Documentation/PCI/pci.rst that's been there for a long > time (it refers to "pci_request_resources()", which has never existed > AFAICT). I'm certainly open to proposals. It seems a bug that the kernel permits MMIO regions with side effects to be ioremap()'ed without request_mem_region() on the resource. I wonder how much log spam would happen if ioremap() reported whenever a non-IORESOURE_BUSY range was passed to it? The current state of affairs to trust *remap users to have claimed their remap target seems too ingrained to unwind now. 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=-6.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 990BBC00A89 for ; Thu, 5 Nov 2020 03:47:11 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E2EB82072E for ; Thu, 5 Nov 2020 03:47:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="tuBjh8fH"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="SX+TvCrh"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=intel-com.20150623.gappssmtp.com header.i=@intel-com.20150623.gappssmtp.com header.b="DLnfeD9J" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E2EB82072E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=qlfYYTKnlFQd7HY6LyTFrTuDubQ2U+EWA72AD5DEG0A=; b=tuBjh8fHdz24Zj61LX7FGTBZ4 HeDjSqcpAp+FV3udbTUtu9CVgNqd3K9w80PoOzZRg4AnzdAYdnNMsRcZvTErO1UCL/a/YwzsCJ8ay t70p2d659Q1rnhmgwEbt53mFLw4QiD76FKIXbWRibLw+FGD5aTAhgXKlX3JIQaVuWVE1iH6+vZ+V5 Uaebi7Bt7DqdA7/u+FNvsLiCQOdidfUeyZvR//Y8cTvDPP0io4wtDx2wbTfVJMoMCELTAUZykndX7 WZa+V0AkOP041sRYCDh7TxJX2Rd/sSagD+9OO7G2Rw+CCfr0yF/KBKxH0FYR8O6GouV+3r6OYDRW7 A/JUNPZFQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kaWCQ-0004ot-ED; Thu, 05 Nov 2020 03:44:34 +0000 Received: from casper.infradead.org ([2001:8b0:10b:1236::1]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kaW9s-0002bw-0m for linux-arm-kernel@merlin.infradead.org; Thu, 05 Nov 2020 03:41:56 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=Content-Type:Cc:To:Subject:Message-ID: Date:From:In-Reply-To:References:MIME-Version:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=rEEEXt1os4uvlr5FpRnmuBcc7L7msKpqjcsUXwjQF/c=; b=SX+TvCrheASLs18LNt5QIcPdO0 SgejSn6eu35pizrpjwMFXTtkZNx0/Ix4Tqpjs86KIkKjaZAGgL2k6n00G/ozRE3cl7qhpPpCD7aWk drFtJf52RAMs6zItzpQe4ex8+ywLMWB2J9QwLVjZJ1GGDqbYMVRMkuypd2wUZkIV4lDIXzdKJw9kc rpi7LozTIbNHSyrpBzPzSl2ezIY/4AwKxjt4HBxK+2R9SUH7/uODRI/3gB61lMYVJo975TihWbWcV Zx9JTfR5THtXLYQvbC+/nMd60+gcS6Z0Dze3dvlMO263+Yc3sn9warpGqs/j1uHgGjLilslRIjxVc XjWPtVUQ==; Received: from mail-qt1-x844.google.com ([2607:f8b0:4864:20::844]) by casper.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kaPAR-0003dR-JD for linux-arm-kernel@lists.infradead.org; Wed, 04 Nov 2020 20:14:06 +0000 Received: by mail-qt1-x844.google.com with SMTP id i7so13131756qti.6 for ; Wed, 04 Nov 2020 12:13:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=rEEEXt1os4uvlr5FpRnmuBcc7L7msKpqjcsUXwjQF/c=; b=DLnfeD9J1ORdQ1bfs5+3AntXSfjpW/03OlgMKpfMczcb0Yo31gAQmfKmU/MxwQ17Tz p8ZagfXHFsC1dHS6FW5fSkhChMKNOPBoZuaf0P1ssilDHMYkmfYcvMK/N4bu0vQXK3wy c2tGk1nId5n4tetbAX3T+IoXKixnSiRJU2P+RgF0i9/MzM9vLJQC4pR4SHkQIDhJjOCm nI9uYdoL8DUDIOHzXzueS+XUGj/Xv/0E4OLEq6oqPKUHmkx/sfooqnKn5e06EiNpsCvu pIeS4Ja9kfYHVEpTmDFGZ6rRJqSaQZ+CUMbWPT9DrPeXAPhB8c08+nN3zrd9Xy424Dsr vK5Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=rEEEXt1os4uvlr5FpRnmuBcc7L7msKpqjcsUXwjQF/c=; b=kTVrfEWUNQxjOpfmP1tAV/QmQt0+VI3Ay3xb2cdJpj60Y/TKJ0oDFWu7rD6hWDYqBJ bQ8YzlgW0JccB2Sd+SxVzT98rrprmPJi1nRSf5nz+GjJg7ojQNbWx6Vh7zvzarC4HB7m Yc8fJz+UwSeJbTds3vn/kqIPCV8AwUQBpU+AQYMnQIkk4o0XdHohz45dw+RlYbjLYl4c wu0+36LOo8Q883Fqee5qcW3W4zJa3AkgZ+mrmw3mQW4LTDXRzW8ajHVrvkWcy0Xqy3py OhurmPTOENP7ydPMAJmx7rfnUYhDlzgkxK3cRu3SUcp513AJ8d82Mcq68wmlJGkeoWkp 7Ubg== X-Gm-Message-State: AOAM533jslDYuvm9beUUaOWGMDnE0UkV6ugQY9354KAn1tajJT0kOyNA FANxqoqGYopFdAQiET+XiEhEY/Hh/N0a1a3D4/5dYg== X-Google-Smtp-Source: ABdhPJyImQ0VMkgJ13gDdAbq5pEtNIjA5jffxfB/P+jRlJioL0jIF2z8jDtJ1zF31+FQiHcdLBXSWIzDRip6n19HrNg= X-Received: by 2002:ac8:4b79:: with SMTP id g25mr21823130qts.19.1604520747497; Wed, 04 Nov 2020 12:12:27 -0800 (PST) MIME-Version: 1.0 References: <20201104165017.GA352206@bjorn-Precision-5520> In-Reply-To: <20201104165017.GA352206@bjorn-Precision-5520> From: Dan Williams Date: Wed, 4 Nov 2020 12:12:15 -0800 Message-ID: Subject: Re: [PATCH v5 11/15] PCI: Obey iomem restrictions for procfs mmap To: Bjorn Helgaas X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201104_201403_847085_8A14AE68 X-CRM114-Status: GOOD ( 40.75 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-samsung-soc , Jan Kara , Kees Cook , KVM list , Jason Gunthorpe , Daniel Vetter , Linux PCI , LKML , DRI Development , Linux MM , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , John Hubbard , Bjorn Helgaas , Daniel Vetter , Andrew Morton , Linux ARM , "Linux-media@vger.kernel.org" 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 Wed, Nov 4, 2020 at 8:50 AM Bjorn Helgaas wrote: > > On Wed, Nov 04, 2020 at 09:44:04AM +0100, Daniel Vetter wrote: > > On Tue, Nov 3, 2020 at 11:09 PM Dan Williams wrote: > > > On Tue, Nov 3, 2020 at 1:28 PM Bjorn Helgaas wrote: > > > > On Fri, Oct 30, 2020 at 11:08:11AM +0100, Daniel Vetter wrote: > > > > > There's three ways to access PCI BARs from userspace: /dev/mem, sysfs > > > > > files, and the old proc interface. Two check against > > > > > iomem_is_exclusive, proc never did. And with CONFIG_IO_STRICT_DEVMEM, > > > > > this starts to matter, since we don't want random userspace having > > > > > access to PCI BARs while a driver is loaded and using it. > > > > > > > > > > Fix this by adding the same iomem_is_exclusive() check we already have > > > > > on the sysfs side in pci_mmap_resource(). > > > > > > > > > > References: 90a545e98126 ("restrict /dev/mem to idle io memory ranges") > > > > > Signed-off-by: Daniel Vetter > > > > > > > > This is OK with me but it looks like IORESOURCE_EXCLUSIVE is currently > > > > only used in a few places: > > > > > > > > e1000_probe() calls pci_request_selected_regions_exclusive(), > > > > ne_pci_probe() calls pci_request_regions_exclusive(), > > > > vmbus_allocate_mmio() calls request_mem_region_exclusive() > > > > > > > > which raises the question of whether it's worth keeping > > > > IORESOURCE_EXCLUSIVE at all. I'm totally fine with removing it > > > > completely. > > > > > > Now that CONFIG_IO_STRICT_DEVMEM upgrades IORESOURCE_BUSY to > > > IORESOURCE_EXCLUSIVE semantics the latter has lost its meaning so I'd > > > be in favor of removing it as well. > > > > Still has some value since it enforces exclusive access even if the > > config isn't enabled, and iirc e1000 had some fun with userspace tools > > clobbering the firmware and bricking the chip. > > There's *some* value; I'm just skeptical since only three drivers use > it. > > IORESOURCE_EXCLUSIVE is from e8de1481fd71 ("resource: allow MMIO > exclusivity for device drivers"), and the commit message says this is > only active when CONFIG_STRICT_DEVMEM is set. I didn't check to see > whether that's still true. > > That commit adds a bunch of wrappers and "__"-prefixed functions to > pass the IORESOURCE_EXCLUSIVE flag around. That's a fair bit of > uglification for three drivers. > > > Another thing I kinda wondered, since pci maintainer is here: At least > > in drivers/gpu I see very few drivers explicitly requestion regions > > (this might be a historical artifact due to the shadow attach stuff > > before we had real modesetting drivers). And pci core doesn't do that > > either, even when a driver is bound. Is this intentional, or > > should/could we do better? Since drivers work happily without > > reserving regions I don't think "the drivers need to remember to do > > this" will ever really work out well. > > You're right, many drivers don't call pci_request_regions(). Maybe we > could do better, but I haven't looked into that recently. There is a > related note in Documentation/PCI/pci.rst that's been there for a long > time (it refers to "pci_request_resources()", which has never existed > AFAICT). I'm certainly open to proposals. It seems a bug that the kernel permits MMIO regions with side effects to be ioremap()'ed without request_mem_region() on the resource. I wonder how much log spam would happen if ioremap() reported whenever a non-IORESOURE_BUSY range was passed to it? The current state of affairs to trust *remap users to have claimed their remap target seems too ingrained to unwind now. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel 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=-6.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 57FB5C4742C for ; Wed, 4 Nov 2020 20:12:31 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id BC17E2087D for ; Wed, 4 Nov 2020 20:12:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=intel-com.20150623.gappssmtp.com header.i=@intel-com.20150623.gappssmtp.com header.b="DLnfeD9J" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BC17E2087D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4E2E96E220; Wed, 4 Nov 2020 20:12:29 +0000 (UTC) Received: from mail-qt1-x844.google.com (mail-qt1-x844.google.com [IPv6:2607:f8b0:4864:20::844]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8837E6E220 for ; Wed, 4 Nov 2020 20:12:28 +0000 (UTC) Received: by mail-qt1-x844.google.com with SMTP id f93so13111680qtb.10 for ; Wed, 04 Nov 2020 12:12:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=rEEEXt1os4uvlr5FpRnmuBcc7L7msKpqjcsUXwjQF/c=; b=DLnfeD9J1ORdQ1bfs5+3AntXSfjpW/03OlgMKpfMczcb0Yo31gAQmfKmU/MxwQ17Tz p8ZagfXHFsC1dHS6FW5fSkhChMKNOPBoZuaf0P1ssilDHMYkmfYcvMK/N4bu0vQXK3wy c2tGk1nId5n4tetbAX3T+IoXKixnSiRJU2P+RgF0i9/MzM9vLJQC4pR4SHkQIDhJjOCm nI9uYdoL8DUDIOHzXzueS+XUGj/Xv/0E4OLEq6oqPKUHmkx/sfooqnKn5e06EiNpsCvu pIeS4Ja9kfYHVEpTmDFGZ6rRJqSaQZ+CUMbWPT9DrPeXAPhB8c08+nN3zrd9Xy424Dsr vK5Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=rEEEXt1os4uvlr5FpRnmuBcc7L7msKpqjcsUXwjQF/c=; b=fMoXTwNfjezFR2jmdEsHXZtlK9LKMh2gstpnAlS08sSi6dwlahULar117VSMtsTrVE 78gwu6A4KtNhEHca9xGHoBQM/MYyJw7l9hyqUxgi4HIw1ZLn/Dy/M6wGYrV9lJglnwJN U8OpQNMjwK+DFZdwFGpP4Fa86K2Gxk6Mrr9NLqnKPV7fgPD2t+W9oo+dNsXXJrEbWJKU QiSJzTjG4whuIHEb4eZE4kwP7Jp52y/lwD/0GfGN70VWdJ0meLyVLxOh4wlFtdPkSiOo dN6euGu7xh1MmqT5AOCwi1w2vjt+5eNk9dyDesOiqsEqXXpl4myy/r1yuQBBb093MEP9 JuFw== X-Gm-Message-State: AOAM530/4srAS0RF+PJFtcaMUQts1LsdgnCYN1t0f4v5nydLwC2O3btP 7Z4nd4GuCZHRjpiGbiiZ1Z6y00xFg4DhAhnzczjdVA== X-Google-Smtp-Source: ABdhPJyImQ0VMkgJ13gDdAbq5pEtNIjA5jffxfB/P+jRlJioL0jIF2z8jDtJ1zF31+FQiHcdLBXSWIzDRip6n19HrNg= X-Received: by 2002:ac8:4b79:: with SMTP id g25mr21823130qts.19.1604520747497; Wed, 04 Nov 2020 12:12:27 -0800 (PST) MIME-Version: 1.0 References: <20201104165017.GA352206@bjorn-Precision-5520> In-Reply-To: <20201104165017.GA352206@bjorn-Precision-5520> From: Dan Williams Date: Wed, 4 Nov 2020 12:12:15 -0800 Message-ID: Subject: Re: [PATCH v5 11/15] PCI: Obey iomem restrictions for procfs mmap To: Bjorn Helgaas X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-samsung-soc , Jan Kara , Kees Cook , KVM list , Jason Gunthorpe , Daniel Vetter , Linux PCI , LKML , DRI Development , Linux MM , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , John Hubbard , Bjorn Helgaas , Daniel Vetter , Andrew Morton , Linux ARM , "Linux-media@vger.kernel.org" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Wed, Nov 4, 2020 at 8:50 AM Bjorn Helgaas wrote: > > On Wed, Nov 04, 2020 at 09:44:04AM +0100, Daniel Vetter wrote: > > On Tue, Nov 3, 2020 at 11:09 PM Dan Williams wrote: > > > On Tue, Nov 3, 2020 at 1:28 PM Bjorn Helgaas wrote: > > > > On Fri, Oct 30, 2020 at 11:08:11AM +0100, Daniel Vetter wrote: > > > > > There's three ways to access PCI BARs from userspace: /dev/mem, sysfs > > > > > files, and the old proc interface. Two check against > > > > > iomem_is_exclusive, proc never did. And with CONFIG_IO_STRICT_DEVMEM, > > > > > this starts to matter, since we don't want random userspace having > > > > > access to PCI BARs while a driver is loaded and using it. > > > > > > > > > > Fix this by adding the same iomem_is_exclusive() check we already have > > > > > on the sysfs side in pci_mmap_resource(). > > > > > > > > > > References: 90a545e98126 ("restrict /dev/mem to idle io memory ranges") > > > > > Signed-off-by: Daniel Vetter > > > > > > > > This is OK with me but it looks like IORESOURCE_EXCLUSIVE is currently > > > > only used in a few places: > > > > > > > > e1000_probe() calls pci_request_selected_regions_exclusive(), > > > > ne_pci_probe() calls pci_request_regions_exclusive(), > > > > vmbus_allocate_mmio() calls request_mem_region_exclusive() > > > > > > > > which raises the question of whether it's worth keeping > > > > IORESOURCE_EXCLUSIVE at all. I'm totally fine with removing it > > > > completely. > > > > > > Now that CONFIG_IO_STRICT_DEVMEM upgrades IORESOURCE_BUSY to > > > IORESOURCE_EXCLUSIVE semantics the latter has lost its meaning so I'd > > > be in favor of removing it as well. > > > > Still has some value since it enforces exclusive access even if the > > config isn't enabled, and iirc e1000 had some fun with userspace tools > > clobbering the firmware and bricking the chip. > > There's *some* value; I'm just skeptical since only three drivers use > it. > > IORESOURCE_EXCLUSIVE is from e8de1481fd71 ("resource: allow MMIO > exclusivity for device drivers"), and the commit message says this is > only active when CONFIG_STRICT_DEVMEM is set. I didn't check to see > whether that's still true. > > That commit adds a bunch of wrappers and "__"-prefixed functions to > pass the IORESOURCE_EXCLUSIVE flag around. That's a fair bit of > uglification for three drivers. > > > Another thing I kinda wondered, since pci maintainer is here: At least > > in drivers/gpu I see very few drivers explicitly requestion regions > > (this might be a historical artifact due to the shadow attach stuff > > before we had real modesetting drivers). And pci core doesn't do that > > either, even when a driver is bound. Is this intentional, or > > should/could we do better? Since drivers work happily without > > reserving regions I don't think "the drivers need to remember to do > > this" will ever really work out well. > > You're right, many drivers don't call pci_request_regions(). Maybe we > could do better, but I haven't looked into that recently. There is a > related note in Documentation/PCI/pci.rst that's been there for a long > time (it refers to "pci_request_resources()", which has never existed > AFAICT). I'm certainly open to proposals. It seems a bug that the kernel permits MMIO regions with side effects to be ioremap()'ed without request_mem_region() on the resource. I wonder how much log spam would happen if ioremap() reported whenever a non-IORESOURE_BUSY range was passed to it? The current state of affairs to trust *remap users to have claimed their remap target seems too ingrained to unwind now. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel