[PATCH 0/2] io_uring: Fix io_uring mmap() by using architecture-provided get_unmapped_area()

[PATCH 0/2] io_uring: Fix io_uring mmap() by using architecture-provided get_unmapped_area()

[Helge Deller]

Fix io_uring on IA64 (and x86-32?) which was broken since commit
d808459b2e31 ("io_uring: Adjust mapping wrt architecture aliasing
requirements").
The fix is to switch back to the get_unmapped_area() which is provided by
each architecture.

Patch 1 switches io_uring back to use per-arch get_unmapped_area().
Patch 2 (for IA64) is an independend cleanup.

Helge

Helge Deller (2):
  io_uring: Fix io_uring mmap() by using architecture-provided
    get_unmapped_area()
  ia64: mmap: Consider pgoff when searching for free mapping

 arch/ia64/kernel/sys_ia64.c     |  2 +-
 arch/parisc/kernel/sys_parisc.c | 15 ++++++++----
 io_uring/io_uring.c             | 42 +++++++++++++--------------------
 3 files changed, 28 insertions(+), 31 deletions(-)

--
2.41.0

[PATCH 1/2] io_uring: Fix io_uring mmap() by using architecture-provided get_unmapped_area()

[Helge Deller]

The io_uring testcase is broken on IA-64 since commit d808459b2e31
("io_uring: Adjust mapping wrt architecture aliasing requirements").

The reason is, that this commit introduced an own architecture
independend get_unmapped_area() search algorithm which finds on IA-64 a
memory region which is outside of the regular memory region used for
shared userspace mappings and which can't be used on that platform
due to aliasing.

To avoid similar problems on IA-64 and other platforms in the future,
it's better to switch back to the architecture-provided
get_unmapped_area() function and adjust the needed input parameters
before the call. Beside fixing the issue, the function now becomes
easier to understand and maintain.

This patch has been successfully tested with the io_uring testcase on
physical x86-64, ppc64le, IA-64 and PA-RISC machines. On PA-RISC the LTP
mmmap testcases did not report any regressions.

Signed-off-by: Helge Deller <deller@gmx.de>
Cc: Jens Axboe <axboe@kernel.dk>
Reported-by: matoro <matoro_mailinglist_kernel@matoro.tk>
Fixes: d808459b2e31 ("io_uring: Adjust mapping wrt architecture aliasing requirements")
---
 arch/parisc/kernel/sys_parisc.c | 15 ++++++++----
 io_uring/io_uring.c             | 42 +++++++++++++--------------------
 2 files changed, 27 insertions(+), 30 deletions(-)

diff --git a/arch/parisc/kernel/sys_parisc.c b/arch/parisc/kernel/sys_parisc.c
index 39acccabf2ed..465b7cb9d44f 100644
--- a/arch/parisc/kernel/sys_parisc.c
+++ b/arch/parisc/kernel/sys_parisc.c
@@ -26,12 +26,17 @@
 #include <linux/compat.h>

 /*
- * Construct an artificial page offset for the mapping based on the physical
+ * Construct an artificial page offset for the mapping based on the virtual
  * address of the kernel file mapping variable.
+ * If filp is zero the calculated pgoff value aliases the memory of the given
+ * address. This is useful for io_uring where the mapping shall alias a kernel
+ * address and a userspace adress where both the kernel and the userspace
+ * access the same memory region.
  */
-#define GET_FILP_PGOFF(filp)		\
-	(filp ? (((unsigned long) filp->f_mapping) >> 8)	\
-		 & ((SHM_COLOUR-1) >> PAGE_SHIFT) : 0UL)
+#define GET_FILP_PGOFF(filp, addr)		\
+	((filp ? (((unsigned long) filp->f_mapping) >> 8)	\
+		 & ((SHM_COLOUR-1) >> PAGE_SHIFT) : 0UL)	\
+	  + (addr >> PAGE_SHIFT))

 static unsigned long shared_align_offset(unsigned long filp_pgoff,
 					 unsigned long pgoff)
@@ -111,7 +116,7 @@ static unsigned long arch_get_unmapped_area_common(struct file *filp,
 	do_color_align = 0;
 	if (filp || (flags & MAP_SHARED))
 		do_color_align = 1;
-	filp_pgoff = GET_FILP_PGOFF(filp);
+	filp_pgoff = GET_FILP_PGOFF(filp, addr);

 	if (flags & MAP_FIXED) {
 		/* Even MAP_FIXED mappings must reside within TASK_SIZE */
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 3bca7a79efda..227e50aa9af0 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3398,8 +3398,6 @@ static unsigned long io_uring_mmu_get_unmapped_area(struct file *filp,
 			unsigned long addr, unsigned long len,
 			unsigned long pgoff, unsigned long flags)
 {
-	const unsigned long mmap_end = arch_get_mmap_end(addr, len, flags);
-	struct vm_unmapped_area_info info;
 	void *ptr;

 	/*
@@ -3414,32 +3412,26 @@ static unsigned long io_uring_mmu_get_unmapped_area(struct file *filp,
 	if (IS_ERR(ptr))
 		return -ENOMEM;

-	info.flags = VM_UNMAPPED_AREA_TOPDOWN;
-	info.length = len;
-	info.low_limit = max(PAGE_SIZE, mmap_min_addr);
-	info.high_limit = arch_get_mmap_base(addr, current->mm->mmap_base);
+	/*
+	 * Some architectures have strong cache aliasing requirements.
+	 * For such architectures we need a coherent mapping which aliases
+	 * kernel memory *and* userspace memory. To achieve that:
+	 * - use a NULL file pointer to reference physical memory, and
+	 * - use the kernel virtual address of the shared io_uring context
+	 *   (instead of the userspace-provided address, which has to be 0UL
+	 *   anyway).
+	 * For architectures without such aliasing requirements, the
+	 * architecture will return any suitable mapping because addr is 0.
+	 */
+	filp = NULL;
+	flags |= MAP_SHARED;
+	pgoff = 0;	/* has been translated to ptr above */
 #ifdef SHM_COLOUR
-	info.align_mask = PAGE_MASK & (SHM_COLOUR - 1UL);
+	addr = (uintptr_t) ptr;
 #else
-	info.align_mask = PAGE_MASK & (SHMLBA - 1UL);
+	addr = 0UL;
 #endif
-	info.align_offset = (unsigned long) ptr;
-
-	/*
-	 * A failed mmap() very likely causes application failure,
-	 * so fall back to the bottom-up function here. This scenario
-	 * can happen with large stack limits and large mmap()
-	 * allocations.
-	 */
-	addr = vm_unmapped_area(&info);
-	if (offset_in_page(addr)) {
-		info.flags = 0;
-		info.low_limit = TASK_UNMAPPED_BASE;
-		info.high_limit = mmap_end;
-		addr = vm_unmapped_area(&info);
-	}
-
-	return addr;
+	return current->mm->get_unmapped_area(filp, addr, len, pgoff, flags);
 }

 #else /* !CONFIG_MMU */
--
2.41.0

[PATCH 2/2] ia64: mmap: Consider pgoff when searching for free mapping

[Helge Deller]

IA64 is the only architecture which does not consider the pgoff value when
searching for a possible free memory region with vm_unmapped_area().
Adding this seems to have no negative side effect on IA64, so add it now
to make IA64 consistent with all other architectures.

Signed-off-by: Helge Deller <deller@gmx.de>
Tested-by: matoro <matoro_mailinglist_kernel@matoro.tk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-ia64@vger.kernel.org
---
 arch/ia64/kernel/sys_ia64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/kernel/sys_ia64.c b/arch/ia64/kernel/sys_ia64.c
index 6e948d015332..eb561cc93632 100644
--- a/arch/ia64/kernel/sys_ia64.c
+++ b/arch/ia64/kernel/sys_ia64.c
@@ -63,7 +63,7 @@ arch_get_unmapped_area (struct file *filp, unsigned long addr, unsigned long len
 	info.low_limit = addr;
 	info.high_limit = TASK_SIZE;
 	info.align_mask = align_mask;
-	info.align_offset = 0;
+	info.align_offset = pgoff << PAGE_SHIFT;
 	return vm_unmapped_area(&info);
 }

--
2.41.0

Re: [PATCH 0/2] io_uring: Fix io_uring mmap() by using architecture-provided get_unmapped_area()

[Jens Axboe]

On 7/21/23 9:24?AM, Helge Deller wrote:
> Fix io_uring on IA64 (and x86-32?) which was broken since commit
> d808459b2e31 ("io_uring: Adjust mapping wrt architecture aliasing
> requirements").
> The fix is to switch back to the get_unmapped_area() which is provided by
> each architecture.
> 
> Patch 1 switches io_uring back to use per-arch get_unmapped_area().
> Patch 2 (for IA64) is an independend cleanup.

Let's get this queued up - I marked it for stable as well, for 6.4.
Thanks for taking care of this!

-- 
Jens Axboe

Re: [PATCH 0/2] io_uring: Fix io_uring mmap() by using architecture-provided get_unmapped_area()

[Jens Axboe]

On Fri, 21 Jul 2023 17:24:30 +0200, Helge Deller wrote:
> Fix io_uring on IA64 (and x86-32?) which was broken since commit
> d808459b2e31 ("io_uring: Adjust mapping wrt architecture aliasing
> requirements").
> The fix is to switch back to the get_unmapped_area() which is provided by
> each architecture.
> 
> Patch 1 switches io_uring back to use per-arch get_unmapped_area().
> Patch 2 (for IA64) is an independend cleanup.
> 
> [...]

Applied, thanks!

[1/2] io_uring: Fix io_uring mmap() by using architecture-provided get_unmapped_area()
      commit: 32832a407a7178eec3215fad9b1a3298c14b0d69
[2/2] ia64: mmap: Consider pgoff when searching for free mapping
      commit: 07e981137f17e5275b6fa5fd0c28b0ddb4519702

Best regards,
-- 
Jens Axboe

Re: [PATCH 1/2] io_uring: Fix io_uring mmap() by using architecture-provided get_unmapped_area()

[Jiri Slaby]

On 21. 07. 23, 17:24, Helge Deller wrote:
> The io_uring testcase is broken on IA-64 since commit d808459b2e31
> ("io_uring: Adjust mapping wrt architecture aliasing requirements").
> 
> The reason is, that this commit introduced an own architecture
> independend get_unmapped_area() search algorithm which finds on IA-64 a
> memory region which is outside of the regular memory region used for
> shared userspace mappings and which can't be used on that platform
> due to aliasing.
> 
> To avoid similar problems on IA-64 and other platforms in the future,
> it's better to switch back to the architecture-provided
> get_unmapped_area() function and adjust the needed input parameters
> before the call. Beside fixing the issue, the function now becomes
> easier to understand and maintain.
> 
> This patch has been successfully tested with the io_uring testcase on
> physical x86-64, ppc64le, IA-64 and PA-RISC machines. On PA-RISC the LTP
> mmmap testcases did not report any regressions.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> Cc: Jens Axboe <axboe@kernel.dk>
> Reported-by: matoro <matoro_mailinglist_kernel@matoro.tk>
> Fixes: d808459b2e31 ("io_uring: Adjust mapping wrt architecture aliasing requirements")

Tested-by: Jiri Slaby <jirislaby@kernel.org>

thanks,
-- 
js
suse labs