All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] userfaultfd: do not untag user pointers
@ 2021-07-02 22:57 ` Peter Collingbourne
  0 siblings, 0 replies; 19+ messages in thread
From: Peter Collingbourne @ 2021-07-02 22:57 UTC (permalink / raw)
  To: Catalin Marinas, Vincenzo Frascino, Dave Martin, Will Deacon,
	Andrew Morton, Andrea Arcangeli
  Cc: Peter Collingbourne, Alistair Delva, Lokesh Gidra,
	William McVicker, Evgenii Stepanov, Mitch Phillips, Linux ARM,
	linux-mm, Andrey Konovalov

If a user program uses userfaultfd on ranges of heap memory, it may
end up passing a tagged pointer to the kernel in the range.start
field of the UFFDIO_REGISTER ioctl. This can happen when using an
MTE-capable allocator, or on Android if using the Tagged Pointers
feature for MTE readiness [1].

When a fault subsequently occurs, the tag is stripped from the fault
address returned to the application in the fault.address field
of struct uffd_msg. However, from the application's perspective,
the tagged address *is* the memory address, so if the application
is unaware of memory tags, it may get confused by receiving an
address that is, from its point of view, outside of the bounds of the
allocation. We observed this behavior in the kselftest for userfaultfd
[2] but other applications could have the same problem.

Address this by not untagging pointers passed to the userfaultfd
ioctls. Instead, let the system call fail. Also change the kselftest
to use mmap so that it doesn't encounter this problem.

[1] https://source.android.com/devices/tech/debug/tagged-pointers
[2] tools/testing/selftests/vm/userfaultfd.c

Peter Collingbourne (2):
  userfaultfd: do not untag user pointers
  selftest: use mmap instead of posix_memalign to allocate memory

 Documentation/arm64/tagged-address-abi.rst | 25 +++++++++++++++-------
 fs/userfaultfd.c                           | 22 +++++++++----------
 tools/testing/selftests/vm/userfaultfd.c   |  6 ++++--
 3 files changed, 31 insertions(+), 22 deletions(-)

-- 
2.32.0.93.g670b81a890-goog



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 0/2] userfaultfd: do not untag user pointers
@ 2021-07-02 22:57 ` Peter Collingbourne
  0 siblings, 0 replies; 19+ messages in thread
From: Peter Collingbourne @ 2021-07-02 22:57 UTC (permalink / raw)
  To: Catalin Marinas, Vincenzo Frascino, Dave Martin, Will Deacon,
	Andrew Morton, Andrea Arcangeli
  Cc: Peter Collingbourne, Alistair Delva, Lokesh Gidra,
	William McVicker, Evgenii Stepanov, Mitch Phillips, Linux ARM,
	linux-mm, Andrey Konovalov

If a user program uses userfaultfd on ranges of heap memory, it may
end up passing a tagged pointer to the kernel in the range.start
field of the UFFDIO_REGISTER ioctl. This can happen when using an
MTE-capable allocator, or on Android if using the Tagged Pointers
feature for MTE readiness [1].

When a fault subsequently occurs, the tag is stripped from the fault
address returned to the application in the fault.address field
of struct uffd_msg. However, from the application's perspective,
the tagged address *is* the memory address, so if the application
is unaware of memory tags, it may get confused by receiving an
address that is, from its point of view, outside of the bounds of the
allocation. We observed this behavior in the kselftest for userfaultfd
[2] but other applications could have the same problem.

Address this by not untagging pointers passed to the userfaultfd
ioctls. Instead, let the system call fail. Also change the kselftest
to use mmap so that it doesn't encounter this problem.

[1] https://source.android.com/devices/tech/debug/tagged-pointers
[2] tools/testing/selftests/vm/userfaultfd.c

Peter Collingbourne (2):
  userfaultfd: do not untag user pointers
  selftest: use mmap instead of posix_memalign to allocate memory

 Documentation/arm64/tagged-address-abi.rst | 25 +++++++++++++++-------
 fs/userfaultfd.c                           | 22 +++++++++----------
 tools/testing/selftests/vm/userfaultfd.c   |  6 ++++--
 3 files changed, 31 insertions(+), 22 deletions(-)

-- 
2.32.0.93.g670b81a890-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 1/2] userfaultfd: do not untag user pointers
  2021-07-02 22:57 ` Peter Collingbourne
  (?)
@ 2021-07-02 22:57   ` Peter Collingbourne
  -1 siblings, 0 replies; 19+ messages in thread
From: Peter Collingbourne @ 2021-07-02 22:57 UTC (permalink / raw)
  To: Catalin Marinas, Vincenzo Frascino, Dave Martin, Will Deacon,
	Andrew Morton, Andrea Arcangeli
  Cc: Peter Collingbourne, Alistair Delva, Lokesh Gidra,
	William McVicker, Evgenii Stepanov, Mitch Phillips, Linux ARM,
	linux-mm, Andrey Konovalov, stable

If a user program uses userfaultfd on ranges of heap memory, it may
end up passing a tagged pointer to the kernel in the range.start
field of the UFFDIO_REGISTER ioctl. This can happen when using an
MTE-capable allocator, or on Android if using the Tagged Pointers
feature for MTE readiness [1].

When a fault subsequently occurs, the tag is stripped from the fault
address returned to the application in the fault.address field
of struct uffd_msg. However, from the application's perspective,
the tagged address *is* the memory address, so if the application
is unaware of memory tags, it may get confused by receiving an
address that is, from its point of view, outside of the bounds of the
allocation. We observed this behavior in the kselftest for userfaultfd
[2] but other applications could have the same problem.

Address this by not untagging pointers passed to the userfaultfd
ioctls. Instead, let the system call fail. This will provide an
early indication of problems with tag-unaware userspace code instead
of letting the code get confused later, and is consistent with how
we decided to handle brk/mmap/mremap in commit dcde237319e6 ("mm:
Avoid creating virtual address aliases in brk()/mmap()/mremap()"),
as well as being consistent with the existing tagged address ABI
documentation relating to how ioctl arguments are handled.

The code change is a revert of commit 7d0325749a6c ("userfaultfd:
untag user pointers").

[1] https://source.android.com/devices/tech/debug/tagged-pointers
[2] tools/testing/selftests/vm/userfaultfd.c

Signed-off-by: Peter Collingbourne <pcc@google.com>
Link: https://linux-review.googlesource.com/id/I761aa9f0344454c482b83fcfcce547db0a25501b
Fixes: 63f0c6037965 ("arm64: Introduce prctl() options to control the tagged user addresses ABI")
Cc: <stable@vger.kernel.org> # 5.4
---
 Documentation/arm64/tagged-address-abi.rst | 25 +++++++++++++++-------
 fs/userfaultfd.c                           | 22 +++++++++----------
 2 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/Documentation/arm64/tagged-address-abi.rst b/Documentation/arm64/tagged-address-abi.rst
index 459e6b66ff68..737f9d8565a2 100644
--- a/Documentation/arm64/tagged-address-abi.rst
+++ b/Documentation/arm64/tagged-address-abi.rst
@@ -45,14 +45,23 @@ how the user addresses are used by the kernel:
 
 1. User addresses not accessed by the kernel but used for address space
    management (e.g. ``mprotect()``, ``madvise()``). The use of valid
-   tagged pointers in this context is allowed with the exception of
-   ``brk()``, ``mmap()`` and the ``new_address`` argument to
-   ``mremap()`` as these have the potential to alias with existing
-   user addresses.
-
-   NOTE: This behaviour changed in v5.6 and so some earlier kernels may
-   incorrectly accept valid tagged pointers for the ``brk()``,
-   ``mmap()`` and ``mremap()`` system calls.
+   tagged pointers in this context is allowed with these exceptions:
+
+   - ``brk()``, ``mmap()`` and the ``new_address`` argument to
+     ``mremap()`` as these have the potential to alias with existing
+      user addresses.
+
+     NOTE: This behaviour changed in v5.6 and so some earlier kernels may
+     incorrectly accept valid tagged pointers for the ``brk()``,
+     ``mmap()`` and ``mremap()`` system calls.
+
+   - The ``range.start`` argument to the ``UFFDIO_REGISTER`` ``ioctl()``
+     used on a file descriptor obtained from ``userfaultfd()``, as
+     fault addresses subsequently obtained by reading the file descriptor
+     will be untagged, which may otherwise confuse tag-unaware programs.
+
+     NOTE: This behaviour changed in v5.14 and so some earlier kernels may
+     incorrectly accept valid tagged pointers for this system call.
 
 2. User addresses accessed by the kernel (e.g. ``write()``). This ABI
    relaxation is disabled by default and the application thread needs to
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index dd7a6c62b56f..7613efe002c1 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1236,23 +1236,21 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx,
 }
 
 static __always_inline int validate_range(struct mm_struct *mm,
-					  __u64 *start, __u64 len)
+					  __u64 start, __u64 len)
 {
 	__u64 task_size = mm->task_size;
 
-	*start = untagged_addr(*start);
-
-	if (*start & ~PAGE_MASK)
+	if (start & ~PAGE_MASK)
 		return -EINVAL;
 	if (len & ~PAGE_MASK)
 		return -EINVAL;
 	if (!len)
 		return -EINVAL;
-	if (*start < mmap_min_addr)
+	if (start < mmap_min_addr)
 		return -EINVAL;
-	if (*start >= task_size)
+	if (start >= task_size)
 		return -EINVAL;
-	if (len > task_size - *start)
+	if (len > task_size - start)
 		return -EINVAL;
 	return 0;
 }
@@ -1313,7 +1311,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
 		vm_flags |= VM_UFFD_MINOR;
 	}
 
-	ret = validate_range(mm, &uffdio_register.range.start,
+	ret = validate_range(mm, uffdio_register.range.start,
 			     uffdio_register.range.len);
 	if (ret)
 		goto out;
@@ -1519,7 +1517,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
 	if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister)))
 		goto out;
 
-	ret = validate_range(mm, &uffdio_unregister.start,
+	ret = validate_range(mm, uffdio_unregister.start,
 			     uffdio_unregister.len);
 	if (ret)
 		goto out;
@@ -1668,7 +1666,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
 	if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake)))
 		goto out;
 
-	ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len);
+	ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len);
 	if (ret)
 		goto out;
 
@@ -1708,7 +1706,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
 			   sizeof(uffdio_copy)-sizeof(__s64)))
 		goto out;
 
-	ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len);
+	ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len);
 	if (ret)
 		goto out;
 	/*
@@ -1765,7 +1763,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
 			   sizeof(uffdio_zeropage)-sizeof(__s64)))
 		goto out;
 
-	ret = validate_range(ctx->mm, &uffdio_zeropage.range.start,
+	ret = validate_range(ctx->mm, uffdio_zeropage.range.start,
 			     uffdio_zeropage.range.len);
 	if (ret)
 		goto out;
-- 
2.32.0.93.g670b81a890-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 1/2] userfaultfd: do not untag user pointers
@ 2021-07-02 22:57   ` Peter Collingbourne
  0 siblings, 0 replies; 19+ messages in thread
From: Peter Collingbourne @ 2021-07-02 22:57 UTC (permalink / raw)
  To: Catalin Marinas, Vincenzo Frascino, Dave Martin, Will Deacon,
	Andrew Morton, Andrea Arcangeli
  Cc: Peter Collingbourne, Alistair Delva, Lokesh Gidra,
	William McVicker, Evgenii Stepanov, Mitch Phillips, Linux ARM,
	linux-mm, Andrey Konovalov, stable

If a user program uses userfaultfd on ranges of heap memory, it may
end up passing a tagged pointer to the kernel in the range.start
field of the UFFDIO_REGISTER ioctl. This can happen when using an
MTE-capable allocator, or on Android if using the Tagged Pointers
feature for MTE readiness [1].

When a fault subsequently occurs, the tag is stripped from the fault
address returned to the application in the fault.address field
of struct uffd_msg. However, from the application's perspective,
the tagged address *is* the memory address, so if the application
is unaware of memory tags, it may get confused by receiving an
address that is, from its point of view, outside of the bounds of the
allocation. We observed this behavior in the kselftest for userfaultfd
[2] but other applications could have the same problem.

Address this by not untagging pointers passed to the userfaultfd
ioctls. Instead, let the system call fail. This will provide an
early indication of problems with tag-unaware userspace code instead
of letting the code get confused later, and is consistent with how
we decided to handle brk/mmap/mremap in commit dcde237319e6 ("mm:
Avoid creating virtual address aliases in brk()/mmap()/mremap()"),
as well as being consistent with the existing tagged address ABI
documentation relating to how ioctl arguments are handled.

The code change is a revert of commit 7d0325749a6c ("userfaultfd:
untag user pointers").

[1] https://source.android.com/devices/tech/debug/tagged-pointers
[2] tools/testing/selftests/vm/userfaultfd.c

Signed-off-by: Peter Collingbourne <pcc@google.com>
Link: https://linux-review.googlesource.com/id/I761aa9f0344454c482b83fcfcce547db0a25501b
Fixes: 63f0c6037965 ("arm64: Introduce prctl() options to control the tagged user addresses ABI")
Cc: <stable@vger.kernel.org> # 5.4
---
 Documentation/arm64/tagged-address-abi.rst | 25 +++++++++++++++-------
 fs/userfaultfd.c                           | 22 +++++++++----------
 2 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/Documentation/arm64/tagged-address-abi.rst b/Documentation/arm64/tagged-address-abi.rst
index 459e6b66ff68..737f9d8565a2 100644
--- a/Documentation/arm64/tagged-address-abi.rst
+++ b/Documentation/arm64/tagged-address-abi.rst
@@ -45,14 +45,23 @@ how the user addresses are used by the kernel:
 
 1. User addresses not accessed by the kernel but used for address space
    management (e.g. ``mprotect()``, ``madvise()``). The use of valid
-   tagged pointers in this context is allowed with the exception of
-   ``brk()``, ``mmap()`` and the ``new_address`` argument to
-   ``mremap()`` as these have the potential to alias with existing
-   user addresses.
-
-   NOTE: This behaviour changed in v5.6 and so some earlier kernels may
-   incorrectly accept valid tagged pointers for the ``brk()``,
-   ``mmap()`` and ``mremap()`` system calls.
+   tagged pointers in this context is allowed with these exceptions:
+
+   - ``brk()``, ``mmap()`` and the ``new_address`` argument to
+     ``mremap()`` as these have the potential to alias with existing
+      user addresses.
+
+     NOTE: This behaviour changed in v5.6 and so some earlier kernels may
+     incorrectly accept valid tagged pointers for the ``brk()``,
+     ``mmap()`` and ``mremap()`` system calls.
+
+   - The ``range.start`` argument to the ``UFFDIO_REGISTER`` ``ioctl()``
+     used on a file descriptor obtained from ``userfaultfd()``, as
+     fault addresses subsequently obtained by reading the file descriptor
+     will be untagged, which may otherwise confuse tag-unaware programs.
+
+     NOTE: This behaviour changed in v5.14 and so some earlier kernels may
+     incorrectly accept valid tagged pointers for this system call.
 
 2. User addresses accessed by the kernel (e.g. ``write()``). This ABI
    relaxation is disabled by default and the application thread needs to
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index dd7a6c62b56f..7613efe002c1 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1236,23 +1236,21 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx,
 }
 
 static __always_inline int validate_range(struct mm_struct *mm,
-					  __u64 *start, __u64 len)
+					  __u64 start, __u64 len)
 {
 	__u64 task_size = mm->task_size;
 
-	*start = untagged_addr(*start);
-
-	if (*start & ~PAGE_MASK)
+	if (start & ~PAGE_MASK)
 		return -EINVAL;
 	if (len & ~PAGE_MASK)
 		return -EINVAL;
 	if (!len)
 		return -EINVAL;
-	if (*start < mmap_min_addr)
+	if (start < mmap_min_addr)
 		return -EINVAL;
-	if (*start >= task_size)
+	if (start >= task_size)
 		return -EINVAL;
-	if (len > task_size - *start)
+	if (len > task_size - start)
 		return -EINVAL;
 	return 0;
 }
@@ -1313,7 +1311,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
 		vm_flags |= VM_UFFD_MINOR;
 	}
 
-	ret = validate_range(mm, &uffdio_register.range.start,
+	ret = validate_range(mm, uffdio_register.range.start,
 			     uffdio_register.range.len);
 	if (ret)
 		goto out;
@@ -1519,7 +1517,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
 	if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister)))
 		goto out;
 
-	ret = validate_range(mm, &uffdio_unregister.start,
+	ret = validate_range(mm, uffdio_unregister.start,
 			     uffdio_unregister.len);
 	if (ret)
 		goto out;
@@ -1668,7 +1666,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
 	if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake)))
 		goto out;
 
-	ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len);
+	ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len);
 	if (ret)
 		goto out;
 
@@ -1708,7 +1706,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
 			   sizeof(uffdio_copy)-sizeof(__s64)))
 		goto out;
 
-	ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len);
+	ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len);
 	if (ret)
 		goto out;
 	/*
@@ -1765,7 +1763,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
 			   sizeof(uffdio_zeropage)-sizeof(__s64)))
 		goto out;
 
-	ret = validate_range(ctx->mm, &uffdio_zeropage.range.start,
+	ret = validate_range(ctx->mm, uffdio_zeropage.range.start,
 			     uffdio_zeropage.range.len);
 	if (ret)
 		goto out;
-- 
2.32.0.93.g670b81a890-goog



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 1/2] userfaultfd: do not untag user pointers
@ 2021-07-02 22:57   ` Peter Collingbourne
  0 siblings, 0 replies; 19+ messages in thread
From: Peter Collingbourne @ 2021-07-02 22:57 UTC (permalink / raw)
  To: Catalin Marinas, Vincenzo Frascino, Dave Martin, Will Deacon,
	Andrew Morton, Andrea Arcangeli
  Cc: Peter Collingbourne, Alistair Delva, Lokesh Gidra,
	William McVicker, Evgenii Stepanov, Mitch Phillips, Linux ARM,
	linux-mm, Andrey Konovalov, stable

If a user program uses userfaultfd on ranges of heap memory, it may
end up passing a tagged pointer to the kernel in the range.start
field of the UFFDIO_REGISTER ioctl. This can happen when using an
MTE-capable allocator, or on Android if using the Tagged Pointers
feature for MTE readiness [1].

When a fault subsequently occurs, the tag is stripped from the fault
address returned to the application in the fault.address field
of struct uffd_msg. However, from the application's perspective,
the tagged address *is* the memory address, so if the application
is unaware of memory tags, it may get confused by receiving an
address that is, from its point of view, outside of the bounds of the
allocation. We observed this behavior in the kselftest for userfaultfd
[2] but other applications could have the same problem.

Address this by not untagging pointers passed to the userfaultfd
ioctls. Instead, let the system call fail. This will provide an
early indication of problems with tag-unaware userspace code instead
of letting the code get confused later, and is consistent with how
we decided to handle brk/mmap/mremap in commit dcde237319e6 ("mm:
Avoid creating virtual address aliases in brk()/mmap()/mremap()"),
as well as being consistent with the existing tagged address ABI
documentation relating to how ioctl arguments are handled.

The code change is a revert of commit 7d0325749a6c ("userfaultfd:
untag user pointers").

[1] https://source.android.com/devices/tech/debug/tagged-pointers
[2] tools/testing/selftests/vm/userfaultfd.c

Signed-off-by: Peter Collingbourne <pcc@google.com>
Link: https://linux-review.googlesource.com/id/I761aa9f0344454c482b83fcfcce547db0a25501b
Fixes: 63f0c6037965 ("arm64: Introduce prctl() options to control the tagged user addresses ABI")
Cc: <stable@vger.kernel.org> # 5.4
---
 Documentation/arm64/tagged-address-abi.rst | 25 +++++++++++++++-------
 fs/userfaultfd.c                           | 22 +++++++++----------
 2 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/Documentation/arm64/tagged-address-abi.rst b/Documentation/arm64/tagged-address-abi.rst
index 459e6b66ff68..737f9d8565a2 100644
--- a/Documentation/arm64/tagged-address-abi.rst
+++ b/Documentation/arm64/tagged-address-abi.rst
@@ -45,14 +45,23 @@ how the user addresses are used by the kernel:
 
 1. User addresses not accessed by the kernel but used for address space
    management (e.g. ``mprotect()``, ``madvise()``). The use of valid
-   tagged pointers in this context is allowed with the exception of
-   ``brk()``, ``mmap()`` and the ``new_address`` argument to
-   ``mremap()`` as these have the potential to alias with existing
-   user addresses.
-
-   NOTE: This behaviour changed in v5.6 and so some earlier kernels may
-   incorrectly accept valid tagged pointers for the ``brk()``,
-   ``mmap()`` and ``mremap()`` system calls.
+   tagged pointers in this context is allowed with these exceptions:
+
+   - ``brk()``, ``mmap()`` and the ``new_address`` argument to
+     ``mremap()`` as these have the potential to alias with existing
+      user addresses.
+
+     NOTE: This behaviour changed in v5.6 and so some earlier kernels may
+     incorrectly accept valid tagged pointers for the ``brk()``,
+     ``mmap()`` and ``mremap()`` system calls.
+
+   - The ``range.start`` argument to the ``UFFDIO_REGISTER`` ``ioctl()``
+     used on a file descriptor obtained from ``userfaultfd()``, as
+     fault addresses subsequently obtained by reading the file descriptor
+     will be untagged, which may otherwise confuse tag-unaware programs.
+
+     NOTE: This behaviour changed in v5.14 and so some earlier kernels may
+     incorrectly accept valid tagged pointers for this system call.
 
 2. User addresses accessed by the kernel (e.g. ``write()``). This ABI
    relaxation is disabled by default and the application thread needs to
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index dd7a6c62b56f..7613efe002c1 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1236,23 +1236,21 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx,
 }
 
 static __always_inline int validate_range(struct mm_struct *mm,
-					  __u64 *start, __u64 len)
+					  __u64 start, __u64 len)
 {
 	__u64 task_size = mm->task_size;
 
-	*start = untagged_addr(*start);
-
-	if (*start & ~PAGE_MASK)
+	if (start & ~PAGE_MASK)
 		return -EINVAL;
 	if (len & ~PAGE_MASK)
 		return -EINVAL;
 	if (!len)
 		return -EINVAL;
-	if (*start < mmap_min_addr)
+	if (start < mmap_min_addr)
 		return -EINVAL;
-	if (*start >= task_size)
+	if (start >= task_size)
 		return -EINVAL;
-	if (len > task_size - *start)
+	if (len > task_size - start)
 		return -EINVAL;
 	return 0;
 }
@@ -1313,7 +1311,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
 		vm_flags |= VM_UFFD_MINOR;
 	}
 
-	ret = validate_range(mm, &uffdio_register.range.start,
+	ret = validate_range(mm, uffdio_register.range.start,
 			     uffdio_register.range.len);
 	if (ret)
 		goto out;
@@ -1519,7 +1517,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
 	if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister)))
 		goto out;
 
-	ret = validate_range(mm, &uffdio_unregister.start,
+	ret = validate_range(mm, uffdio_unregister.start,
 			     uffdio_unregister.len);
 	if (ret)
 		goto out;
@@ -1668,7 +1666,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
 	if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake)))
 		goto out;
 
-	ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len);
+	ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len);
 	if (ret)
 		goto out;
 
@@ -1708,7 +1706,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
 			   sizeof(uffdio_copy)-sizeof(__s64)))
 		goto out;
 
-	ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len);
+	ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len);
 	if (ret)
 		goto out;
 	/*
@@ -1765,7 +1763,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
 			   sizeof(uffdio_zeropage)-sizeof(__s64)))
 		goto out;
 
-	ret = validate_range(ctx->mm, &uffdio_zeropage.range.start,
+	ret = validate_range(ctx->mm, uffdio_zeropage.range.start,
 			     uffdio_zeropage.range.len);
 	if (ret)
 		goto out;
-- 
2.32.0.93.g670b81a890-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 2/2] selftest: use mmap instead of posix_memalign to allocate memory
  2021-07-02 22:57 ` Peter Collingbourne
  (?)
@ 2021-07-02 22:57   ` Peter Collingbourne
  -1 siblings, 0 replies; 19+ messages in thread
From: Peter Collingbourne @ 2021-07-02 22:57 UTC (permalink / raw)
  To: Catalin Marinas, Vincenzo Frascino, Dave Martin, Will Deacon,
	Andrew Morton, Andrea Arcangeli
  Cc: Peter Collingbourne, Alistair Delva, Lokesh Gidra,
	William McVicker, Evgenii Stepanov, Mitch Phillips, Linux ARM,
	linux-mm, Andrey Konovalov, stable

This test passes pointers obtained from anon_allocate_area to the
userfaultfd and mremap APIs. This causes a problem if the system
allocator returns tagged pointers because with the tagged address ABI
the kernel rejects tagged addresses passed to these APIs, which would
end up causing the test to fail. To make this test compatible with
such system allocators, stop using the system allocator to allocate
memory in anon_allocate_area, and instead just use mmap.

Co-developed-by: Lokesh Gidra <lokeshgidra@google.com>
Signed-off-by: Lokesh Gidra <lokeshgidra@google.com>
Signed-off-by: Peter Collingbourne <pcc@google.com>
Fixes: c47174fc362a ("userfaultfd: selftest")
Cc: <stable@vger.kernel.org> # 5.4
Link: https://linux-review.googlesource.com/id/Icac91064fcd923f77a83e8e133f8631c5b8fc241
---
 tools/testing/selftests/vm/userfaultfd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index f5ab5e0312e7..d0f802053dfd 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -197,8 +197,10 @@ static int anon_release_pages(char *rel_area)
 
 static void anon_allocate_area(void **alloc_area)
 {
-	if (posix_memalign(alloc_area, page_size, nr_pages * page_size)) {
-		fprintf(stderr, "out of memory\n");
+	*alloc_area = mmap(NULL, nr_pages * page_size, PROT_READ | PROT_WRITE,
+			   MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+	if (*alloc_area == MAP_FAILED) {
+		fprintf(stderr, "anon memory mmap failed\n");
 		*alloc_area = NULL;
 	}
 }
-- 
2.32.0.93.g670b81a890-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 2/2] selftest: use mmap instead of posix_memalign to allocate memory
@ 2021-07-02 22:57   ` Peter Collingbourne
  0 siblings, 0 replies; 19+ messages in thread
From: Peter Collingbourne @ 2021-07-02 22:57 UTC (permalink / raw)
  To: Catalin Marinas, Vincenzo Frascino, Dave Martin, Will Deacon,
	Andrew Morton, Andrea Arcangeli
  Cc: Peter Collingbourne, Alistair Delva, Lokesh Gidra,
	William McVicker, Evgenii Stepanov, Mitch Phillips, Linux ARM,
	linux-mm, Andrey Konovalov, stable

This test passes pointers obtained from anon_allocate_area to the
userfaultfd and mremap APIs. This causes a problem if the system
allocator returns tagged pointers because with the tagged address ABI
the kernel rejects tagged addresses passed to these APIs, which would
end up causing the test to fail. To make this test compatible with
such system allocators, stop using the system allocator to allocate
memory in anon_allocate_area, and instead just use mmap.

Co-developed-by: Lokesh Gidra <lokeshgidra@google.com>
Signed-off-by: Lokesh Gidra <lokeshgidra@google.com>
Signed-off-by: Peter Collingbourne <pcc@google.com>
Fixes: c47174fc362a ("userfaultfd: selftest")
Cc: <stable@vger.kernel.org> # 5.4
Link: https://linux-review.googlesource.com/id/Icac91064fcd923f77a83e8e133f8631c5b8fc241
---
 tools/testing/selftests/vm/userfaultfd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index f5ab5e0312e7..d0f802053dfd 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -197,8 +197,10 @@ static int anon_release_pages(char *rel_area)
 
 static void anon_allocate_area(void **alloc_area)
 {
-	if (posix_memalign(alloc_area, page_size, nr_pages * page_size)) {
-		fprintf(stderr, "out of memory\n");
+	*alloc_area = mmap(NULL, nr_pages * page_size, PROT_READ | PROT_WRITE,
+			   MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+	if (*alloc_area == MAP_FAILED) {
+		fprintf(stderr, "anon memory mmap failed\n");
 		*alloc_area = NULL;
 	}
 }
-- 
2.32.0.93.g670b81a890-goog



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 2/2] selftest: use mmap instead of posix_memalign to allocate memory
@ 2021-07-02 22:57   ` Peter Collingbourne
  0 siblings, 0 replies; 19+ messages in thread
From: Peter Collingbourne @ 2021-07-02 22:57 UTC (permalink / raw)
  To: Catalin Marinas, Vincenzo Frascino, Dave Martin, Will Deacon,
	Andrew Morton, Andrea Arcangeli
  Cc: Peter Collingbourne, Alistair Delva, Lokesh Gidra,
	William McVicker, Evgenii Stepanov, Mitch Phillips, Linux ARM,
	linux-mm, Andrey Konovalov, stable

This test passes pointers obtained from anon_allocate_area to the
userfaultfd and mremap APIs. This causes a problem if the system
allocator returns tagged pointers because with the tagged address ABI
the kernel rejects tagged addresses passed to these APIs, which would
end up causing the test to fail. To make this test compatible with
such system allocators, stop using the system allocator to allocate
memory in anon_allocate_area, and instead just use mmap.

Co-developed-by: Lokesh Gidra <lokeshgidra@google.com>
Signed-off-by: Lokesh Gidra <lokeshgidra@google.com>
Signed-off-by: Peter Collingbourne <pcc@google.com>
Fixes: c47174fc362a ("userfaultfd: selftest")
Cc: <stable@vger.kernel.org> # 5.4
Link: https://linux-review.googlesource.com/id/Icac91064fcd923f77a83e8e133f8631c5b8fc241
---
 tools/testing/selftests/vm/userfaultfd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index f5ab5e0312e7..d0f802053dfd 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -197,8 +197,10 @@ static int anon_release_pages(char *rel_area)
 
 static void anon_allocate_area(void **alloc_area)
 {
-	if (posix_memalign(alloc_area, page_size, nr_pages * page_size)) {
-		fprintf(stderr, "out of memory\n");
+	*alloc_area = mmap(NULL, nr_pages * page_size, PROT_READ | PROT_WRITE,
+			   MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+	if (*alloc_area == MAP_FAILED) {
+		fprintf(stderr, "anon memory mmap failed\n");
 		*alloc_area = NULL;
 	}
 }
-- 
2.32.0.93.g670b81a890-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
  2021-07-02 22:57   ` Peter Collingbourne
@ 2021-07-03  2:46     ` kernel test robot
  -1 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-07-03  2:46 UTC (permalink / raw)
  To: Peter Collingbourne, Catalin Marinas, Vincenzo Frascino,
	Dave Martin, Will Deacon, Andrew Morton, Andrea Arcangeli
  Cc: kbuild-all, Linux Memory Management List, Peter Collingbourne,
	Alistair Delva, Lokesh Gidra

[-- Attachment #1: Type: text/plain, Size: 5836 bytes --]

Hi Peter,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on kselftest/next linus/master v5.13 next-20210701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: x86_64-randconfig-s032-20210702 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/42177546697fa573571799dc11ecd12a65449886
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
        git checkout 42177546697fa573571799dc11ecd12a65449886
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> fs/userfaultfd.c:1816:40: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned long long [usertype] start @@     got unsigned long long * @@
   fs/userfaultfd.c:1816:40: sparse:     expected unsigned long long [usertype] start
   fs/userfaultfd.c:1816:40: sparse:     got unsigned long long *
   fs/userfaultfd.c:1864:40: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned long long [usertype] start @@     got unsigned long long * @@
   fs/userfaultfd.c:1864:40: sparse:     expected unsigned long long [usertype] start
   fs/userfaultfd.c:1864:40: sparse:     got unsigned long long *

vim +1816 fs/userfaultfd.c

ad465cae96b456 Andrea Arcangeli 2015-09-04  1797  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1798  static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1799  				    unsigned long arg)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1800  {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1801  	int ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1802  	struct uffdio_writeprotect uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1803  	struct uffdio_writeprotect __user *user_uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1804  	struct userfaultfd_wake_range range;
23080e2783ba45 Peter Xu         2020-04-06  1805  	bool mode_wp, mode_dontwake;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1806  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1807  	if (READ_ONCE(ctx->mmap_changing))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1808  		return -EAGAIN;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1809  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1810  	user_uffdio_wp = (struct uffdio_writeprotect __user *) arg;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1811  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1812  	if (copy_from_user(&uffdio_wp, user_uffdio_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1813  			   sizeof(struct uffdio_writeprotect)))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1814  		return -EFAULT;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1815  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06 @1816  	ret = validate_range(ctx->mm, &uffdio_wp.range.start,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1817  			     uffdio_wp.range.len);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1818  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1819  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1820  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1821  	if (uffdio_wp.mode & ~(UFFDIO_WRITEPROTECT_MODE_DONTWAKE |
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1822  			       UFFDIO_WRITEPROTECT_MODE_WP))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1823  		return -EINVAL;
23080e2783ba45 Peter Xu         2020-04-06  1824  
23080e2783ba45 Peter Xu         2020-04-06  1825  	mode_wp = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_WP;
23080e2783ba45 Peter Xu         2020-04-06  1826  	mode_dontwake = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_DONTWAKE;
23080e2783ba45 Peter Xu         2020-04-06  1827  
23080e2783ba45 Peter Xu         2020-04-06  1828  	if (mode_wp && mode_dontwake)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1829  		return -EINVAL;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1830  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1831  	ret = mwriteprotect_range(ctx->mm, uffdio_wp.range.start,
23080e2783ba45 Peter Xu         2020-04-06  1832  				  uffdio_wp.range.len, mode_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1833  				  &ctx->mmap_changing);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1834  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1835  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1836  
23080e2783ba45 Peter Xu         2020-04-06  1837  	if (!mode_wp && !mode_dontwake) {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1838  		range.start = uffdio_wp.range.start;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1839  		range.len = uffdio_wp.range.len;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1840  		wake_userfault(ctx, &range);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1841  	}
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1842  	return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1843  }
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1844  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 41548 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
@ 2021-07-03  2:46     ` kernel test robot
  0 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-07-03  2:46 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 5928 bytes --]

Hi Peter,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on kselftest/next linus/master v5.13 next-20210701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: x86_64-randconfig-s032-20210702 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/42177546697fa573571799dc11ecd12a65449886
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
        git checkout 42177546697fa573571799dc11ecd12a65449886
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> fs/userfaultfd.c:1816:40: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned long long [usertype] start @@     got unsigned long long * @@
   fs/userfaultfd.c:1816:40: sparse:     expected unsigned long long [usertype] start
   fs/userfaultfd.c:1816:40: sparse:     got unsigned long long *
   fs/userfaultfd.c:1864:40: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned long long [usertype] start @@     got unsigned long long * @@
   fs/userfaultfd.c:1864:40: sparse:     expected unsigned long long [usertype] start
   fs/userfaultfd.c:1864:40: sparse:     got unsigned long long *

vim +1816 fs/userfaultfd.c

ad465cae96b456 Andrea Arcangeli 2015-09-04  1797  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1798  static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1799  				    unsigned long arg)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1800  {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1801  	int ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1802  	struct uffdio_writeprotect uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1803  	struct uffdio_writeprotect __user *user_uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1804  	struct userfaultfd_wake_range range;
23080e2783ba45 Peter Xu         2020-04-06  1805  	bool mode_wp, mode_dontwake;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1806  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1807  	if (READ_ONCE(ctx->mmap_changing))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1808  		return -EAGAIN;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1809  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1810  	user_uffdio_wp = (struct uffdio_writeprotect __user *) arg;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1811  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1812  	if (copy_from_user(&uffdio_wp, user_uffdio_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1813  			   sizeof(struct uffdio_writeprotect)))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1814  		return -EFAULT;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1815  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06 @1816  	ret = validate_range(ctx->mm, &uffdio_wp.range.start,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1817  			     uffdio_wp.range.len);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1818  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1819  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1820  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1821  	if (uffdio_wp.mode & ~(UFFDIO_WRITEPROTECT_MODE_DONTWAKE |
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1822  			       UFFDIO_WRITEPROTECT_MODE_WP))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1823  		return -EINVAL;
23080e2783ba45 Peter Xu         2020-04-06  1824  
23080e2783ba45 Peter Xu         2020-04-06  1825  	mode_wp = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_WP;
23080e2783ba45 Peter Xu         2020-04-06  1826  	mode_dontwake = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_DONTWAKE;
23080e2783ba45 Peter Xu         2020-04-06  1827  
23080e2783ba45 Peter Xu         2020-04-06  1828  	if (mode_wp && mode_dontwake)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1829  		return -EINVAL;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1830  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1831  	ret = mwriteprotect_range(ctx->mm, uffdio_wp.range.start,
23080e2783ba45 Peter Xu         2020-04-06  1832  				  uffdio_wp.range.len, mode_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1833  				  &ctx->mmap_changing);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1834  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1835  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1836  
23080e2783ba45 Peter Xu         2020-04-06  1837  	if (!mode_wp && !mode_dontwake) {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1838  		range.start = uffdio_wp.range.start;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1839  		range.len = uffdio_wp.range.len;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1840  		wake_userfault(ctx, &range);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1841  	}
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1842  	return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1843  }
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1844  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 41548 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
  2021-07-02 22:57   ` Peter Collingbourne
@ 2021-07-03  3:06     ` kernel test robot
  -1 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-07-03  3:06 UTC (permalink / raw)
  To: Peter Collingbourne, Catalin Marinas, Vincenzo Frascino,
	Dave Martin, Will Deacon, Andrew Morton, Andrea Arcangeli
  Cc: clang-built-linux, kbuild-all, Linux Memory Management List,
	Peter Collingbourne, Alistair Delva, Lokesh Gidra

[-- Attachment #1: Type: text/plain, Size: 6546 bytes --]

Hi Peter,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on kselftest/next linus/master v5.13 next-20210701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: x86_64-randconfig-r034-20210702 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project cb5de7c813f976dd458bd2a7f40702ba648bf650)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/42177546697fa573571799dc11ecd12a65449886
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
        git checkout 42177546697fa573571799dc11ecd12a65449886
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/userfaultfd.c:1816:32: warning: incompatible pointer to integer conversion passing '__u64 *' (aka 'unsigned long long *') to parameter of type '__u64' (aka 'unsigned long long'); remove & [-Wint-conversion]
           ret = validate_range(ctx->mm, &uffdio_wp.range.start,
                                         ^~~~~~~~~~~~~~~~~~~~~~
   fs/userfaultfd.c:1239:14: note: passing argument to parameter 'start' here
                                             __u64 start, __u64 len)
                                                   ^
   fs/userfaultfd.c:1864:32: warning: incompatible pointer to integer conversion passing '__u64 *' (aka 'unsigned long long *') to parameter of type '__u64' (aka 'unsigned long long'); remove & [-Wint-conversion]
           ret = validate_range(ctx->mm, &uffdio_continue.range.start,
                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/userfaultfd.c:1239:14: note: passing argument to parameter 'start' here
                                             __u64 start, __u64 len)
                                                   ^
   2 warnings generated.


vim +1816 fs/userfaultfd.c

ad465cae96b456 Andrea Arcangeli 2015-09-04  1797  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1798  static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1799  				    unsigned long arg)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1800  {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1801  	int ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1802  	struct uffdio_writeprotect uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1803  	struct uffdio_writeprotect __user *user_uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1804  	struct userfaultfd_wake_range range;
23080e2783ba45 Peter Xu         2020-04-06  1805  	bool mode_wp, mode_dontwake;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1806  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1807  	if (READ_ONCE(ctx->mmap_changing))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1808  		return -EAGAIN;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1809  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1810  	user_uffdio_wp = (struct uffdio_writeprotect __user *) arg;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1811  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1812  	if (copy_from_user(&uffdio_wp, user_uffdio_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1813  			   sizeof(struct uffdio_writeprotect)))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1814  		return -EFAULT;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1815  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06 @1816  	ret = validate_range(ctx->mm, &uffdio_wp.range.start,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1817  			     uffdio_wp.range.len);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1818  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1819  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1820  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1821  	if (uffdio_wp.mode & ~(UFFDIO_WRITEPROTECT_MODE_DONTWAKE |
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1822  			       UFFDIO_WRITEPROTECT_MODE_WP))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1823  		return -EINVAL;
23080e2783ba45 Peter Xu         2020-04-06  1824  
23080e2783ba45 Peter Xu         2020-04-06  1825  	mode_wp = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_WP;
23080e2783ba45 Peter Xu         2020-04-06  1826  	mode_dontwake = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_DONTWAKE;
23080e2783ba45 Peter Xu         2020-04-06  1827  
23080e2783ba45 Peter Xu         2020-04-06  1828  	if (mode_wp && mode_dontwake)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1829  		return -EINVAL;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1830  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1831  	ret = mwriteprotect_range(ctx->mm, uffdio_wp.range.start,
23080e2783ba45 Peter Xu         2020-04-06  1832  				  uffdio_wp.range.len, mode_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1833  				  &ctx->mmap_changing);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1834  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1835  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1836  
23080e2783ba45 Peter Xu         2020-04-06  1837  	if (!mode_wp && !mode_dontwake) {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1838  		range.start = uffdio_wp.range.start;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1839  		range.len = uffdio_wp.range.len;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1840  		wake_userfault(ctx, &range);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1841  	}
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1842  	return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1843  }
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1844  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 42286 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
@ 2021-07-03  3:06     ` kernel test robot
  0 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-07-03  3:06 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 6648 bytes --]

Hi Peter,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on kselftest/next linus/master v5.13 next-20210701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: x86_64-randconfig-r034-20210702 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project cb5de7c813f976dd458bd2a7f40702ba648bf650)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/42177546697fa573571799dc11ecd12a65449886
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
        git checkout 42177546697fa573571799dc11ecd12a65449886
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/userfaultfd.c:1816:32: warning: incompatible pointer to integer conversion passing '__u64 *' (aka 'unsigned long long *') to parameter of type '__u64' (aka 'unsigned long long'); remove & [-Wint-conversion]
           ret = validate_range(ctx->mm, &uffdio_wp.range.start,
                                         ^~~~~~~~~~~~~~~~~~~~~~
   fs/userfaultfd.c:1239:14: note: passing argument to parameter 'start' here
                                             __u64 start, __u64 len)
                                                   ^
   fs/userfaultfd.c:1864:32: warning: incompatible pointer to integer conversion passing '__u64 *' (aka 'unsigned long long *') to parameter of type '__u64' (aka 'unsigned long long'); remove & [-Wint-conversion]
           ret = validate_range(ctx->mm, &uffdio_continue.range.start,
                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/userfaultfd.c:1239:14: note: passing argument to parameter 'start' here
                                             __u64 start, __u64 len)
                                                   ^
   2 warnings generated.


vim +1816 fs/userfaultfd.c

ad465cae96b456 Andrea Arcangeli 2015-09-04  1797  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1798  static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1799  				    unsigned long arg)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1800  {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1801  	int ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1802  	struct uffdio_writeprotect uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1803  	struct uffdio_writeprotect __user *user_uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1804  	struct userfaultfd_wake_range range;
23080e2783ba45 Peter Xu         2020-04-06  1805  	bool mode_wp, mode_dontwake;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1806  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1807  	if (READ_ONCE(ctx->mmap_changing))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1808  		return -EAGAIN;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1809  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1810  	user_uffdio_wp = (struct uffdio_writeprotect __user *) arg;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1811  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1812  	if (copy_from_user(&uffdio_wp, user_uffdio_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1813  			   sizeof(struct uffdio_writeprotect)))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1814  		return -EFAULT;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1815  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06 @1816  	ret = validate_range(ctx->mm, &uffdio_wp.range.start,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1817  			     uffdio_wp.range.len);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1818  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1819  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1820  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1821  	if (uffdio_wp.mode & ~(UFFDIO_WRITEPROTECT_MODE_DONTWAKE |
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1822  			       UFFDIO_WRITEPROTECT_MODE_WP))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1823  		return -EINVAL;
23080e2783ba45 Peter Xu         2020-04-06  1824  
23080e2783ba45 Peter Xu         2020-04-06  1825  	mode_wp = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_WP;
23080e2783ba45 Peter Xu         2020-04-06  1826  	mode_dontwake = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_DONTWAKE;
23080e2783ba45 Peter Xu         2020-04-06  1827  
23080e2783ba45 Peter Xu         2020-04-06  1828  	if (mode_wp && mode_dontwake)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1829  		return -EINVAL;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1830  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1831  	ret = mwriteprotect_range(ctx->mm, uffdio_wp.range.start,
23080e2783ba45 Peter Xu         2020-04-06  1832  				  uffdio_wp.range.len, mode_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1833  				  &ctx->mmap_changing);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1834  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1835  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1836  
23080e2783ba45 Peter Xu         2020-04-06  1837  	if (!mode_wp && !mode_dontwake) {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1838  		range.start = uffdio_wp.range.start;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1839  		range.len = uffdio_wp.range.len;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1840  		wake_userfault(ctx, &range);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1841  	}
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1842  	return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1843  }
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1844  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 42286 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
  2021-07-02 22:57   ` Peter Collingbourne
@ 2021-07-03  5:02     ` kernel test robot
  -1 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-07-03  5:02 UTC (permalink / raw)
  To: Peter Collingbourne, Catalin Marinas, Vincenzo Frascino,
	Dave Martin, Will Deacon, Andrew Morton, Andrea Arcangeli
  Cc: kbuild-all, Linux Memory Management List, Peter Collingbourne,
	Alistair Delva, Lokesh Gidra

[-- Attachment #1: Type: text/plain, Size: 6624 bytes --]

Hi Peter,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on kselftest/next linus/master v5.13 next-20210701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: openrisc-randconfig-c004-20210702 (attached as .config)
compiler: or1k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/42177546697fa573571799dc11ecd12a65449886
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
        git checkout 42177546697fa573571799dc11ecd12a65449886
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=openrisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   fs/userfaultfd.c: In function 'userfaultfd_writeprotect':
>> fs/userfaultfd.c:1816:32: warning: passing argument 2 of 'validate_range' makes integer from pointer without a cast [-Wint-conversion]
    1816 |  ret = validate_range(ctx->mm, &uffdio_wp.range.start,
         |                                ^~~~~~~~~~~~~~~~~~~~~~
         |                                |
         |                                __u64 * {aka long long unsigned int *}
   fs/userfaultfd.c:1239:14: note: expected '__u64' {aka 'long long unsigned int'} but argument is of type '__u64 *' {aka 'long long unsigned int *'}
    1239 |        __u64 start, __u64 len)
         |        ~~~~~~^~~~~
   fs/userfaultfd.c: In function 'userfaultfd_continue':
   fs/userfaultfd.c:1864:32: warning: passing argument 2 of 'validate_range' makes integer from pointer without a cast [-Wint-conversion]
    1864 |  ret = validate_range(ctx->mm, &uffdio_continue.range.start,
         |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                                |
         |                                __u64 * {aka long long unsigned int *}
   fs/userfaultfd.c:1239:14: note: expected '__u64' {aka 'long long unsigned int'} but argument is of type '__u64 *' {aka 'long long unsigned int *'}
    1239 |        __u64 start, __u64 len)
         |        ~~~~~~^~~~~


vim +/validate_range +1816 fs/userfaultfd.c

ad465cae96b456 Andrea Arcangeli 2015-09-04  1797  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1798  static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1799  				    unsigned long arg)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1800  {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1801  	int ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1802  	struct uffdio_writeprotect uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1803  	struct uffdio_writeprotect __user *user_uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1804  	struct userfaultfd_wake_range range;
23080e2783ba45 Peter Xu         2020-04-06  1805  	bool mode_wp, mode_dontwake;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1806  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1807  	if (READ_ONCE(ctx->mmap_changing))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1808  		return -EAGAIN;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1809  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1810  	user_uffdio_wp = (struct uffdio_writeprotect __user *) arg;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1811  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1812  	if (copy_from_user(&uffdio_wp, user_uffdio_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1813  			   sizeof(struct uffdio_writeprotect)))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1814  		return -EFAULT;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1815  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06 @1816  	ret = validate_range(ctx->mm, &uffdio_wp.range.start,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1817  			     uffdio_wp.range.len);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1818  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1819  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1820  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1821  	if (uffdio_wp.mode & ~(UFFDIO_WRITEPROTECT_MODE_DONTWAKE |
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1822  			       UFFDIO_WRITEPROTECT_MODE_WP))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1823  		return -EINVAL;
23080e2783ba45 Peter Xu         2020-04-06  1824  
23080e2783ba45 Peter Xu         2020-04-06  1825  	mode_wp = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_WP;
23080e2783ba45 Peter Xu         2020-04-06  1826  	mode_dontwake = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_DONTWAKE;
23080e2783ba45 Peter Xu         2020-04-06  1827  
23080e2783ba45 Peter Xu         2020-04-06  1828  	if (mode_wp && mode_dontwake)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1829  		return -EINVAL;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1830  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1831  	ret = mwriteprotect_range(ctx->mm, uffdio_wp.range.start,
23080e2783ba45 Peter Xu         2020-04-06  1832  				  uffdio_wp.range.len, mode_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1833  				  &ctx->mmap_changing);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1834  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1835  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1836  
23080e2783ba45 Peter Xu         2020-04-06  1837  	if (!mode_wp && !mode_dontwake) {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1838  		range.start = uffdio_wp.range.start;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1839  		range.len = uffdio_wp.range.len;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1840  		wake_userfault(ctx, &range);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1841  	}
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1842  	return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1843  }
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1844  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24375 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
@ 2021-07-03  5:02     ` kernel test robot
  0 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-07-03  5:02 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 6729 bytes --]

Hi Peter,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on kselftest/next linus/master v5.13 next-20210701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: openrisc-randconfig-c004-20210702 (attached as .config)
compiler: or1k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/42177546697fa573571799dc11ecd12a65449886
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
        git checkout 42177546697fa573571799dc11ecd12a65449886
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=openrisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   fs/userfaultfd.c: In function 'userfaultfd_writeprotect':
>> fs/userfaultfd.c:1816:32: warning: passing argument 2 of 'validate_range' makes integer from pointer without a cast [-Wint-conversion]
    1816 |  ret = validate_range(ctx->mm, &uffdio_wp.range.start,
         |                                ^~~~~~~~~~~~~~~~~~~~~~
         |                                |
         |                                __u64 * {aka long long unsigned int *}
   fs/userfaultfd.c:1239:14: note: expected '__u64' {aka 'long long unsigned int'} but argument is of type '__u64 *' {aka 'long long unsigned int *'}
    1239 |        __u64 start, __u64 len)
         |        ~~~~~~^~~~~
   fs/userfaultfd.c: In function 'userfaultfd_continue':
   fs/userfaultfd.c:1864:32: warning: passing argument 2 of 'validate_range' makes integer from pointer without a cast [-Wint-conversion]
    1864 |  ret = validate_range(ctx->mm, &uffdio_continue.range.start,
         |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                                |
         |                                __u64 * {aka long long unsigned int *}
   fs/userfaultfd.c:1239:14: note: expected '__u64' {aka 'long long unsigned int'} but argument is of type '__u64 *' {aka 'long long unsigned int *'}
    1239 |        __u64 start, __u64 len)
         |        ~~~~~~^~~~~


vim +/validate_range +1816 fs/userfaultfd.c

ad465cae96b456 Andrea Arcangeli 2015-09-04  1797  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1798  static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1799  				    unsigned long arg)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1800  {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1801  	int ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1802  	struct uffdio_writeprotect uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1803  	struct uffdio_writeprotect __user *user_uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1804  	struct userfaultfd_wake_range range;
23080e2783ba45 Peter Xu         2020-04-06  1805  	bool mode_wp, mode_dontwake;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1806  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1807  	if (READ_ONCE(ctx->mmap_changing))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1808  		return -EAGAIN;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1809  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1810  	user_uffdio_wp = (struct uffdio_writeprotect __user *) arg;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1811  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1812  	if (copy_from_user(&uffdio_wp, user_uffdio_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1813  			   sizeof(struct uffdio_writeprotect)))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1814  		return -EFAULT;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1815  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06 @1816  	ret = validate_range(ctx->mm, &uffdio_wp.range.start,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1817  			     uffdio_wp.range.len);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1818  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1819  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1820  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1821  	if (uffdio_wp.mode & ~(UFFDIO_WRITEPROTECT_MODE_DONTWAKE |
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1822  			       UFFDIO_WRITEPROTECT_MODE_WP))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1823  		return -EINVAL;
23080e2783ba45 Peter Xu         2020-04-06  1824  
23080e2783ba45 Peter Xu         2020-04-06  1825  	mode_wp = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_WP;
23080e2783ba45 Peter Xu         2020-04-06  1826  	mode_dontwake = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_DONTWAKE;
23080e2783ba45 Peter Xu         2020-04-06  1827  
23080e2783ba45 Peter Xu         2020-04-06  1828  	if (mode_wp && mode_dontwake)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1829  		return -EINVAL;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1830  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1831  	ret = mwriteprotect_range(ctx->mm, uffdio_wp.range.start,
23080e2783ba45 Peter Xu         2020-04-06  1832  				  uffdio_wp.range.len, mode_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1833  				  &ctx->mmap_changing);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1834  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1835  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1836  
23080e2783ba45 Peter Xu         2020-04-06  1837  	if (!mode_wp && !mode_dontwake) {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1838  		range.start = uffdio_wp.range.start;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1839  		range.len = uffdio_wp.range.len;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1840  		wake_userfault(ctx, &range);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1841  	}
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1842  	return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1843  }
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1844  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 24375 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
  2021-07-02 22:57   ` Peter Collingbourne
@ 2021-07-03  9:05     ` kernel test robot
  -1 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-07-03  9:05 UTC (permalink / raw)
  To: Peter Collingbourne, Catalin Marinas, Vincenzo Frascino,
	Dave Martin, Will Deacon, Andrew Morton, Andrea Arcangeli
  Cc: kbuild-all, Linux Memory Management List, Peter Collingbourne,
	Alistair Delva, Lokesh Gidra

[-- Attachment #1: Type: text/plain, Size: 6010 bytes --]

Hi Peter,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on kselftest/next v5.13]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: i386-randconfig-s002-20210702 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/42177546697fa573571799dc11ecd12a65449886
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
        git checkout 42177546697fa573571799dc11ecd12a65449886
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   fs/userfaultfd.c:1816:40: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned long long [usertype] start @@     got unsigned long long * @@
   fs/userfaultfd.c:1816:40: sparse:     expected unsigned long long [usertype] start
   fs/userfaultfd.c:1816:40: sparse:     got unsigned long long *
   fs/userfaultfd.c:1864:40: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned long long [usertype] start @@     got unsigned long long * @@
   fs/userfaultfd.c:1864:40: sparse:     expected unsigned long long [usertype] start
   fs/userfaultfd.c:1864:40: sparse:     got unsigned long long *
>> fs/userfaultfd.c:1816:40: sparse: sparse: non size-preserving pointer to integer cast
   fs/userfaultfd.c:1864:40: sparse: sparse: non size-preserving pointer to integer cast

vim +1816 fs/userfaultfd.c

ad465cae96b456 Andrea Arcangeli 2015-09-04  1797  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1798  static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1799  				    unsigned long arg)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1800  {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1801  	int ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1802  	struct uffdio_writeprotect uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1803  	struct uffdio_writeprotect __user *user_uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1804  	struct userfaultfd_wake_range range;
23080e2783ba45 Peter Xu         2020-04-06  1805  	bool mode_wp, mode_dontwake;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1806  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1807  	if (READ_ONCE(ctx->mmap_changing))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1808  		return -EAGAIN;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1809  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1810  	user_uffdio_wp = (struct uffdio_writeprotect __user *) arg;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1811  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1812  	if (copy_from_user(&uffdio_wp, user_uffdio_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1813  			   sizeof(struct uffdio_writeprotect)))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1814  		return -EFAULT;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1815  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06 @1816  	ret = validate_range(ctx->mm, &uffdio_wp.range.start,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1817  			     uffdio_wp.range.len);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1818  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1819  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1820  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1821  	if (uffdio_wp.mode & ~(UFFDIO_WRITEPROTECT_MODE_DONTWAKE |
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1822  			       UFFDIO_WRITEPROTECT_MODE_WP))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1823  		return -EINVAL;
23080e2783ba45 Peter Xu         2020-04-06  1824  
23080e2783ba45 Peter Xu         2020-04-06  1825  	mode_wp = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_WP;
23080e2783ba45 Peter Xu         2020-04-06  1826  	mode_dontwake = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_DONTWAKE;
23080e2783ba45 Peter Xu         2020-04-06  1827  
23080e2783ba45 Peter Xu         2020-04-06  1828  	if (mode_wp && mode_dontwake)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1829  		return -EINVAL;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1830  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1831  	ret = mwriteprotect_range(ctx->mm, uffdio_wp.range.start,
23080e2783ba45 Peter Xu         2020-04-06  1832  				  uffdio_wp.range.len, mode_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1833  				  &ctx->mmap_changing);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1834  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1835  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1836  
23080e2783ba45 Peter Xu         2020-04-06  1837  	if (!mode_wp && !mode_dontwake) {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1838  		range.start = uffdio_wp.range.start;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1839  		range.len = uffdio_wp.range.len;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1840  		wake_userfault(ctx, &range);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1841  	}
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1842  	return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1843  }
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1844  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 42320 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
@ 2021-07-03  9:05     ` kernel test robot
  0 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-07-03  9:05 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 6104 bytes --]

Hi Peter,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on kselftest/next v5.13]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: i386-randconfig-s002-20210702 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/42177546697fa573571799dc11ecd12a65449886
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Peter-Collingbourne/userfaultfd-do-not-untag-user-pointers/20210703-065801
        git checkout 42177546697fa573571799dc11ecd12a65449886
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   fs/userfaultfd.c:1816:40: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned long long [usertype] start @@     got unsigned long long * @@
   fs/userfaultfd.c:1816:40: sparse:     expected unsigned long long [usertype] start
   fs/userfaultfd.c:1816:40: sparse:     got unsigned long long *
   fs/userfaultfd.c:1864:40: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned long long [usertype] start @@     got unsigned long long * @@
   fs/userfaultfd.c:1864:40: sparse:     expected unsigned long long [usertype] start
   fs/userfaultfd.c:1864:40: sparse:     got unsigned long long *
>> fs/userfaultfd.c:1816:40: sparse: sparse: non size-preserving pointer to integer cast
   fs/userfaultfd.c:1864:40: sparse: sparse: non size-preserving pointer to integer cast

vim +1816 fs/userfaultfd.c

ad465cae96b456 Andrea Arcangeli 2015-09-04  1797  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1798  static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1799  				    unsigned long arg)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1800  {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1801  	int ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1802  	struct uffdio_writeprotect uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1803  	struct uffdio_writeprotect __user *user_uffdio_wp;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1804  	struct userfaultfd_wake_range range;
23080e2783ba45 Peter Xu         2020-04-06  1805  	bool mode_wp, mode_dontwake;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1806  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1807  	if (READ_ONCE(ctx->mmap_changing))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1808  		return -EAGAIN;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1809  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1810  	user_uffdio_wp = (struct uffdio_writeprotect __user *) arg;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1811  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1812  	if (copy_from_user(&uffdio_wp, user_uffdio_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1813  			   sizeof(struct uffdio_writeprotect)))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1814  		return -EFAULT;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1815  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06 @1816  	ret = validate_range(ctx->mm, &uffdio_wp.range.start,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1817  			     uffdio_wp.range.len);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1818  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1819  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1820  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1821  	if (uffdio_wp.mode & ~(UFFDIO_WRITEPROTECT_MODE_DONTWAKE |
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1822  			       UFFDIO_WRITEPROTECT_MODE_WP))
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1823  		return -EINVAL;
23080e2783ba45 Peter Xu         2020-04-06  1824  
23080e2783ba45 Peter Xu         2020-04-06  1825  	mode_wp = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_WP;
23080e2783ba45 Peter Xu         2020-04-06  1826  	mode_dontwake = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_DONTWAKE;
23080e2783ba45 Peter Xu         2020-04-06  1827  
23080e2783ba45 Peter Xu         2020-04-06  1828  	if (mode_wp && mode_dontwake)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1829  		return -EINVAL;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1830  
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1831  	ret = mwriteprotect_range(ctx->mm, uffdio_wp.range.start,
23080e2783ba45 Peter Xu         2020-04-06  1832  				  uffdio_wp.range.len, mode_wp,
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1833  				  &ctx->mmap_changing);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1834  	if (ret)
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1835  		return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1836  
23080e2783ba45 Peter Xu         2020-04-06  1837  	if (!mode_wp && !mode_dontwake) {
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1838  		range.start = uffdio_wp.range.start;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1839  		range.len = uffdio_wp.range.len;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1840  		wake_userfault(ctx, &range);
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1841  	}
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1842  	return ret;
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1843  }
63b2d4174c4ad1 Andrea Arcangeli 2020-04-06  1844  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 42320 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
  2021-07-02 22:57   ` Peter Collingbourne
  (?)
@ 2021-07-04 15:39     ` Andrey Konovalov
  -1 siblings, 0 replies; 19+ messages in thread
From: Andrey Konovalov @ 2021-07-04 15:39 UTC (permalink / raw)
  To: Peter Collingbourne
  Cc: Catalin Marinas, Vincenzo Frascino, Dave Martin, Will Deacon,
	Andrew Morton, Andrea Arcangeli, Alistair Delva, Lokesh Gidra,
	William McVicker, Evgenii Stepanov, Mitch Phillips, Linux ARM,
	Linux Memory Management List, stable

On Sat, Jul 3, 2021 at 12:57 AM Peter Collingbourne <pcc@google.com> wrote:
>
> If a user program uses userfaultfd on ranges of heap memory, it may
> end up passing a tagged pointer to the kernel in the range.start
> field of the UFFDIO_REGISTER ioctl. This can happen when using an
> MTE-capable allocator, or on Android if using the Tagged Pointers
> feature for MTE readiness [1].
>
> When a fault subsequently occurs, the tag is stripped from the fault
> address returned to the application in the fault.address field
> of struct uffd_msg. However, from the application's perspective,
> the tagged address *is* the memory address, so if the application
> is unaware of memory tags, it may get confused by receiving an
> address that is, from its point of view, outside of the bounds of the
> allocation. We observed this behavior in the kselftest for userfaultfd
> [2] but other applications could have the same problem.
>
> Address this by not untagging pointers passed to the userfaultfd
> ioctls. Instead, let the system call fail. This will provide an
> early indication of problems with tag-unaware userspace code instead
> of letting the code get confused later, and is consistent with how
> we decided to handle brk/mmap/mremap in commit dcde237319e6 ("mm:
> Avoid creating virtual address aliases in brk()/mmap()/mremap()"),
> as well as being consistent with the existing tagged address ABI
> documentation relating to how ioctl arguments are handled.
>
> The code change is a revert of commit 7d0325749a6c ("userfaultfd:
> untag user pointers").
>
> [1] https://source.android.com/devices/tech/debug/tagged-pointers
> [2] tools/testing/selftests/vm/userfaultfd.c
>
> Signed-off-by: Peter Collingbourne <pcc@google.com>
> Link: https://linux-review.googlesource.com/id/I761aa9f0344454c482b83fcfcce547db0a25501b
> Fixes: 63f0c6037965 ("arm64: Introduce prctl() options to control the tagged user addresses ABI")
> Cc: <stable@vger.kernel.org> # 5.4
> ---
>  Documentation/arm64/tagged-address-abi.rst | 25 +++++++++++++++-------
>  fs/userfaultfd.c                           | 22 +++++++++----------
>  2 files changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/arm64/tagged-address-abi.rst b/Documentation/arm64/tagged-address-abi.rst
> index 459e6b66ff68..737f9d8565a2 100644
> --- a/Documentation/arm64/tagged-address-abi.rst
> +++ b/Documentation/arm64/tagged-address-abi.rst
> @@ -45,14 +45,23 @@ how the user addresses are used by the kernel:
>
>  1. User addresses not accessed by the kernel but used for address space
>     management (e.g. ``mprotect()``, ``madvise()``). The use of valid
> -   tagged pointers in this context is allowed with the exception of
> -   ``brk()``, ``mmap()`` and the ``new_address`` argument to
> -   ``mremap()`` as these have the potential to alias with existing
> -   user addresses.
> -
> -   NOTE: This behaviour changed in v5.6 and so some earlier kernels may
> -   incorrectly accept valid tagged pointers for the ``brk()``,
> -   ``mmap()`` and ``mremap()`` system calls.
> +   tagged pointers in this context is allowed with these exceptions:
> +
> +   - ``brk()``, ``mmap()`` and the ``new_address`` argument to
> +     ``mremap()`` as these have the potential to alias with existing
> +      user addresses.
> +
> +     NOTE: This behaviour changed in v5.6 and so some earlier kernels may
> +     incorrectly accept valid tagged pointers for the ``brk()``,
> +     ``mmap()`` and ``mremap()`` system calls.
> +
> +   - The ``range.start`` argument to the ``UFFDIO_REGISTER`` ``ioctl()``
> +     used on a file descriptor obtained from ``userfaultfd()``, as
> +     fault addresses subsequently obtained by reading the file descriptor
> +     will be untagged, which may otherwise confuse tag-unaware programs.
> +
> +     NOTE: This behaviour changed in v5.14 and so some earlier kernels may
> +     incorrectly accept valid tagged pointers for this system call.
>
>  2. User addresses accessed by the kernel (e.g. ``write()``). This ABI
>     relaxation is disabled by default and the application thread needs to
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index dd7a6c62b56f..7613efe002c1 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -1236,23 +1236,21 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx,
>  }
>
>  static __always_inline int validate_range(struct mm_struct *mm,
> -                                         __u64 *start, __u64 len)
> +                                         __u64 start, __u64 len)
>  {
>         __u64 task_size = mm->task_size;
>
> -       *start = untagged_addr(*start);
> -
> -       if (*start & ~PAGE_MASK)
> +       if (start & ~PAGE_MASK)
>                 return -EINVAL;
>         if (len & ~PAGE_MASK)
>                 return -EINVAL;
>         if (!len)
>                 return -EINVAL;
> -       if (*start < mmap_min_addr)
> +       if (start < mmap_min_addr)
>                 return -EINVAL;
> -       if (*start >= task_size)
> +       if (start >= task_size)
>                 return -EINVAL;
> -       if (len > task_size - *start)
> +       if (len > task_size - start)
>                 return -EINVAL;
>         return 0;
>  }
> @@ -1313,7 +1311,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
>                 vm_flags |= VM_UFFD_MINOR;
>         }
>
> -       ret = validate_range(mm, &uffdio_register.range.start,
> +       ret = validate_range(mm, uffdio_register.range.start,
>                              uffdio_register.range.len);
>         if (ret)
>                 goto out;
> @@ -1519,7 +1517,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
>         if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister)))
>                 goto out;
>
> -       ret = validate_range(mm, &uffdio_unregister.start,
> +       ret = validate_range(mm, uffdio_unregister.start,
>                              uffdio_unregister.len);
>         if (ret)
>                 goto out;
> @@ -1668,7 +1666,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
>         if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake)))
>                 goto out;
>
> -       ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len);
> +       ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len);
>         if (ret)
>                 goto out;
>
> @@ -1708,7 +1706,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
>                            sizeof(uffdio_copy)-sizeof(__s64)))
>                 goto out;
>
> -       ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len);
> +       ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len);
>         if (ret)
>                 goto out;
>         /*
> @@ -1765,7 +1763,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
>                            sizeof(uffdio_zeropage)-sizeof(__s64)))
>                 goto out;
>
> -       ret = validate_range(ctx->mm, &uffdio_zeropage.range.start,
> +       ret = validate_range(ctx->mm, uffdio_zeropage.range.start,
>                              uffdio_zeropage.range.len);
>         if (ret)
>                 goto out;
> --
> 2.32.0.93.g670b81a890-goog
>

Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
@ 2021-07-04 15:39     ` Andrey Konovalov
  0 siblings, 0 replies; 19+ messages in thread
From: Andrey Konovalov @ 2021-07-04 15:39 UTC (permalink / raw)
  To: Peter Collingbourne
  Cc: Catalin Marinas, Vincenzo Frascino, Dave Martin, Will Deacon,
	Andrew Morton, Andrea Arcangeli, Alistair Delva, Lokesh Gidra,
	William McVicker, Evgenii Stepanov, Mitch Phillips, Linux ARM,
	Linux Memory Management List, stable

On Sat, Jul 3, 2021 at 12:57 AM Peter Collingbourne <pcc@google.com> wrote:
>
> If a user program uses userfaultfd on ranges of heap memory, it may
> end up passing a tagged pointer to the kernel in the range.start
> field of the UFFDIO_REGISTER ioctl. This can happen when using an
> MTE-capable allocator, or on Android if using the Tagged Pointers
> feature for MTE readiness [1].
>
> When a fault subsequently occurs, the tag is stripped from the fault
> address returned to the application in the fault.address field
> of struct uffd_msg. However, from the application's perspective,
> the tagged address *is* the memory address, so if the application
> is unaware of memory tags, it may get confused by receiving an
> address that is, from its point of view, outside of the bounds of the
> allocation. We observed this behavior in the kselftest for userfaultfd
> [2] but other applications could have the same problem.
>
> Address this by not untagging pointers passed to the userfaultfd
> ioctls. Instead, let the system call fail. This will provide an
> early indication of problems with tag-unaware userspace code instead
> of letting the code get confused later, and is consistent with how
> we decided to handle brk/mmap/mremap in commit dcde237319e6 ("mm:
> Avoid creating virtual address aliases in brk()/mmap()/mremap()"),
> as well as being consistent with the existing tagged address ABI
> documentation relating to how ioctl arguments are handled.
>
> The code change is a revert of commit 7d0325749a6c ("userfaultfd:
> untag user pointers").
>
> [1] https://source.android.com/devices/tech/debug/tagged-pointers
> [2] tools/testing/selftests/vm/userfaultfd.c
>
> Signed-off-by: Peter Collingbourne <pcc@google.com>
> Link: https://linux-review.googlesource.com/id/I761aa9f0344454c482b83fcfcce547db0a25501b
> Fixes: 63f0c6037965 ("arm64: Introduce prctl() options to control the tagged user addresses ABI")
> Cc: <stable@vger.kernel.org> # 5.4
> ---
>  Documentation/arm64/tagged-address-abi.rst | 25 +++++++++++++++-------
>  fs/userfaultfd.c                           | 22 +++++++++----------
>  2 files changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/arm64/tagged-address-abi.rst b/Documentation/arm64/tagged-address-abi.rst
> index 459e6b66ff68..737f9d8565a2 100644
> --- a/Documentation/arm64/tagged-address-abi.rst
> +++ b/Documentation/arm64/tagged-address-abi.rst
> @@ -45,14 +45,23 @@ how the user addresses are used by the kernel:
>
>  1. User addresses not accessed by the kernel but used for address space
>     management (e.g. ``mprotect()``, ``madvise()``). The use of valid
> -   tagged pointers in this context is allowed with the exception of
> -   ``brk()``, ``mmap()`` and the ``new_address`` argument to
> -   ``mremap()`` as these have the potential to alias with existing
> -   user addresses.
> -
> -   NOTE: This behaviour changed in v5.6 and so some earlier kernels may
> -   incorrectly accept valid tagged pointers for the ``brk()``,
> -   ``mmap()`` and ``mremap()`` system calls.
> +   tagged pointers in this context is allowed with these exceptions:
> +
> +   - ``brk()``, ``mmap()`` and the ``new_address`` argument to
> +     ``mremap()`` as these have the potential to alias with existing
> +      user addresses.
> +
> +     NOTE: This behaviour changed in v5.6 and so some earlier kernels may
> +     incorrectly accept valid tagged pointers for the ``brk()``,
> +     ``mmap()`` and ``mremap()`` system calls.
> +
> +   - The ``range.start`` argument to the ``UFFDIO_REGISTER`` ``ioctl()``
> +     used on a file descriptor obtained from ``userfaultfd()``, as
> +     fault addresses subsequently obtained by reading the file descriptor
> +     will be untagged, which may otherwise confuse tag-unaware programs.
> +
> +     NOTE: This behaviour changed in v5.14 and so some earlier kernels may
> +     incorrectly accept valid tagged pointers for this system call.
>
>  2. User addresses accessed by the kernel (e.g. ``write()``). This ABI
>     relaxation is disabled by default and the application thread needs to
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index dd7a6c62b56f..7613efe002c1 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -1236,23 +1236,21 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx,
>  }
>
>  static __always_inline int validate_range(struct mm_struct *mm,
> -                                         __u64 *start, __u64 len)
> +                                         __u64 start, __u64 len)
>  {
>         __u64 task_size = mm->task_size;
>
> -       *start = untagged_addr(*start);
> -
> -       if (*start & ~PAGE_MASK)
> +       if (start & ~PAGE_MASK)
>                 return -EINVAL;
>         if (len & ~PAGE_MASK)
>                 return -EINVAL;
>         if (!len)
>                 return -EINVAL;
> -       if (*start < mmap_min_addr)
> +       if (start < mmap_min_addr)
>                 return -EINVAL;
> -       if (*start >= task_size)
> +       if (start >= task_size)
>                 return -EINVAL;
> -       if (len > task_size - *start)
> +       if (len > task_size - start)
>                 return -EINVAL;
>         return 0;
>  }
> @@ -1313,7 +1311,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
>                 vm_flags |= VM_UFFD_MINOR;
>         }
>
> -       ret = validate_range(mm, &uffdio_register.range.start,
> +       ret = validate_range(mm, uffdio_register.range.start,
>                              uffdio_register.range.len);
>         if (ret)
>                 goto out;
> @@ -1519,7 +1517,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
>         if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister)))
>                 goto out;
>
> -       ret = validate_range(mm, &uffdio_unregister.start,
> +       ret = validate_range(mm, uffdio_unregister.start,
>                              uffdio_unregister.len);
>         if (ret)
>                 goto out;
> @@ -1668,7 +1666,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
>         if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake)))
>                 goto out;
>
> -       ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len);
> +       ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len);
>         if (ret)
>                 goto out;
>
> @@ -1708,7 +1706,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
>                            sizeof(uffdio_copy)-sizeof(__s64)))
>                 goto out;
>
> -       ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len);
> +       ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len);
>         if (ret)
>                 goto out;
>         /*
> @@ -1765,7 +1763,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
>                            sizeof(uffdio_zeropage)-sizeof(__s64)))
>                 goto out;
>
> -       ret = validate_range(ctx->mm, &uffdio_zeropage.range.start,
> +       ret = validate_range(ctx->mm, uffdio_zeropage.range.start,
>                              uffdio_zeropage.range.len);
>         if (ret)
>                 goto out;
> --
> 2.32.0.93.g670b81a890-goog
>

Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
@ 2021-07-04 15:39     ` Andrey Konovalov
  0 siblings, 0 replies; 19+ messages in thread
From: Andrey Konovalov @ 2021-07-04 15:39 UTC (permalink / raw)
  To: Peter Collingbourne
  Cc: Catalin Marinas, Vincenzo Frascino, Dave Martin, Will Deacon,
	Andrew Morton, Andrea Arcangeli, Alistair Delva, Lokesh Gidra,
	William McVicker, Evgenii Stepanov, Mitch Phillips, Linux ARM,
	Linux Memory Management List, stable

On Sat, Jul 3, 2021 at 12:57 AM Peter Collingbourne <pcc@google.com> wrote:
>
> If a user program uses userfaultfd on ranges of heap memory, it may
> end up passing a tagged pointer to the kernel in the range.start
> field of the UFFDIO_REGISTER ioctl. This can happen when using an
> MTE-capable allocator, or on Android if using the Tagged Pointers
> feature for MTE readiness [1].
>
> When a fault subsequently occurs, the tag is stripped from the fault
> address returned to the application in the fault.address field
> of struct uffd_msg. However, from the application's perspective,
> the tagged address *is* the memory address, so if the application
> is unaware of memory tags, it may get confused by receiving an
> address that is, from its point of view, outside of the bounds of the
> allocation. We observed this behavior in the kselftest for userfaultfd
> [2] but other applications could have the same problem.
>
> Address this by not untagging pointers passed to the userfaultfd
> ioctls. Instead, let the system call fail. This will provide an
> early indication of problems with tag-unaware userspace code instead
> of letting the code get confused later, and is consistent with how
> we decided to handle brk/mmap/mremap in commit dcde237319e6 ("mm:
> Avoid creating virtual address aliases in brk()/mmap()/mremap()"),
> as well as being consistent with the existing tagged address ABI
> documentation relating to how ioctl arguments are handled.
>
> The code change is a revert of commit 7d0325749a6c ("userfaultfd:
> untag user pointers").
>
> [1] https://source.android.com/devices/tech/debug/tagged-pointers
> [2] tools/testing/selftests/vm/userfaultfd.c
>
> Signed-off-by: Peter Collingbourne <pcc@google.com>
> Link: https://linux-review.googlesource.com/id/I761aa9f0344454c482b83fcfcce547db0a25501b
> Fixes: 63f0c6037965 ("arm64: Introduce prctl() options to control the tagged user addresses ABI")
> Cc: <stable@vger.kernel.org> # 5.4
> ---
>  Documentation/arm64/tagged-address-abi.rst | 25 +++++++++++++++-------
>  fs/userfaultfd.c                           | 22 +++++++++----------
>  2 files changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/arm64/tagged-address-abi.rst b/Documentation/arm64/tagged-address-abi.rst
> index 459e6b66ff68..737f9d8565a2 100644
> --- a/Documentation/arm64/tagged-address-abi.rst
> +++ b/Documentation/arm64/tagged-address-abi.rst
> @@ -45,14 +45,23 @@ how the user addresses are used by the kernel:
>
>  1. User addresses not accessed by the kernel but used for address space
>     management (e.g. ``mprotect()``, ``madvise()``). The use of valid
> -   tagged pointers in this context is allowed with the exception of
> -   ``brk()``, ``mmap()`` and the ``new_address`` argument to
> -   ``mremap()`` as these have the potential to alias with existing
> -   user addresses.
> -
> -   NOTE: This behaviour changed in v5.6 and so some earlier kernels may
> -   incorrectly accept valid tagged pointers for the ``brk()``,
> -   ``mmap()`` and ``mremap()`` system calls.
> +   tagged pointers in this context is allowed with these exceptions:
> +
> +   - ``brk()``, ``mmap()`` and the ``new_address`` argument to
> +     ``mremap()`` as these have the potential to alias with existing
> +      user addresses.
> +
> +     NOTE: This behaviour changed in v5.6 and so some earlier kernels may
> +     incorrectly accept valid tagged pointers for the ``brk()``,
> +     ``mmap()`` and ``mremap()`` system calls.
> +
> +   - The ``range.start`` argument to the ``UFFDIO_REGISTER`` ``ioctl()``
> +     used on a file descriptor obtained from ``userfaultfd()``, as
> +     fault addresses subsequently obtained by reading the file descriptor
> +     will be untagged, which may otherwise confuse tag-unaware programs.
> +
> +     NOTE: This behaviour changed in v5.14 and so some earlier kernels may
> +     incorrectly accept valid tagged pointers for this system call.
>
>  2. User addresses accessed by the kernel (e.g. ``write()``). This ABI
>     relaxation is disabled by default and the application thread needs to
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index dd7a6c62b56f..7613efe002c1 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -1236,23 +1236,21 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx,
>  }
>
>  static __always_inline int validate_range(struct mm_struct *mm,
> -                                         __u64 *start, __u64 len)
> +                                         __u64 start, __u64 len)
>  {
>         __u64 task_size = mm->task_size;
>
> -       *start = untagged_addr(*start);
> -
> -       if (*start & ~PAGE_MASK)
> +       if (start & ~PAGE_MASK)
>                 return -EINVAL;
>         if (len & ~PAGE_MASK)
>                 return -EINVAL;
>         if (!len)
>                 return -EINVAL;
> -       if (*start < mmap_min_addr)
> +       if (start < mmap_min_addr)
>                 return -EINVAL;
> -       if (*start >= task_size)
> +       if (start >= task_size)
>                 return -EINVAL;
> -       if (len > task_size - *start)
> +       if (len > task_size - start)
>                 return -EINVAL;
>         return 0;
>  }
> @@ -1313,7 +1311,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
>                 vm_flags |= VM_UFFD_MINOR;
>         }
>
> -       ret = validate_range(mm, &uffdio_register.range.start,
> +       ret = validate_range(mm, uffdio_register.range.start,
>                              uffdio_register.range.len);
>         if (ret)
>                 goto out;
> @@ -1519,7 +1517,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
>         if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister)))
>                 goto out;
>
> -       ret = validate_range(mm, &uffdio_unregister.start,
> +       ret = validate_range(mm, uffdio_unregister.start,
>                              uffdio_unregister.len);
>         if (ret)
>                 goto out;
> @@ -1668,7 +1666,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
>         if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake)))
>                 goto out;
>
> -       ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len);
> +       ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len);
>         if (ret)
>                 goto out;
>
> @@ -1708,7 +1706,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
>                            sizeof(uffdio_copy)-sizeof(__s64)))
>                 goto out;
>
> -       ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len);
> +       ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len);
>         if (ret)
>                 goto out;
>         /*
> @@ -1765,7 +1763,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
>                            sizeof(uffdio_zeropage)-sizeof(__s64)))
>                 goto out;
>
> -       ret = validate_range(ctx->mm, &uffdio_zeropage.range.start,
> +       ret = validate_range(ctx->mm, uffdio_zeropage.range.start,
>                              uffdio_zeropage.range.len);
>         if (ret)
>                 goto out;
> --
> 2.32.0.93.g670b81a890-goog
>

Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-07-04 15:41 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-02 22:57 [PATCH v3 0/2] userfaultfd: do not untag user pointers Peter Collingbourne
2021-07-02 22:57 ` Peter Collingbourne
2021-07-02 22:57 ` [PATCH v3 1/2] " Peter Collingbourne
2021-07-02 22:57   ` Peter Collingbourne
2021-07-02 22:57   ` Peter Collingbourne
2021-07-03  2:46   ` kernel test robot
2021-07-03  2:46     ` kernel test robot
2021-07-03  3:06   ` kernel test robot
2021-07-03  3:06     ` kernel test robot
2021-07-03  5:02   ` kernel test robot
2021-07-03  5:02     ` kernel test robot
2021-07-03  9:05   ` kernel test robot
2021-07-03  9:05     ` kernel test robot
2021-07-04 15:39   ` Andrey Konovalov
2021-07-04 15:39     ` Andrey Konovalov
2021-07-04 15:39     ` Andrey Konovalov
2021-07-02 22:57 ` [PATCH v3 2/2] selftest: use mmap instead of posix_memalign to allocate memory Peter Collingbourne
2021-07-02 22:57   ` Peter Collingbourne
2021-07-02 22:57   ` Peter Collingbourne

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.