* [PATCH v3 0/2] userfaultfd: do not untag user pointers
@ 2021-07-02 22:57 Peter Collingbourne
2021-07-02 22:57 ` [PATCH v3 1/2] " Peter Collingbourne
2021-07-02 22:57 ` [PATCH v3 2/2] selftest: use mmap instead of posix_memalign to allocate memory Peter Collingbourne
0 siblings, 2 replies; 8+ 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] 8+ messages in thread
* [PATCH v3 1/2] userfaultfd: do not untag user pointers
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-03 2:46 ` kernel test robot
` (4 more replies)
2021-07-02 22:57 ` [PATCH v3 2/2] selftest: use mmap instead of posix_memalign to allocate memory Peter Collingbourne
1 sibling, 5 replies; 8+ 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] 8+ messages in thread
* [PATCH v3 2/2] selftest: use mmap instead of posix_memalign to allocate memory
2021-07-02 22:57 [PATCH v3 0/2] userfaultfd: do not untag user pointers Peter Collingbourne
2021-07-02 22:57 ` [PATCH v3 1/2] " Peter Collingbourne
@ 2021-07-02 22:57 ` Peter Collingbourne
1 sibling, 0 replies; 8+ 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] 8+ messages in thread
* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
2021-07-02 22:57 ` [PATCH v3 1/2] " Peter Collingbourne
@ 2021-07-03 2:46 ` kernel test robot
2021-07-03 3:06 ` kernel test robot
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ 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] 8+ messages in thread
* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
2021-07-02 22:57 ` [PATCH v3 1/2] " Peter Collingbourne
2021-07-03 2:46 ` kernel test robot
@ 2021-07-03 3:06 ` kernel test robot
2021-07-03 5:02 ` kernel test robot
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ 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] 8+ messages in thread
* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
2021-07-02 22:57 ` [PATCH v3 1/2] " Peter Collingbourne
2021-07-03 2:46 ` kernel test robot
2021-07-03 3:06 ` kernel test robot
@ 2021-07-03 5:02 ` kernel test robot
2021-07-03 9:05 ` kernel test robot
2021-07-04 15:39 ` Andrey Konovalov
4 siblings, 0 replies; 8+ 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] 8+ messages in thread
* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
2021-07-02 22:57 ` [PATCH v3 1/2] " Peter Collingbourne
` (2 preceding siblings ...)
2021-07-03 5:02 ` kernel test robot
@ 2021-07-03 9:05 ` kernel test robot
2021-07-04 15:39 ` Andrey Konovalov
4 siblings, 0 replies; 8+ 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] 8+ messages in thread
* Re: [PATCH v3 1/2] userfaultfd: do not untag user pointers
2021-07-02 22:57 ` [PATCH v3 1/2] " Peter Collingbourne
` (3 preceding siblings ...)
2021-07-03 9:05 ` kernel test robot
@ 2021-07-04 15:39 ` Andrey Konovalov
4 siblings, 0 replies; 8+ 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] 8+ messages in thread
end of thread, other threads:[~2021-07-04 15:40 UTC | newest]
Thread overview: 8+ 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 ` [PATCH v3 1/2] " Peter Collingbourne
2021-07-03 2:46 ` kernel test robot
2021-07-03 3:06 ` kernel test robot
2021-07-03 5:02 ` kernel test robot
2021-07-03 9:05 ` kernel test robot
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).