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=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 36EEBC4338F for ; Wed, 25 Aug 2021 00:58:32 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 9E085611AF for ; Wed, 25 Aug 2021 00:58:31 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 9E085611AF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id CE7C58D0001; Tue, 24 Aug 2021 20:58:30 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C985D6B0071; Tue, 24 Aug 2021 20:58:30 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B866E8D0001; Tue, 24 Aug 2021 20:58:30 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0191.hostedemail.com [216.40.44.191]) by kanga.kvack.org (Postfix) with ESMTP id 9C7246B006C for ; Tue, 24 Aug 2021 20:58:30 -0400 (EDT) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 272B98249980 for ; Wed, 25 Aug 2021 00:58:30 +0000 (UTC) X-FDA: 78511792380.21.B2C09AD Received: from mail-pj1-f42.google.com (mail-pj1-f42.google.com [209.85.216.42]) by imf29.hostedemail.com (Postfix) with ESMTP id 84D699000249 for ; Wed, 25 Aug 2021 00:58:29 +0000 (UTC) Received: by mail-pj1-f42.google.com with SMTP id mw10-20020a17090b4d0a00b0017b59213831so3038515pjb.0 for ; Tue, 24 Aug 2021 17:58:29 -0700 (PDT) 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=ob71979d90MnjOj85r+vOA9BYCgvaQIkyLQCHH+jqss=; b=lFFRGGzjrWGffuC8/K65BQQRki3zaQoEIJJxn/JB8Z9FJCYIyv21SAyBd8wMvprRpz 39S3+pYiU0YP7bSg1qvuKs537qZy4BCcwyyNH1MPWoftDDHbwk7gZfYOI7bNJ3YtIc6T cuGaW7xPWpGZK41lD4UKG2TuLU/VI7iH4zVJ9l4ha4Zuwo1SEhwNXNynYDXdA0++zZ8s KjuDGm5qTckmXAaOLVt0cqYac4UZPkjQFSkrz8HvbW0wXVU5LskmmhXjT3jkeS33IQDt bFny6TgBjzOeaT7cMEA60GRacuVSU8Rd/e6d3FDTqGYjIWgl+CswTQQXmqVBZTcg4TqX hTig== 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=ob71979d90MnjOj85r+vOA9BYCgvaQIkyLQCHH+jqss=; b=gFebjaVJd2CbfH3VXS6MoQMHcRQGdpwqENiEY0AKJ5trPcZAZr+yr476g4Z3nnLx5N E8W7UrQGbZeWH2QiKd/fyM/yZp98aoLUPZslwioJloB2M5JCCnbiuvXyHg5hS3qJ/9Mr cqyzgAsq7qG/uxkFWlOu5+VKD0ET11T2k55m4A9zvRJek8Yqe1Y/3nc4+lpiDwucBB3z 3ahJYVA+kd5diNrDNLXAz90GE5wHrGhxUI6sBWrYbHKmOeJ5oPXJksbarnU1Ht9rXkIQ DWZTdeGyjlKYkG5Hemp/oY7DiE7k0J8wxSVgVcuNl517MFJagBE2KkggK7NciFghszvC HI0g== X-Gm-Message-State: AOAM532nNbD0SpLHshe9J+8zldiwgBaM6LgGt/5dQ+Nco9CfobQ0DZsN Paye1+q7y3Yjo0vEZnPOyVV6mDEktRvdL2DI/8ZkKQ== X-Google-Smtp-Source: ABdhPJxuZMTAhxvYtO4b94zLojEvlooMoigU04ezc3LW3HHFIgnsCDKSpuGuZRfSdRqkCYTPvvJxJGUOfUHf9APl8R4= X-Received: by 2002:a17:902:edd0:b0:135:b351:bd5a with SMTP id q16-20020a170902edd000b00135b351bd5amr7583692plk.52.1629853108334; Tue, 24 Aug 2021 17:58:28 -0700 (PDT) MIME-Version: 1.0 References: <20210816142505.28359-1-david@redhat.com> <20210816142505.28359-2-david@redhat.com> In-Reply-To: <20210816142505.28359-2-david@redhat.com> From: Dan Williams Date: Tue, 24 Aug 2021 17:58:17 -0700 Message-ID: Subject: Re: [PATCH v2 1/3] /dev/mem: disallow access to explicitly excluded system RAM regions To: David Hildenbrand Cc: Linux Kernel Mailing List , Arnd Bergmann , Greg Kroah-Hartman , "Michael S. Tsirkin" , Jason Wang , "Rafael J. Wysocki" , Andrew Morton , Hanjun Guo , Andy Shevchenko , virtualization@lists.linux-foundation.org, Linux MM Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=intel-com.20150623.gappssmtp.com header.s=20150623 header.b=lFFRGGzj; spf=none (imf29.hostedemail.com: domain of dan.j.williams@intel.com has no SPF policy when checking 209.85.216.42) smtp.mailfrom=dan.j.williams@intel.com; dmarc=fail reason="No valid SPF, DKIM not aligned (relaxed)" header.from=intel.com (policy=none) X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 84D699000249 X-Stat-Signature: shpsb385szeijri8e5is9mpan886nt5f X-HE-Tag: 1629853109-859053 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 Mon, Aug 16, 2021 at 7:25 AM David Hildenbrand wrote: > > virtio-mem dynamically exposes memory inside a device memory region as > system RAM to Linux, coordinating with the hypervisor which parts are > actually "plugged" and consequently usable/accessible. On the one hand, the > virtio-mem driver adds/removes whole memory blocks, creating/removing busy > IORESOURCE_SYSTEM_RAM resources, on the other hand, it logically (un)plugs > memory inside added memory blocks, dynamically either exposing them to > the buddy or hiding them from the buddy and marking them PG_offline. > > virtio-mem wants to make sure that in a sane environment, nobody > "accidentially" accesses unplugged memory inside the device managed > region. After /proc/kcore has been sanitized and /dev/kmem has been > removed, /dev/mem is the remaining interface that still allows uncontrolled > access to the device-managed region of virtio-mem devices from user > space. > > There is no known sane use case for mapping virtio-mem device memory > via /dev/mem while virtio-mem driver concurrently (un)plugs memory inside > that region. So once the driver was loaded and detected the device > along the device-managed region, we just want to disallow any access via > /dev/mem to it. > > Let's add the basic infrastructure to exclude some physical memory > regions completely from /dev/mem access, on any architecture and under > any system configuration (independent of CONFIG_STRICT_DEVMEM and > independent of "iomem="). I'm certainly on team "/dev/mem considered harmful", but this approach feels awkward. It feels wrong for being non-committal about whether CONFIG_STRICT_DEVMEM is in wide enough use that the safety can be turned on all the time, and the configuration option dropped, or there are users clinging onto /dev/mem where they expect to be able to build a debug kernel to turn all of these restrictions off, even the virtio-mem ones. This splits the difference and says some /dev/mem accesses are always disallowed for "reasons", but I could say the same thing about pmem, there's no sane reason to allow /dev/mem which has no idea about the responsibilities of properly touching pmem to get access to it. > > Any range marked with "IORESOURCE_SYSTEM_RAM | IORESOURCE_EXCLUSIVE" > will be excluded, even if not busy. For now, there are no applicable > ranges and we'll modify virtio-mem next to properly set > IORESOURCE_EXCLUSIVE on the parent resource. > > As next_resource() will iterate over children although we might want to > skip a certain range completely, let's add and use > next_range_skip_children() and for_each_resource(), to optimize that case, > avoding having to traverse subtrees that are not of interest. > > Signed-off-by: David Hildenbrand > --- > drivers/char/mem.c | 22 ++++++++------------ > include/linux/ioport.h | 1 + > kernel/resource.c | 47 ++++++++++++++++++++++++++++++++++++++++++ > lib/Kconfig.debug | 4 +++- > 4 files changed, 60 insertions(+), 14 deletions(-) > > diff --git a/drivers/char/mem.c b/drivers/char/mem.c > index 1c596b5cdb27..1829dc6a1f29 100644 > --- a/drivers/char/mem.c > +++ b/drivers/char/mem.c > @@ -60,13 +60,18 @@ static inline int valid_mmap_phys_addr_range(unsigned long pfn, size_t size) > } > #endif > > -#ifdef CONFIG_STRICT_DEVMEM > static inline int page_is_allowed(unsigned long pfn) > { > - return devmem_is_allowed(pfn); > +#ifdef CONFIG_STRICT_DEVMEM > + if (!devmem_is_allowed(pfn)) > + return 0; > +#endif /* CONFIG_STRICT_DEVMEM */ > + return !iomem_range_contains_excluded_devmem(PFN_PHYS(pfn), PAGE_SIZE); > } > + > static inline int range_is_allowed(unsigned long pfn, unsigned long size) > { > +#ifdef CONFIG_STRICT_DEVMEM > u64 from = ((u64)pfn) << PAGE_SHIFT; > u64 to = from + size; > u64 cursor = from; > @@ -77,18 +82,9 @@ static inline int range_is_allowed(unsigned long pfn, unsigned long size) > cursor += PAGE_SIZE; > pfn++; > } > - return 1; > -} > -#else > -static inline int page_is_allowed(unsigned long pfn) > -{ > - return 1; > -} > -static inline int range_is_allowed(unsigned long pfn, unsigned long size) > -{ > - return 1; > +#endif /* CONFIG_STRICT_DEVMEM */ > + return !iomem_range_contains_excluded_devmem(PFN_PHYS(pfn), size); > } > -#endif > > #ifndef unxlate_dev_mem_ptr > #define unxlate_dev_mem_ptr unxlate_dev_mem_ptr > diff --git a/include/linux/ioport.h b/include/linux/ioport.h > index 8359c50f9988..d31f83281327 100644 > --- a/include/linux/ioport.h > +++ b/include/linux/ioport.h > @@ -308,6 +308,7 @@ extern struct resource * __devm_request_region(struct device *dev, > extern void __devm_release_region(struct device *dev, struct resource *parent, > resource_size_t start, resource_size_t n); > extern int iomem_map_sanity_check(resource_size_t addr, unsigned long size); > +extern bool iomem_range_contains_excluded_devmem(u64 addr, u64 size); > extern bool iomem_is_exclusive(u64 addr); > > extern int > diff --git a/kernel/resource.c b/kernel/resource.c > index ca9f5198a01f..f57a14617c49 100644 > --- a/kernel/resource.c > +++ b/kernel/resource.c > @@ -73,6 +73,18 @@ static struct resource *next_resource(struct resource *p) > return p->sibling; > } > > +static struct resource *next_resource_skip_children(struct resource *p) > +{ > + while (!p->sibling && p->parent) > + p = p->parent; > + return p->sibling; > +} > + > +#define for_each_resource(_root, _p, _skip_children) \ > + for ((_p) = (_root)->child; (_p); \ > + (_p) = (_skip_children) ? next_resource_skip_children(_p) : \ > + next_resource(_p)) > + > static void *r_next(struct seq_file *m, void *v, loff_t *pos) > { > struct resource *p = v; > @@ -1700,6 +1712,41 @@ int iomem_map_sanity_check(resource_size_t addr, unsigned long size) > return err; > } > > +/* > + * Check if a physical memory range is completely excluded from getting > + * mapped/accessed via /dev/mem. > + */ > +bool iomem_range_contains_excluded_devmem(u64 addr, u64 size) > +{ > + const unsigned int flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_EXCLUSIVE; > + bool skip_children = false, excluded = false; > + struct resource *p; > + > + read_lock(&resource_lock); > + for_each_resource(&iomem_resource, p, skip_children) { > + if (p->start >= addr + size) > + break; > + if (p->end < addr) { > + skip_children = true; > + continue; > + } > + skip_children = false; > + > + /* > + * A system RAM resource is excluded if IORESOURCE_EXCLUSIVE > + * is set, even if not busy and even if we don't have strict > + * checks enabled -- no ifs or buts. > + */ > + if ((p->flags & flags) == flags) { > + excluded = true; > + break; > + } > + } > + read_unlock(&resource_lock); > + > + return excluded; > +} > + > #ifdef CONFIG_STRICT_DEVMEM > static int strict_iomem_checks = 1; > #else > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 5ddd575159fb..d0ce6e23a6db 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -1780,7 +1780,9 @@ config STRICT_DEVMEM > access to this is obviously disastrous, but specific access can > be used by people debugging the kernel. Note that with PAT support > enabled, even in this case there are restrictions on /dev/mem > - use due to the cache aliasing requirements. > + use due to the cache aliasing requirements. Further, some drivers > + will still restrict access to some physical memory regions either > + already used or to be used in the future as system RAM. > > If this option is switched on, and IO_STRICT_DEVMEM=n, the /dev/mem > file only allows userspace access to PCI space and the BIOS code and > -- > 2.31.1 >