[PATCH] mm: Fix access_remote_vm() regression on tagged addresses

[PATCH] mm: Fix access_remote_vm() regression on tagged addresses

[Kirill A. Shutemov]

GDB uses /proc/PID/mem to access memory of the target process. GDB
doesn't untag addresses manually, but relies on kernel to do the right
thing.

mem_rw() of procfs uses access_remote_vm() to get data from the target
process. It worked fine until recent changes in __access_remote_vm()
that now checks if there's VMA at target address using raw address.

Untag the address before looking up the VMA.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reported-by: Christina Schimpe <christina.schimpe@intel.com>
Fixes: eee9c708cc89 ("gup: avoid stack expansion warning for known-good case")
Cc: stable@vger.kernel.org
---
 mm/memory.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/memory.c b/mm/memory.c
index 01f39e8144ef..3be9db30db32 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5701,6 +5701,9 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
 	if (mmap_read_lock_killable(mm))
 		return 0;
 
+	/* Untag the address before looking up the VMA */
+	addr = untagged_addr_remote(mm, addr);
+
 	/* Avoid triggering the temporary warning in __get_user_pages */
 	if (!vma_lookup(mm, addr) && !expand_stack(mm, addr))
 		return 0;
-- 
2.41.0

Re: [PATCH] mm: Fix access_remote_vm() regression on tagged addresses

[Linus Torvalds]

On Wed, 9 Aug 2023 at 07:46, Kirill A. Shutemov
<kirill.shutemov@linux.intel.com> wrote:
>
> mem_rw() of procfs uses access_remote_vm() to get data from the target
> process. It worked fine until recent changes in __access_remote_vm()
> that now checks if there's VMA at target address using raw address.
>
> Untag the address before looking up the VMA.

Interesting that it took this long to notice.

Not surprising considering that LAM isn't actually available, but I'd
have expected the arm people to notice more. Yes, I have (and test) my
arm64 laptop, but I obviously don't do user space debugging on it.
Apparently others don't either.

Or maybe TBI is used a lot less than I thought.

Anyway, obviously applied,

            Linus

RE: [PATCH] mm: Fix access_remote_vm() regression on tagged addresses

[Schimpe, Christina]

> Interesting that it took this long to notice.
> 
> Not surprising considering that LAM isn't actually available, but I'd have
> expected the arm people to notice more. Yes, I have (and test) my
> arm64 laptop, but I obviously don't do user space debugging on it.
> Apparently others don't either.
> 
> Or maybe TBI is used a lot less than I thought.

Just for the record:

We don't have any LAM support in GDB yet, we are just working on it.
We currently rely on that feature, but could still change it. We don't 
necessarily require /proc/PID/mem to support tagged addresses.

ARM's TBI support in GDB does not rely on /proc/PID/mem to support tagged
addresses AFAIK.
I also thought that the kernel does not support tagged addresses for
/proc/PID/mem in case of ARM. This is at least reflected by their patches
for TBI and the kernel docs
https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt.

Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] mm: Fix access_remote_vm() regression on tagged addresses

[Linus Torvalds]

On Thu, 10 Aug 2023 at 05:42, Schimpe, Christina
<christina.schimpe@intel.com> wrote:
>
> We don't have any LAM support in GDB yet, we are just working on it.
> We currently rely on that feature, but could still change it. We don't
> necessarily require /proc/PID/mem to support tagged addresses.
>
> ARM's TBI support in GDB does not rely on /proc/PID/mem to support tagged
> addresses AFAIK.

Ahh. That would explain why nobody noticed.

I do wonder if perhaps /proc/<pid>/mem should just match the real
addresses (ie the ones you would see in /proc/<pid>/maps).

The main reason GUP does the untagging is that obviously people will
pass in their own virtual addresses when doing direct-IO etc.

So /proc/<pid>/mem is a bit different.

That said, untagging does make some things easier, so I think it's
probably the right thing to do.

             Linus