The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     7b02ad32d83c16abd4961d79f3154b734d1d5d9c
Gitweb:        https://git.kernel.org/tip/7b02ad32d83c16abd4961d79f3154b734d1d5d9c
Author:        Max Ramanouski <max8rr8@gmail.com>
AuthorDate:    Thu, 15 Aug 2024 23:56:07 +03:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Fri, 16 Aug 2024 11:33:33 +02:00

x86/ioremap: Use is_ioremap_addr() in iounmap()

Restrict iounmap() to memory allocated in the ioremap region, by using
is_ioremap_addr(). Similarly to the generic iounmap() implementation.

Additionally, add a warning in case there is an attempt to iounmap()
invalid memory, instead of silently exiting, thus helping to avoid
incorrect usage of iounmap().

Signed-off-by: Max Ramanouski <max8rr8@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Alistair Popple <apopple@nvidia.com>
Link: https://lore.kernel.org/all/20240815205606.16051-2-max8rr8@gmail.com
---
 arch/x86/mm/ioremap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index aa7d279..70b02fc 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -11,6 +11,7 @@
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/ioport.h>
+#include <linux/ioremap.h>
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/mmiotrace.h>
@@ -457,7 +458,7 @@ void iounmap(volatile void __iomem *addr)
 {
 	struct vm_struct *p, *o;
 
-	if ((void __force *)addr <= high_memory)
+	if (WARN_ON_ONCE(!is_ioremap_addr((void __force *)addr)))
 		return;
 
 	/*

On Fri, Aug 16 2024 at 09:43, tip-bot wrote:
> The following commit has been merged into the x86/urgent branch of tip:
>
> Commit-ID:     7b02ad32d83c16abd4961d79f3154b734d1d5d9c
> Gitweb:        https://git.kernel.org/tip/7b02ad32d83c16abd4961d79f3154b734d1d5d9c
> Author:        Max Ramanouski <max8rr8@gmail.com>
> AuthorDate:    Thu, 15 Aug 2024 23:56:07 +03:00
> Committer:     Thomas Gleixner <tglx@linutronix.de>
> CommitterDate: Fri, 16 Aug 2024 11:33:33 +02:00
>
> x86/ioremap: Use is_ioremap_addr() in iounmap()

This has been removed as it fails on a 32-bit defconfig:

include/linux/ioremap.h: In function ‘is_ioremap_addr’:
include/linux/ioremap.h:14:25: error: ‘VMALLOC_START’ undeclared (first use in this function); did you mean ‘KMALLOC_DMA’?
   14 | #define IOREMAP_START   VMALLOC_START

Can you please have a look?

Thanks,

        tglx

Apparently,  x86-32 doesn't provide VMALLOC_START in asm/pgtable.h,
and after a quick glance it seems to be the only arch to do so...
Probably the correct solution is to add include asm/vmalloc.h to
include/linux/ioremap.h, considering it uses VMALLOC_START. Will
resubmit the v4 version of patch later today.

Best regards,
Max


On Tue, 20 Aug 2024 at 23:35, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Fri, Aug 16 2024 at 09:43, tip-bot wrote:
> > The following commit has been merged into the x86/urgent branch of tip:
> >
> > Commit-ID:     7b02ad32d83c16abd4961d79f3154b734d1d5d9c
> > Gitweb:        https://git.kernel.org/tip/7b02ad32d83c16abd4961d79f3154b734d1d5d9c
> > Author:        Max Ramanouski <max8rr8@gmail.com>
> > AuthorDate:    Thu, 15 Aug 2024 23:56:07 +03:00
> > Committer:     Thomas Gleixner <tglx@linutronix.de>
> > CommitterDate: Fri, 16 Aug 2024 11:33:33 +02:00
> >
> > x86/ioremap: Use is_ioremap_addr() in iounmap()
>
> This has been removed as it fails on a 32-bit defconfig:
>
> include/linux/ioremap.h: In function ‘is_ioremap_addr’:
> include/linux/ioremap.h:14:25: error: ‘VMALLOC_START’ undeclared (first use in this function); did you mean ‘KMALLOC_DMA’?
>    14 | #define IOREMAP_START   VMALLOC_START
>
> Can you please have a look?
>
> Thanks,
>
>         tglx

On Wed, Aug 21 2024 at 11:35, Max R. wrote:

Please do not top-post and trim your replies.

> Apparently,  x86-32 doesn't provide VMALLOC_START in asm/pgtable.h,
> and after a quick glance it seems to be the only arch to do so...
> Probably the correct solution is to add include asm/vmalloc.h to
> include/linux/ioremap.h, considering it uses VMALLOC_START. Will
> resubmit the v4 version of patch later today.

Careful, this might end up with other issues vs. the kernel header
inclusion hell.

If i386 is the only one which does not have VMALLOC start in
asm/pgtable.h, then curing this might be the easier fix, no?

Thanks,

        tglx

> Please do not top-post and trim your replies.

Sorry, got a bit distracted and forgot to remove the default gmail
reply text. Will try to be more careful in future with better email
client.

> Careful, this might end up with other issues vs. the kernel header
> inclusion hell.
> If i386 is the only one which does not have VMALLOC start in
> asm/pgtable.h, then curing this might be the easier fix, no?

I thought about that initially, but then found out that apparently
it is like that for a reason. In commit:

186525bd6b8 ("mm, x86/mm: Untangle address space layout definitions from basic pgtable type definitions")

VMALLOC_START definition was specifically moved out of pgtable
related headers. But for some reason only for 32 bit arch. Plus
I think asm/vmalloc.h is more semantically correct to get
VMALLOC_START. Although on most arches (everywhere except x86,
arm64, powerpc and riscv) asm/vmalloc.h is an empty header. And
on riscv it doesn't actually provide VMALLOC_START... Probably
that is worth a separate fix later.

Best regards,
Max