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=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_RED 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 1EF70C433E0 for ; Sat, 13 Mar 2021 22:36:21 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 48BE664ED7 for ; Sat, 13 Mar 2021 22:36:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 48BE664ED7 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B42856B006E; Sat, 13 Mar 2021 17:36:19 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B056B6B0070; Sat, 13 Mar 2021 17:36:19 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9A6C86B0071; Sat, 13 Mar 2021 17:36:19 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0245.hostedemail.com [216.40.44.245]) by kanga.kvack.org (Postfix) with ESMTP id 821C36B006E for ; Sat, 13 Mar 2021 17:36:19 -0500 (EST) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 33EE0181AEF3E for ; Sat, 13 Mar 2021 22:36:19 +0000 (UTC) X-FDA: 77916310878.03.C044D9A Received: from mail-ot1-f54.google.com (mail-ot1-f54.google.com [209.85.210.54]) by imf24.hostedemail.com (Postfix) with ESMTP id 4DCF0A0009DE for ; Sat, 13 Mar 2021 22:36:18 +0000 (UTC) Received: by mail-ot1-f54.google.com with SMTP id l23-20020a05683004b7b02901b529d1a2fdso3308208otd.8 for ; Sat, 13 Mar 2021 14:36:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=v2nM0F1OSZDnsayP8DJnmj8gvTm944LA45fG5RJlAR4=; b=esIzXz11IK5/xkUIRZWExsywCZP7c0K0xyTCruovMIwohs3QhI9Dkv62TxALgb+ukr OyArZWGPm9Na48g4ZMujf+yBV2K2krqMot8Wn9X6IGICiP7RoX5zhMiSUrOmCFsdNwIN o31ZMOU5Rpeiw9vVUzQ5TG2qfv3gkm1mXP+iU= 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:content-transfer-encoding; bh=v2nM0F1OSZDnsayP8DJnmj8gvTm944LA45fG5RJlAR4=; b=Ihsk1+18xm7iq+yJMK+yoT0WdoyKH172QnDKsy4uNqyYRH4KfaKg5SsNxhqU25iFM9 wvciLILAsTABBGCOAm5qkx8ZQkrwPuFb+bD1rnlIWaBHrGJoXEj9jft7aGzsktLxlFSR GJ8IZVegqZHmNS9wH6IPHvqsgDxwRsl5TMDgzCJZS5kfdbPXDgMGqpprNdDv7qi95Uja SG1J4rZalwLpvu2vWSOTwHHm3dVhhnO6xLs36GFMEtoYyjB28T9bd2Wzo9IZLORtRKtW cZSr/iz758OhcX/RtwofncF5xoouTbsgEXUQwPTynyLU3rdLGFAtu3S0/a0RVUhKaLxK FzjQ== X-Gm-Message-State: AOAM531fa6VOGfMU6wG+C79hDbB7EFRvKICLcjG3T4eTTagGTyc+7Py5 /H3guUx8wNz77p1OPk7c1NE89XqXitENqkaRXgXOjQ== X-Google-Smtp-Source: ABdhPJx+9a30YontWgcNy6nzPQiRyG8JBRoZfcUjE9SPwclc5y9YC+uZVvc9WajnnQVETEW26MKNrTck27mSKqgVcXY= X-Received: by 2002:a9d:7b4e:: with SMTP id f14mr8827539oto.281.1615674977323; Sat, 13 Mar 2021 14:36:17 -0800 (PST) MIME-Version: 1.0 References: <20210204165831.2703772-3-daniel.vetter@ffwll.ch> <20210313215747.GA2394467@bjorn-Precision-5520> In-Reply-To: <20210313215747.GA2394467@bjorn-Precision-5520> From: Daniel Vetter Date: Sat, 13 Mar 2021 23:36:06 +0100 Message-ID: Subject: Re: [PATCH 2/2] PCI: Revoke mappings like devmem To: Bjorn Helgaas Cc: LKML , Stephen Rothwell , linux-samsung-soc , Jan Kara , Kees Cook , Greg Kroah-Hartman , Linux PCI , DRI Development , Linux MM , Jason Gunthorpe , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , John Hubbard , Bjorn Helgaas , Daniel Vetter , Dan Williams , Andrew Morton , Linux ARM , "open list:DMA BUFFER SHARING FRAMEWORK" , =?UTF-8?Q?Krzysztof_Wilczy=C5=84ski?= , =?UTF-8?Q?Pali_Roh=C3=A1r?= , "Oliver O'Halloran" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: y8snrice61kiuh7hed41fodtdfxzwq7i X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 4DCF0A0009DE Received-SPF: none (ffwll.ch>: No applicable sender policy available) receiver=imf24; identity=mailfrom; envelope-from=""; helo=mail-ot1-f54.google.com; client-ip=209.85.210.54 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1615674978-185941 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sat, Mar 13, 2021 at 10:57 PM Bjorn Helgaas wrote: > > [+cc Krzysztof, Pali, Oliver] > > On Thu, Feb 04, 2021 at 05:58:31PM +0100, Daniel Vetter wrote: > > Since 3234ac664a87 ("/dev/mem: Revoke mappings when a driver claims > > the region") /dev/kmem zaps ptes when the kernel requests exclusive > > acccess to an iomem region. And with CONFIG_IO_STRICT_DEVMEM, this is > > the default for all driver uses. > > > > Except there's two more ways to access PCI BARs: sysfs and proc mmap > > support. Let's plug that hole. > > IIUC, the idea is that if a driver calls request_mem_region() on a PCI > BAR, we prevent access to the BAR via sysfs. I guess I'm OK with that > if it's a real security improvement or something. Yup. > But the downside of this implementation is that it depends on > iomem_get_mapping(), which doesn't work until after fs_initcalls, > which means the sysfs files cannot be static attributes of devices > added before that. PCI devices are typically enumerated in > subsys_initcall. > > Krzysztof is converting PCI sysfs files (config, rom, reset, vpd, etc) > to static attributes. This is a major improvement that could get rid > of pci_create_sysfs_dev_files(), the late_initcall pci_sysfs_init(), > and the "sysfs_initialized" hack. This would fix a race reported by > Pali [1] (thanks to Oliver for the idea [2]). > > EXCEPT that this revoke change means the "resource%d", "legacy_io", > and "legacy_mem" files cannot be static attributes because of > iomem_get_mapping(). > > Any ideas on how to deal with this? Having to keep the > pci_sysfs_init() initcall just for these few files seems like the tail > wagging the dog. It's a bit "pick your ugly". Either we have the late init call (not pretty), or the sysfs side needs a callback to fish out the address_space for the mmap at open() time, which didn't stir up much enthusiams with Greg because we need a new callback just for these mmio files. Either approach works. -Daniel > [1] https://lore.kernel.org/r/20200716110423.xtfyb3n6tn5ixedh@pali > [2] https://lore.kernel.org/r/CAOSf1CHss03DBSDO4PmTtMp0tCEu5kScn704ZEwLKG= XQzBfqaA@mail.gmail.com > > > For revoke_devmem() to work we need to link our vma into the same > > address_space, with consistent vma->vm_pgoff. ->pgoff is already > > adjusted, because that's how (io_)remap_pfn_range works, but for the > > mapping we need to adjust vma->vm_file->f_mapping. The cleanest way is > > to adjust this at at ->open time: > > > > - for sysfs this is easy, now that binary attributes support this. We > > just set bin_attr->mapping when mmap is supported > > - for procfs it's a bit more tricky, since procfs pci access has only > > one file per device, and access to a specific resources first needs > > to be set up with some ioctl calls. But mmap is only supported for > > the same resources as sysfs exposes with mmap support, and otherwise > > rejected, so we can set the mapping unconditionally at open time > > without harm. > > > > A special consideration is for arch_can_pci_mmap_io() - we need to > > make sure that the ->f_mapping doesn't alias between ioport and iomem > > space. There's only 2 ways in-tree to support mmap of ioports: generic > > pci mmap (ARCH_GENERIC_PCI_MMAP_RESOURCE), and sparc as the single > > architecture hand-rolling. Both approach support ioport mmap through a > > special pfn range and not through magic pte attributes. Aliasing is > > therefore not a problem. > > > > The only difference in access checks left is that sysfs PCI mmap does > > not check for CAP_RAWIO. I'm not really sure whether that should be > > added or not. > > > > Acked-by: Bjorn Helgaas > > Reviewed-by: Dan Williams > > Signed-off-by: Daniel Vetter > > Cc: Stephen Rothwell > > Cc: Jason Gunthorpe > > Cc: Kees Cook > > Cc: Dan Williams > > Cc: Andrew Morton > > Cc: John Hubbard > > Cc: J=C3=A9r=C3=B4me Glisse > > Cc: Jan Kara > > Cc: Dan Williams > > Cc: Greg Kroah-Hartman > > Cc: linux-mm@kvack.org > > Cc: linux-arm-kernel@lists.infradead.org > > Cc: linux-samsung-soc@vger.kernel.org > > Cc: linux-media@vger.kernel.org > > Cc: Bjorn Helgaas > > Cc: linux-pci@vger.kernel.org > > --- > > drivers/pci/pci-sysfs.c | 4 ++++ > > drivers/pci/proc.c | 1 + > > 2 files changed, 5 insertions(+) > > > > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c > > index 0c45b4f7b214..f8afd54ca3e1 100644 > > --- a/drivers/pci/pci-sysfs.c > > +++ b/drivers/pci/pci-sysfs.c > > @@ -942,6 +942,7 @@ void pci_create_legacy_files(struct pci_bus *b) > > b->legacy_io->read =3D pci_read_legacy_io; > > b->legacy_io->write =3D pci_write_legacy_io; > > b->legacy_io->mmap =3D pci_mmap_legacy_io; > > + b->legacy_io->mapping =3D iomem_get_mapping(); > > pci_adjust_legacy_attr(b, pci_mmap_io); > > error =3D device_create_bin_file(&b->dev, b->legacy_io); > > if (error) > > @@ -954,6 +955,7 @@ void pci_create_legacy_files(struct pci_bus *b) > > b->legacy_mem->size =3D 1024*1024; > > b->legacy_mem->attr.mode =3D 0600; > > b->legacy_mem->mmap =3D pci_mmap_legacy_mem; > > + b->legacy_io->mapping =3D iomem_get_mapping(); > > pci_adjust_legacy_attr(b, pci_mmap_mem); > > error =3D device_create_bin_file(&b->dev, b->legacy_mem); > > if (error) > > @@ -1169,6 +1171,8 @@ static int pci_create_attr(struct pci_dev *pdev, = int num, int write_combine) > > res_attr->mmap =3D pci_mmap_resource_uc; > > } > > } > > + if (res_attr->mmap) > > + res_attr->mapping =3D iomem_get_mapping(); > > res_attr->attr.name =3D res_attr_name; > > res_attr->attr.mode =3D 0600; > > res_attr->size =3D pci_resource_len(pdev, num); > > diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c > > index 3a2f90beb4cb..9bab07302bbf 100644 > > --- a/drivers/pci/proc.c > > +++ b/drivers/pci/proc.c > > @@ -298,6 +298,7 @@ static int proc_bus_pci_open(struct inode *inode, s= truct file *file) > > fpriv->write_combine =3D 0; > > > > file->private_data =3D fpriv; > > + file->f_mapping =3D iomem_get_mapping(); > > > > return 0; > > } > > -- > > 2.30.0 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --=20 Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch