Skip to contentSkip to conversation

mm/mremap: Add comment explaining the untagging behaviour of mremap()

PATCH Applied Will Deacon 3 participants 4 messages Mar 25, 2020 – Mar 26, 2020

Subsystems (2)
Memory management Memory mapping
lore
Share & tools
lore

Will Deacon

author
#0 lore raw

PATCH 1/1 mm/mremap: Add comment explaining the untagging behaviour of mremap()

Commit dcde237319e6 ("mm: Avoid creating virtual address aliases in brk()/mmap()/mremap()") changed mremap() so that only the 'old' address is untagged, leaving the 'new' address in the form it was passed from userspace. This prevents the unexpected creation of aliasing virtual mappings in userspace, but looks a bit odd when you read the code.

Add a comment justifying the untagging behaviour in mremap().

Cc Catalin Marinas <[email protected]>
Reported-by Linus Torvalds <[email protected]>
Signed-off-by Will Deacon <[email protected]>
---
 mm/mremap.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
mm/mremap.c +10 -0
index af363063ea23..d28f08a36b96 100644
@@ -606,6 +606,16 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
 	LIST_HEAD(uf_unmap_early);
 	LIST_HEAD(uf_unmap);
 
+	/*
+	 * There is a deliberate asymmetry here: we strip the pointer tag
+	 * from the old address but leave the new address alone. This is
+	 * for consistency with mmap(), where we prevent the creation of
+	 * aliasing mappings in userspace by leaving the tag bits of the
+	 * mapping address intact. A non-zero tag will cause the subsequent
+	 * range checks to reject the address as invalid.
+	 *
+	 * See Documentation/arm64/tagged-address-abi.rst for more information.
+	 */
 	addr = untagged_addr(addr);
 
 	if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE))
Signature · 1 line
2.25.1.696.g5e7596f4ac-goog
Reviewed-by on v1 1/1
#1 lore raw
Quoted 11 lines · Will Deacon

Commit dcde237319e6 ("mm: Avoid creating virtual address aliases in brk()/mmap()/mremap()") changed mremap() so that only the 'old' address is untagged, leaving the 'new' address in the form it was passed from userspace. This prevents the unexpected creation of aliasing virtual mappings in userspace, but looks a bit odd when you read the code.

Add a comment justifying the untagging behaviour in mremap().

Cc Catalin Marinas <[email protected]>
Reported-by Linus Torvalds <[email protected]>
Signed-off-by Will Deacon <[email protected]>
Reviewed-by Catalin Marinas <[email protected]>
#2 lore raw
In reply to Will Deacon
Quoted 2 lines · Will Deacon

Add a comment justifying the untagging behaviour in mremap().

Ack. This makes that odd "do it on one address, not the other" thing clear.

Otherwise I bet somebody would come along in a few years and go "Oh, people missed that other case".

Linus

Applied
#3 lore raw
In reply to Will Deacon
Quoted 11 lines · Will Deacon

Commit dcde237319e6 ("mm: Avoid creating virtual address aliases in brk()/mmap()/mremap()") changed mremap() so that only the 'old' address is untagged, leaving the 'new' address in the form it was passed from userspace. This prevents the unexpected creation of aliasing virtual mappings in userspace, but looks a bit odd when you read the code.

Add a comment justifying the untagging behaviour in mremap().

Cc Catalin Marinas <[email protected]>
Reported-by Linus Torvalds <[email protected]>
Signed-off-by Will Deacon <[email protected]>

I queued this patch via the arm64 tree (for 5.7-rc1). Thanks.

Signature · 1 line
Catalin