Message ID | 20201026105818.2585306-1-daniel.vetter@ffwll.ch (mailing list archive) |
---|---|
Headers |
Received: from vger.kernel.org ([23.128.96.18]) by www.linuxtv.org with esmtp (Exim 4.92) (envelope-from <linux-media-owner@vger.kernel.org>) id 1kX064-0011sy-Ch; Mon, 26 Oct 2020 10:51:29 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1771774AbgJZK62 (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Mon, 26 Oct 2020 06:58:28 -0400 Received: from mail-wm1-f66.google.com ([209.85.128.66]:36858 "EHLO mail-wm1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1771833AbgJZK61 (ORCPT <rfc822;linux-media@vger.kernel.org>); Mon, 26 Oct 2020 06:58:27 -0400 Received: by mail-wm1-f66.google.com with SMTP id e2so11984893wme.1 for <linux-media@vger.kernel.org>; Mon, 26 Oct 2020 03:58:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=FLkPK/wakWgPcP2XtBGkyHk3/K5f5jOPCyDZpBtuyVA=; b=UWVGBGjyj6MIRYbUDiITJq5L6QqEg6/q2/NHckpyd8RPSBh26b0bPaluvTWItOuYKc uaoPMBpxALWoXXsF90NCYVWJoITCFikSp8O7FpqdOPDdJXFgIlHFG/TgfvpItfUwxafp 6wW5he4gsgXTW7dkqWJgYLc8PHDC8/A2/zOmc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=FLkPK/wakWgPcP2XtBGkyHk3/K5f5jOPCyDZpBtuyVA=; b=pvUWE67V37bL/odKkf/K895cdLCN/fTfhmMLHZVqCmXfOqj2UDbA74J84dICRzXotm gjnU7VmzOUlWkCauQYUP3F7MPy7GEao5KWpZgW+LZWOY6vb11j7wgg6NOoNxQrxg8VsI 9oLeRmpVJerpxh7f58zyiZzk9KIwBeXGKZBKsTA98J2CX9hhMdv3WPOLfBNgKdFAyR+0 PBYB1S8Wmg3DS7FXKQnNUulsEo1isDY0GNaQNF42F9JGgaf+AM0vFYMpsHBhRJ/KAvNJ t0ZTYIiS9uMVRqTNmwmH2gyaTNgvrSv0j8e/glIlej+P/UjvtujOVMj5ltYlrYv6sIDf xsqA== X-Gm-Message-State: AOAM533kqo0oGGfH/CUYQxsoi3vOjnEEhoTbgel9GQpXxCP9Y28Lx01S tx4EeKUtbgV8k952yzuEIv9YIg== X-Google-Smtp-Source: ABdhPJxesVax0WQRMCyO4OuQvYno6G/x4zHb3UoXArN6uKT0KIo/6VWoGjLr6p0ejXLAy+E8fDIECg== X-Received: by 2002:a1c:3243:: with SMTP id y64mr15267147wmy.175.1603709905111; Mon, 26 Oct 2020 03:58:25 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:57f4:0:efd0:b9e5:5ae6:c2fa]) by smtp.gmail.com with ESMTPSA id w83sm21165156wmg.48.2020.10.26.03.58.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Oct 2020 03:58:24 -0700 (PDT) From: Daniel Vetter <daniel.vetter@ffwll.ch> To: DRI Development <dri-devel@lists.freedesktop.org>, LKML <linux-kernel@vger.kernel.org> Cc: kvm@vger.kernel.org, linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-media@vger.kernel.org, linux-s390@vger.kernel.org, Daniel Vetter <daniel.vetter@ffwll.ch> Subject: [PATCH v4 00/15] follow_pfn and other iomap races Date: Mon, 26 Oct 2020 11:58:03 +0100 Message-Id: <20201026105818.2585306-1-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.28.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-LSpam-Score: -2.5 (--) X-LSpam-Report: No, score=-2.5 required=5.0 tests=BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1 autolearn=ham autolearn_force=no |
Series |
follow_pfn and other iomap races
|
|
Message
Daniel Vetter
Oct. 26, 2020, 10:58 a.m. UTC
Hi all Round 3 of my patch series to clamp down a bunch of races and gaps around follow_pfn and other access to iomem mmaps. Previous version: v1: https://lore.kernel.org/dri-devel/20201007164426.1812530-1-daniel.vetter@ffwll.ch/ v2: https://lore.kernel.org/dri-devel/20201009075934.3509076-1-daniel.vetter@ffwll.ch v3: https://lore.kernel.org/dri-devel/20201021085655.1192025-1-daniel.vetter@ffwll.ch/ And the discussion that sparked this journey: https://lore.kernel.org/dri-devel/20201007164426.1812530-1-daniel.vetter@ffwll.ch/ Changes in v4: - Drop the s390 patch, that was very stand-alone and now queued up to land through s390 trees. - Comment polish per Dan's review. Changes in v3: - Bunch of polish all over, no functional changes aside from one barrier in the resource code, for consistency. - A few more r-b tags. Changes in v2: - tons of small polish&fixes all over, thanks to all the reviewers who spotted issues - I managed to test at least the generic_access_phys and pci mmap revoke stuff with a few gdb sessions using our i915 debug tools (hence now also the drm/i915 patch to properly request all the pci bar regions) - reworked approach for the pci mmap revoke: Infrastructure moved into kernel/resource.c, address_space mapping is now set up at open time for everyone (which required some sysfs changes). Does indeed look a lot cleaner and a lot less invasive than I feared at first. I feel like this is ready for some wider soaking. Since the remaining bits are all kinda connnected probably simplest if it all goes through -mm. Cheers, Daniel Daniel Vetter (15): drm/exynos: Stop using frame_vector helpers drm/exynos: Use FOLL_LONGTERM for g2d cmdlists misc/habana: Stop using frame_vector helpers misc/habana: Use FOLL_LONGTERM for userptr mm/frame-vector: Use FOLL_LONGTERM media: videobuf2: Move frame_vector into media subsystem mm: Close race in generic_access_phys mm: Add unsafe_follow_pfn media/videbuf1|2: Mark follow_pfn usage as unsafe vfio/type1: Mark follow_pfn as unsafe PCI: Obey iomem restrictions for procfs mmap /dev/mem: Only set filp->f_mapping resource: Move devmem revoke code to resource framework sysfs: Support zapping of binary attr mmaps PCI: Revoke mappings like devmem drivers/char/mem.c | 86 +-------------- drivers/gpu/drm/exynos/Kconfig | 1 - drivers/gpu/drm/exynos/exynos_drm_g2d.c | 48 ++++----- drivers/media/common/videobuf2/Kconfig | 1 - drivers/media/common/videobuf2/Makefile | 1 + .../media/common/videobuf2}/frame_vector.c | 54 ++++------ drivers/media/platform/omap/Kconfig | 1 - drivers/media/v4l2-core/videobuf-dma-contig.c | 2 +- drivers/misc/habanalabs/Kconfig | 1 - drivers/misc/habanalabs/common/habanalabs.h | 6 +- drivers/misc/habanalabs/common/memory.c | 50 ++++----- drivers/pci/pci-sysfs.c | 4 + drivers/pci/proc.c | 6 ++ drivers/vfio/vfio_iommu_type1.c | 4 +- fs/sysfs/file.c | 11 ++ include/linux/ioport.h | 6 +- include/linux/mm.h | 47 +------- include/linux/sysfs.h | 2 + include/media/frame_vector.h | 47 ++++++++ include/media/videobuf2-core.h | 1 + kernel/resource.c | 101 +++++++++++++++++- mm/Kconfig | 3 - mm/Makefile | 1 - mm/memory.c | 78 +++++++++++++- mm/nommu.c | 17 +++ security/Kconfig | 13 +++ 26 files changed, 347 insertions(+), 245 deletions(-) rename {mm => drivers/media/common/videobuf2}/frame_vector.c (85%) create mode 100644 include/media/frame_vector.h
Comments
Maybe I'm missing something, but shouldn't follow_pfn be unexported at the end of the series?
On Thu, Oct 29, 2020 at 9:57 AM Christoph Hellwig <hch@infradead.org> wrote: > > Maybe I'm missing something, but shouldn't follow_pfn be unexported > at the end of the series? kvm is a legit user and modular afaict. But since you can't use this without an mmu_notifier anyway (or digging around in pagetable locking), maybe it should be EXPORT_SYMBOL_GPL now at least? -Daniel
On Thu, Oct 29, 2020 at 10:25:16AM +0100, Daniel Vetter wrote: > On Thu, Oct 29, 2020 at 9:57 AM Christoph Hellwig <hch@infradead.org> wrote: > > > > Maybe I'm missing something, but shouldn't follow_pfn be unexported > > at the end of the series? > > kvm is a legit user and modular afaict. But since you can't use this > without an mmu_notifier anyway (or digging around in pagetable > locking), maybe it should be EXPORT_SYMBOL_GPL now at least? I think it should then take the notifier as an argument even if it isn't diretly used as a safety check, and get a new name describing it. EXPORT_SYMBOL_GPL is probably ok for now, but I'm drafting a new EXPORT_SYMBOL_FOR_MODULE() which will export symbols that can only be used by one specific module, with kvm being a prime user due to all the odd exports it requires that aren't really the kernel interface by any normal means.
On Thu, Oct 29, 2020 at 10:28 AM Christoph Hellwig <hch@infradead.org> wrote: > > On Thu, Oct 29, 2020 at 10:25:16AM +0100, Daniel Vetter wrote: > > On Thu, Oct 29, 2020 at 9:57 AM Christoph Hellwig <hch@infradead.org> wrote: > > > > > > Maybe I'm missing something, but shouldn't follow_pfn be unexported > > > at the end of the series? > > > > kvm is a legit user and modular afaict. But since you can't use this > > without an mmu_notifier anyway (or digging around in pagetable > > locking), maybe it should be EXPORT_SYMBOL_GPL now at least? > > I think it should then take the notifier as an argument even if it isn't > diretly used as a safety check, and get a new name describing it. Hm so Jason and me discussed this, but e.g. the s390 is safe with with just the pagetable locks. So we'd need two versions. The more practical problem is that I haven't found a reasonable way to check that a passed in mmu_notifier is registered against the mm we're working on, and without that check it feels a bit silly. But if you see how to do that I think we can do an EXPORT_SYMBOL_GPL follow_pfn which takes the notifier, and an __follow_pfn for s390 and similar internal code which isn't exported. > EXPORT_SYMBOL_GPL is probably ok for now, but I'm drafting a new > EXPORT_SYMBOL_FOR_MODULE() which will export symbols that can only be > used by one specific module, with kvm being a prime user due to all > the odd exports it requires that aren't really the kernel interface by > any normal means. Hm yeah that's another one. There's also some virt stuff that's currently on unsafe_follow_pfn and needs to be switched over, and I think that would also need an mmu notifier of some sorts to close the gaps. -Daniel
On Thu, Oct 29, 2020 at 10:38:16AM +0100, Daniel Vetter wrote: > Hm so Jason and me discussed this, but e.g. the s390 is safe with with > just the pagetable locks. So we'd need two versions. > > The more practical problem is that I haven't found a reasonable way to > check that a passed in mmu_notifier is registered against the mm we're > working on, and without that check it feels a bit silly. But if you > see how to do that I think we can do an EXPORT_SYMBOL_GPL follow_pfn > which takes the notifier, and an __follow_pfn for s390 and similar > internal code which isn't exported. True, this is a bit of a mess. So maybe just rename it to __follow_pfn, proper documentation of the requirements and a switch to EXPORT_SYMBOL_GPL.