linux-kernel - Re: [PATCH v5 RESEND 14/17] mm/ioremap: Consider IOREMAP space in generic ioremap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 30 May 2023 17:37:23 +0800
From:   Baoquan He <bhe@...hat.com>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
        linux-mm@...ck.org, arnd@...db.de, christophe.leroy@...roup.eu,
        agordeev@...ux.ibm.com, wangkefeng.wang@...wei.com,
        schnelle@...ux.ibm.com, David.Laight@...lab.com, shorne@...il.com,
        willy@...radead.org, deller@....de
Subject: Re: [PATCH v5 RESEND 14/17] mm/ioremap: Consider IOREMAP space in
 generic ioremap

On 05/16/23 at 11:44pm, Christoph Hellwig wrote:
> On Tue, May 16, 2023 at 11:41:26PM -0700, Christoph Hellwig wrote:
> > I think this would be cleaner if we'd just always use
> > __get_vm_area_caller and at the top of the file add a:
> > 
> > #ifndef IOREMAP_START
> > #define IOREMAP_START	VMALLOC_START
> > #define IOREMAP_END	VMALLOC_END
> > #endif
> > 
> > Together with a little comment that ioremap often, but not always
> > uses the generic vmalloc area.
> 
> .. and with that we can also simply is_ioremap_addr by moving it
> to ioremap.c and making it always operate on the IOREMAP constants.

In the current code, is_ioremap_addr() is being used in kernel/iomem.c.
However, mm/ioremap.c is only built in when CONFIG_GENERIC_IOREMAP is
enabled. This will impact those architectures which haven't taken
GENERIC_IOREMAP way.

[~]$ git grep is_ioremap_addr
arch/powerpc/include/asm/pgtable.h:#define is_ioremap_addr is_ioremap_addr
arch/powerpc/include/asm/pgtable.h:static inline bool is_ioremap_addr(const void *x)
include/linux/mm.h:static inline bool is_ioremap_addr(const void *x)
include/linux/mm.h:static inline bool is_ioremap_addr(const void *x)
kernel/iomem.c: if (is_ioremap_addr(addr))
mm/ioremap.c:   if (is_ioremap_addr(vaddr))

[bhe@...iFi-R3L-srv linux-arm64]$ git grep ioremap mm/Makefile
mm/Makefile:obj-$(CONFIG_GENERIC_EARLY_IOREMAP) += early_ioremap.o
mm/Makefile:obj-$(CONFIG_GENERIC_IOREMAP) += ioremap.o

If we want to consolidate code, we can move is_ioremap_addr() to
include/linux/mm.h libe below. Not sure if it's fine. With it,
both kernel/iomem.c and mm/ioremap.c can use is_ioremap_addr().

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 27ce77080c79..0fbb94f0f025 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1041,9 +1041,25 @@ unsigned long vmalloc_to_pfn(const void *addr);
  * On nommu, vmalloc/vfree wrap through kmalloc/kfree directly, so there
  * is no special casing required.
  */
-
-#ifndef is_ioremap_addr
-#define is_ioremap_addr(x) is_vmalloc_addr(x)
+#if defined(CONFIG_HAS_IOMEM) || defined(CONFIG_GENERIC_IOREMAP)
+/*
+ * Ioremap often, but not always uses the generic vmalloc area. E.g on
+ * Power ARCH, it could have different ioremap space. 
+ */
+#ifndef IOREMAP_START
+#define IOREMAP_START   VMALLOC_START
+#define IOREMAP_END     VMALLOC_END
+#endif
+static inline bool is_ioremap_addr(const void *x)
+{
+	unsigned long addr = (unsigned long)kasan_reset_tag(x);
+	return addr >= IOREMAP_START && addr < IOREMAP_END;
+}
+#else
+static inline bool is_ioremap_addr(const void *x)
+{
+	return false;
+}
 #endif
 
 #ifdef CONFIG_MMU