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=-8.3 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=unavailable 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 4600DC4360F for ; Thu, 4 Apr 2019 16:41:43 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 124DC206DF for ; Thu, 4 Apr 2019 16:41:42 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="UwB8zBa6"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="e/eUApgI" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 124DC206DF Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=roeck-us.net Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-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=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=+qbOjezOO3rPiROmbxlC4ClAZ7QvzbnshQ9d0umA4r8=; b=UwB8zBa65uFIqR OJdnTvGGUTW8df8QIwzpHCjfdP9wj3F+ncA75IuxNXOMW6uUkHR1V0YuCHY95kX1+kXACOFuYHpPK gtC2DnZZcJktEu/l5HsTiRBdlkCyJ+Pqop9t4BDK7kLWlo44Izk2YQzJf4CACtdwWAG/mHL+tTfe0 rGgIIgIx/wtsgc4PTU406EtKiyQcyU4Wrcl+1Gp6td4uM+pQwZIovivEPlgeGRlx6WTdTfQopUb76 Xdyk3ZqrJaMwtTFT/IGWmPkm0uL+vMiNq/In5rb90y9eSrsBUsoFnww0y8GvPaipfpBqoVryK+dRO qHY265x92c32SK+ki7Ow==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hC5Qn-000617-Nt; Thu, 04 Apr 2019 16:41:37 +0000 Received: from mail-pl1-x643.google.com ([2607:f8b0:4864:20::643]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1hC5Qk-00060V-H4 for linux-arm-kernel@lists.infradead.org; Thu, 04 Apr 2019 16:41:36 +0000 Received: by mail-pl1-x643.google.com with SMTP id b3so1432405plr.7 for ; Thu, 04 Apr 2019 09:41:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=uenWNpIoJ4m0YsZyBYyNcvf/yqPcCp16FMlFmc3fw54=; b=e/eUApgIjYMbx+cHGGPuN6SrAhe9M+WvztlXyVWNu+spUsEBe08JkuqvdzAsu/eIsH arULfOS7tfpAm4zzioI8ZFPW6iy9Owi7qQfEy/XNMIX5JCu/GAYHj2gz8bHBBfVp1OZl N+xqucdLzCzy2MPIrNJwRTMd6BC7G1Y029MLOBa3P2x70/uinDTJ+QUul9IbmtzT7MyH tKFO9Ma6qeNQe44EtgcXN+mzlmwBhMae/kQkIjsjhO9jB8J8+Dk8cDBW39pR11mq31zF j0g2YIGcN47NLfZv//fTY0rvbEo2g1b8K+X1wn1pQUCYCNqb6zbk3NbGaLdE6Swnxl1G kTVw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=uenWNpIoJ4m0YsZyBYyNcvf/yqPcCp16FMlFmc3fw54=; b=DAlEFzNlWnVnrisG5G+QOPpovqBp4KHIoTxJs4eYrx+AmtrOS00kausPOpNYODS2E4 6tKqGAoSGjW12zwfQEgGS/bHFcnnCdAOEeb039ydWpwjhPFOno3Ab273MAo/sAAHz3Vp jlazG/6kovXtHZ/OyVJWl2dK6c6lcjqJ+JwlISXKuTNJ4GHh1cf8Ha9MjgBzdDVWtA0W uAPyNH0+vaduq2CXtkgh/K8c2nHU4elzOREsYY23oHU/ANnpnT8Mfa98bZBMuBEaVH4J 7RYAgMNIa3EzmNi86urTEA4IjasGQWU3gqQDI4Q7G4BUlEXqA19r/YpieNNUcNU91f8A UY7w== X-Gm-Message-State: APjAAAWxvXso4kKW+wJVV5u3JVXV0JEcKdF6ulqVWVTvI+mIfGErQ22X 4MyhRmgwbCnnUpQb1J7OYV8= X-Google-Smtp-Source: APXvYqyCE86GOkSayF6AwsvdAiWgMPja5B1GtS2tUgEbKpHW+z+LYGzl2gXFRPn7cUrfpD32g8HTrQ== X-Received: by 2002:a17:902:e084:: with SMTP id cb4mr7603441plb.77.1554396093398; Thu, 04 Apr 2019 09:41:33 -0700 (PDT) Received: from localhost ([2600:1700:e321:62f0:329c:23ff:fee3:9d7c]) by smtp.gmail.com with ESMTPSA id p81sm33367209pfi.186.2019.04.04.09.41.31 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 04 Apr 2019 09:41:32 -0700 (PDT) Date: Thu, 4 Apr 2019 09:41:31 -0700 From: Guenter Roeck To: John Garry Subject: Re: [PATCH v3 3/4] lib: logic_pio: Reject accesses to unregistered CPU MMIO regions Message-ID: <20190404164130.GA12203@roeck-us.net> References: <1554393602-152448-1-git-send-email-john.garry@huawei.com> <1554393602-152448-4-git-send-email-john.garry@huawei.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1554393602-152448-4-git-send-email-john.garry@huawei.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190404_094134_595237_9949381D X-CRM114-Status: GOOD ( 24.02 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: wangkefeng.wang@huawei.com, lorenzo.pieralisi@arm.com, arnd@arndb.de, rafael@kernel.org, linux-pci@vger.kernel.org, will.deacon@arm.com, linux-kernel@vger.kernel.org, linuxarm@huawei.com, andy.shevchenko@gmail.com, catalin.marinas@arm.com, bhelgaas@google.com, bp@suse.de, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Apr 05, 2019 at 12:00:01AM +0800, John Garry wrote: > Currently when accessing logical indirect PIO addresses in > logic_{in, out}{,s}, we first ensure that the region is registered. > > However, no such check exists for CPU MMIO regions. The CPU MMIO regions > would be registered by the PCI host - when PCI_IOBASE is defined - in > pci_register_io_range(). > > We have seen scenarios when systems which don't have a PCI host or, they > do, and the PCI host probe fails, that certain devices attempts to still > attempt to access PCI IO ports; examples are in [1] and [2]. > > And even though we should protect against this by ensuring the driver > calls request_{muxed_}region(), some don't do this: > > root@(none)$ insmod hwmon/f71805f.ko > Unable to handle kernel paging request at virtual address ffff7dfffee0002e > Mem abort info: > ESR = 0x96000046 > Exception class = DABT (current EL), IL = 32 bits > SET = 0, FnV = 0 > EA = 0, S1PTW = 0 > Data abort info: > ISV = 0, ISS = 0x00000046 > CM = 0, WnR = 1 > swapper pgtable: 4k pages, 48-bit VAs, pgdp = (____ptrval____) > [ffff7dfffee0002e] pgd=000000000141c003, pud=000000000141d003, pmd=0000000000000000 > Internal error: Oops: 96000046 [#1] PREEMPT SMP > Modules linked in: f71805f(+) > CPU: 20 PID: 2736 Comm: insmod Not tainted 5.1.0-rc1-00003-g6f1bfec2a620-dirty #99 > Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon D05 IT21 Nemo 2.0 RC0 04/18/2018 > pstate: 80000005 (Nzcv daif -PAN -UAO) > pc : logic_outb+0x54/0xb8 > lr : f71805f_find+0x2c/0x1b8 [f71805f] > sp : ffff000025fbba90 > x29: ffff000025fbba90 x28: ffff000008b944d0 > x27: ffff000025fbbdf0 x26: 0000000000000100 > x25: ffff801f8c270580 x24: ffff000011420000 > x23: ffff000025fbbb3e x22: ffff000025fbbb40 > x21: ffff000008b991b8 x20: 0000000000000087 > x19: 000000000000002e x18: ffffffffffffffff > x17: 0000000000000000 x16: 0000000000000000 > x15: ffff00001127d6c8 x14: 0000000000000000 > x13: 0000000000000000 x12: 0000000000000000 > x11: 0000000000010820 x10: 0000841fdac40000 > x9 : 0000000000000001 x8 : 0000000040000000 > x7 : 0000000000210d00 x6 : 0000000000000000 > x5 : ffff801fb6a46040 x4 : ffff841febeaeda0 > x3 : 0000000000ffbffe x2 : ffff000025fbbb40 > x1 : ffff7dfffee0002e x0 : ffff7dfffee00000 > Process insmod (pid: 2736, stack limit = 0x(____ptrval____)) > Call trace: > logic_outb+0x54/0xb8 > f71805f_find+0x2c/0x1b8 [f71805f] > f71805f_init+0x38/0xe48 [f71805f] > do_one_initcall+0x5c/0x198 > do_init_module+0x54/0x1b0 > load_module+0x1dc4/0x2158 > __se_sys_init_module+0x14c/0x1e8 > __arm64_sys_init_module+0x18/0x20 > el0_svc_common+0x5c/0x100 > el0_svc_handler+0x2c/0x80 > el0_svc+0x8/0xc > Code: d2bfdc00 f2cfbfe0 f2ffffe0 8b000021 (39000034) > ---[ end trace 10ea80bde051bbfc ]--- > root@(none)$ > > Note that the f71805f driver does not call request_{muxed_}region(), as it > should. > ... which is the real problem, one that is not solved by this patch. This may result in parallel and descructive accesses if there is another device on the LPC bus, and another driver accessing that device. Personally I'd rather have request_muxed_region() added to the f71805f driver. Guenter > This patch adds a check to ensure that the CPU MMIO region is registered > prior to accessing the PCI IO ports. > > [1] https://lore.kernel.org/linux-pci/56F209A9.4040304@huawei.com > [2] https://lore.kernel.org/linux-arm-kernel/e6995b4a-184a-d8d4-f4d4-9ce75d8f47c0@huawei.com/ > > This patch includes some other tidy-up. > > Signed-off-by: John Garry > --- > lib/logic_pio.c | 103 +++++++++++++++++++++++++++++++++++------------- > 1 file changed, 75 insertions(+), 28 deletions(-) > > diff --git a/lib/logic_pio.c b/lib/logic_pio.c > index 431cd8d99236..3d8d986e9dcb 100644 > --- a/lib/logic_pio.c > +++ b/lib/logic_pio.c > @@ -193,95 +193,135 @@ unsigned long logic_pio_trans_cpuaddr(resource_size_t addr) > > #if defined(PCI_IOBASE) > #if defined(CONFIG_INDIRECT_PIO) > +#define INVALID_RANGE(range) \ > + (!(range) || ((range)->flags == LOGIC_PIO_INDIRECT && !(range)->ops)) > + > #define BUILD_LOGIC_IO(bw, type) \ > type logic_in##bw(unsigned long addr) \ > { \ > type ret = (type)~0; \ > + struct logic_pio_hwaddr *range = find_io_range(addr); \ > + \ > + if (INVALID_RANGE(range)) { \ > + WARN_ON_ONCE(1); \ > + return ret; \ > + } \ > \ > if (addr < MMIO_UPPER_LIMIT) { \ > ret = read##bw(PCI_IOBASE + addr); \ > } else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) { \ > - struct logic_pio_hwaddr *entry = find_io_range(addr); \ > size_t sz = sizeof(type); \ > + void *hostdata = range->hostdata; \ > \ > - if (entry && entry->ops) \ > - ret = entry->ops->in(entry->hostdata, addr, sz);\ > - else \ > - WARN_ON_ONCE(1); \ > + if (range->ops->in) \ > + ret = range->ops->in(hostdata, addr, sz); \ > } \ > return ret; \ > } \ > \ > -void logic_out##bw(type value, unsigned long addr) \ > +void logic_out##bw(type val, unsigned long addr) \ > { \ > + struct logic_pio_hwaddr *range = find_io_range(addr); \ > + \ > + if (INVALID_RANGE(range)) { \ > + WARN_ON_ONCE(1); \ > + return; \ > + } \ > + \ > if (addr < MMIO_UPPER_LIMIT) { \ > - write##bw(value, PCI_IOBASE + addr); \ > + write##bw(val, PCI_IOBASE + addr); \ > } else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) { \ > - struct logic_pio_hwaddr *entry = find_io_range(addr); \ > size_t sz = sizeof(type); \ > + void *hostdata = range->hostdata; \ > \ > - if (entry && entry->ops) \ > - entry->ops->out(entry->hostdata, \ > - addr, value, sz); \ > - else \ > - WARN_ON_ONCE(1); \ > + if (range->ops->out) \ > + range->ops->out(hostdata, addr, val, sz); \ > } \ > } \ > \ > void logic_ins##bw(unsigned long addr, void *buf, unsigned int cnt) \ > { \ > + struct logic_pio_hwaddr *range = find_io_range(addr); \ > + \ > + if (INVALID_RANGE(range)) { \ > + WARN_ON_ONCE(1); \ > + return; \ > + } \ > + \ > if (addr < MMIO_UPPER_LIMIT) { \ > reads##bw(PCI_IOBASE + addr, buf, cnt); \ > } else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) { \ > - struct logic_pio_hwaddr *entry = find_io_range(addr); \ > size_t sz = sizeof(type); \ > + void *hostdata = range->hostdata; \ > \ > - if (entry && entry->ops) \ > - entry->ops->ins(entry->hostdata, \ > - addr, buf, sz, cnt); \ > - else \ > - WARN_ON_ONCE(1); \ > + if (range->ops->ins) \ > + range->ops->ins(hostdata, addr, buf, sz, cnt); \ > } \ > - \ > } \ > \ > void logic_outs##bw(unsigned long addr, const void *buf, \ > unsigned int cnt) \ > { \ > + struct logic_pio_hwaddr *range = find_io_range(addr); \ > + \ > + if (INVALID_RANGE(range)) { \ > + WARN_ON_ONCE(1); \ > + return; \ > + } \ > + \ > if (addr < MMIO_UPPER_LIMIT) { \ > writes##bw(PCI_IOBASE + addr, buf, cnt); \ > } else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) { \ > - struct logic_pio_hwaddr *entry = find_io_range(addr); \ > size_t sz = sizeof(type); \ > + void *hostdata = range->hostdata; \ > \ > - if (entry && entry->ops) \ > - entry->ops->outs(entry->hostdata, \ > - addr, buf, sz, cnt); \ > - else \ > - WARN_ON_ONCE(1); \ > + if (range->ops->outs) \ > + range->ops->outs(hostdata, addr, buf, sz, cnt); \ > } \ > } > > #else /* CONFIG_INDIRECT_PIO */ > > +#define INVALID_RANGE(range) (!(range)) > + > #define BUILD_LOGIC_IO(bw, type) \ > type logic_in##bw(unsigned long addr) \ > { \ > type ret = (type)~0; \ > + struct logic_pio_hwaddr *range = find_io_range(addr); \ > + \ > + if (INVALID_RANGE(range)) { \ > + WARN_ON_ONCE(1); \ > + return ret; \ > + } \ > \ > if (addr < MMIO_UPPER_LIMIT) \ > ret = read##bw(PCI_IOBASE + addr); \ > return ret; \ > } \ > \ > -void logic_out##bw(type value, unsigned long addr) \ > +void logic_out##bw(type val, unsigned long addr) \ > { \ > + struct logic_pio_hwaddr *range = find_io_range(addr); \ > + \ > + if (INVALID_RANGE(range)) { \ > + WARN_ON_ONCE(1); \ > + return; \ > + } \ > + \ > if (addr < MMIO_UPPER_LIMIT) \ > - write##bw(value, PCI_IOBASE + addr); \ > + write##bw(val, PCI_IOBASE + addr); \ > } \ > \ > void logic_ins##bw(unsigned long addr, void *buf, unsigned int cnt) \ > { \ > + struct logic_pio_hwaddr *range = find_io_range(addr); \ > + \ > + if (INVALID_RANGE(range)) { \ > + WARN_ON_ONCE(1); \ > + return; \ > + } \ > + \ > if (addr < MMIO_UPPER_LIMIT) \ > reads##bw(PCI_IOBASE + addr, buf, cnt); \ > } \ > @@ -289,6 +329,13 @@ void logic_ins##bw(unsigned long addr, void *buf, unsigned int cnt) \ > void logic_outs##bw(unsigned long addr, const void *buf, \ > unsigned int cnt) \ > { \ > + struct logic_pio_hwaddr *range = find_io_range(addr); \ > + \ > + if (INVALID_RANGE(range)) { \ > + WARN_ON_ONCE(1); \ > + return; \ > + } \ > + \ > if (addr < MMIO_UPPER_LIMIT) \ > writes##bw(PCI_IOBASE + addr, buf, cnt); \ > } > -- > 2.17.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel