Right now it appears that the code is relying upon the returned destination
address having bits outside PAGE_MASK to indicate whether an error value is
specified, and decrementing the increased refcount on the uffd ctx if so.
This is not a safe means of determining an error value, so instead, be
specific. It makes far more sense to do so in a dedicated error path, so
add mremap_userfaultfd_fail() for this purpose and use this when an error
arises.
A vm_userfaultfd_ctx is not established until we are at the point where
mremap_userfaultfd_prep() is invoked in copy_vma_and_data(), so this is a
no-op until this happens.
That is - uffd remap notification only occurs if the VMA is actually moved
- at which point a UFFD_EVENT_REMAP event is raised.
No errors can occur after this point currently, though it's certainly not
guaranteed this will always remain the case, and we mustn't rely on this.
However, the reason for needing to handle this case is that, when an error
arises on a VMA move at the point of adjusting page tables, we revert this
operation, and propagate the error.
At this point, it is not correct to raise a uffd remap event, and we must
handle it.
This refactoring makes it abundantly clear what we are doing.
We assume vrm->new_addr is always valid, which a prior change made the case
even for mremap() invocations which don't move the VMA, however given no
uffd context would be set up in this case it's immaterial to this change
anyway.
No functional change intended.
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
fs/userfaultfd.c | 15 ++++++++++-----
include/linux/userfaultfd_k.h | 1 +
mm/mremap.c | 16 ++++++++++++----
3 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 2a644aa1a510..54c6cc7fe9c6 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -750,11 +750,6 @@ void mremap_userfaultfd_complete(struct vm_userfaultfd_ctx *vm_ctx,
if (!ctx)
return;
- if (to & ~PAGE_MASK) {
- userfaultfd_ctx_put(ctx);
- return;
- }
-
msg_init(&ewq.msg);
ewq.msg.event = UFFD_EVENT_REMAP;
@@ -765,6 +760,16 @@ void mremap_userfaultfd_complete(struct vm_userfaultfd_ctx *vm_ctx,
userfaultfd_event_wait_completion(ctx, &ewq);
}
+void mremap_userfaultfd_fail(struct vm_userfaultfd_ctx *vm_ctx)
+{
+ struct userfaultfd_ctx *ctx = vm_ctx->ctx;
+
+ if (!ctx)
+ return;
+
+ userfaultfd_ctx_put(ctx);
+}
+
bool userfaultfd_remove(struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
index df85330bcfa6..6680a4de40b3 100644
--- a/include/linux/userfaultfd_k.h
+++ b/include/linux/userfaultfd_k.h
@@ -259,6 +259,7 @@ extern void mremap_userfaultfd_prep(struct vm_area_struct *,
extern void mremap_userfaultfd_complete(struct vm_userfaultfd_ctx *,
unsigned long from, unsigned long to,
unsigned long len);
+void mremap_userfaultfd_fail(struct vm_userfaultfd_ctx *);
extern bool userfaultfd_remove(struct vm_area_struct *vma,
unsigned long start,
diff --git a/mm/mremap.c b/mm/mremap.c
index 660bdb75e2f9..db7e773d0884 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -1729,12 +1729,17 @@ static int check_prep_vma(struct vma_remap_struct *vrm)
return 0;
}
-static void notify_uffd(struct vma_remap_struct *vrm, unsigned long ret)
+static void notify_uffd(struct vma_remap_struct *vrm, bool failed)
{
struct mm_struct *mm = current->mm;
+ /* Regardless of success/failure, we always notify of any unmaps. */
userfaultfd_unmap_complete(mm, vrm->uf_unmap_early);
- mremap_userfaultfd_complete(vrm->uf, vrm->addr, ret, vrm->old_len);
+ if (failed)
+ mremap_userfaultfd_fail(vrm->uf);
+ else
+ mremap_userfaultfd_complete(vrm->uf, vrm->addr,
+ vrm->new_addr, vrm->old_len);
userfaultfd_unmap_complete(mm, vrm->uf_unmap);
}
@@ -1742,6 +1747,7 @@ static unsigned long do_mremap(struct vma_remap_struct *vrm)
{
struct mm_struct *mm = current->mm;
unsigned long res;
+ bool failed;
vrm->old_len = PAGE_ALIGN(vrm->old_len);
vrm->new_len = PAGE_ALIGN(vrm->new_len);
@@ -1763,13 +1769,15 @@ static unsigned long do_mremap(struct vma_remap_struct *vrm)
res = vrm_implies_new_addr(vrm) ? mremap_to(vrm) : mremap_at(vrm);
out:
+ failed = IS_ERR_VALUE(res);
+
if (vrm->mmap_locked)
mmap_write_unlock(mm);
- if (!IS_ERR_VALUE(res) && vrm->mlocked && vrm->new_len > vrm->old_len)
+ if (!failed && vrm->mlocked && vrm->new_len > vrm->old_len)
mm_populate(vrm->new_addr + vrm->old_len, vrm->delta);
- notify_uffd(vrm, res);
+ notify_uffd(vrm, failed);
return res;
}
--
2.50.0On 7/7/25 07:27, Lorenzo Stoakes wrote: > Right now it appears that the code is relying upon the returned destination > address having bits outside PAGE_MASK to indicate whether an error value is > specified, and decrementing the increased refcount on the uffd ctx if so. > > This is not a safe means of determining an error value, so instead, be > specific. It makes far more sense to do so in a dedicated error path, so > add mremap_userfaultfd_fail() for this purpose and use this when an error > arises. > > A vm_userfaultfd_ctx is not established until we are at the point where > mremap_userfaultfd_prep() is invoked in copy_vma_and_data(), so this is a > no-op until this happens. > > That is - uffd remap notification only occurs if the VMA is actually moved > - at which point a UFFD_EVENT_REMAP event is raised. > > No errors can occur after this point currently, though it's certainly not > guaranteed this will always remain the case, and we mustn't rely on this. > > However, the reason for needing to handle this case is that, when an error > arises on a VMA move at the point of adjusting page tables, we revert this > operation, and propagate the error. > > At this point, it is not correct to raise a uffd remap event, and we must > handle it. > > This refactoring makes it abundantly clear what we are doing. > > We assume vrm->new_addr is always valid, which a prior change made the case > even for mremap() invocations which don't move the VMA, however given no > uffd context would be set up in this case it's immaterial to this change > anyway. > > No functional change intended. > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> Reviewed-by: Vlastimil Babka <vbabka@suse.cz> Guess that renders my previous nit unimportant.
Hi Andrew,
I missed the !CONFIG_USERFAULTFD stub, could you apply the attached fix-patch?
Thanks!
----8<----
From 048bfb6ee415843bd584c64a2c6e6be9b1114962 Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Date: Mon, 7 Jul 2025 11:15:18 +0100
Subject: [PATCH] add missing mremap_userfaultfd_fail() stub
This covers the !CONFIG_USERFAULTFD case.
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
include/linux/userfaultfd_k.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
index 6680a4de40b3..c0e716aec26a 100644
--- a/include/linux/userfaultfd_k.h
+++ b/include/linux/userfaultfd_k.h
@@ -372,6 +372,10 @@ static inline void mremap_userfaultfd_complete(struct vm_userfaultfd_ctx *ctx,
{
}
+static inline void mremap_userfaultfd_fail(struct vm_userfaultfd_ctx *ctx)
+{
+}
+
static inline bool userfaultfd_remove(struct vm_area_struct *vma,
unsigned long start,
unsigned long end)
--
2.50.0
Hi Lorenzo,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything]
url: https://github.com/intel-lab-lkp/linux/commits/Lorenzo-Stoakes/mm-mremap-perform-some-simple-cleanups/20250707-133132
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/be3e068c77107d385d89eae634317cb59e04e5ba.1751865330.git.lorenzo.stoakes%40oracle.com
patch subject: [PATCH 05/10] mm/mremap: use an explicit uffd failure path for mremap
config: i386-buildonly-randconfig-002-20250707 (https://download.01.org/0day-ci/archive/20250707/202507071505.e2HFMCH2-lkp@intel.com/config)
compiler: clang version 20.1.7 (https://github.com/llvm/llvm-project 6146a88f60492b520a36f8f8f3231e15f3cc6082)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250707/202507071505.e2HFMCH2-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507071505.e2HFMCH2-lkp@intel.com/
All errors (new ones prefixed by >>):
>> mm/mremap.c:1739:3: error: call to undeclared function 'mremap_userfaultfd_fail'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1739 | mremap_userfaultfd_fail(vrm->uf);
| ^
mm/mremap.c:1739:3: note: did you mean 'mremap_userfaultfd_prep'?
include/linux/userfaultfd_k.h:363:20: note: 'mremap_userfaultfd_prep' declared here
363 | static inline void mremap_userfaultfd_prep(struct vm_area_struct *vma,
| ^
1 error generated.
vim +/mremap_userfaultfd_fail +1739 mm/mremap.c
1731
1732 static void notify_uffd(struct vma_remap_struct *vrm, bool failed)
1733 {
1734 struct mm_struct *mm = current->mm;
1735
1736 /* Regardless of success/failure, we always notify of any unmaps. */
1737 userfaultfd_unmap_complete(mm, vrm->uf_unmap_early);
1738 if (failed)
> 1739 mremap_userfaultfd_fail(vrm->uf);
1740 else
1741 mremap_userfaultfd_complete(vrm->uf, vrm->addr,
1742 vrm->new_addr, vrm->old_len);
1743 userfaultfd_unmap_complete(mm, vrm->uf_unmap);
1744 }
1745
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Mon, Jul 07, 2025 at 03:56:53PM +0800, kernel test robot wrote:
> Hi Lorenzo,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on akpm-mm/mm-everything]
Thanks for the report, I just need to add a stub for this, will send a fix-patch!
Cheers, Lorenzo
>
> url: https://github.com/intel-lab-lkp/linux/commits/Lorenzo-Stoakes/mm-mremap-perform-some-simple-cleanups/20250707-133132
> base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
> patch link: https://lore.kernel.org/r/be3e068c77107d385d89eae634317cb59e04e5ba.1751865330.git.lorenzo.stoakes%40oracle.com
> patch subject: [PATCH 05/10] mm/mremap: use an explicit uffd failure path for mremap
> config: i386-buildonly-randconfig-002-20250707 (https://download.01.org/0day-ci/archive/20250707/202507071505.e2HFMCH2-lkp@intel.com/config)
> compiler: clang version 20.1.7 (https://github.com/llvm/llvm-project 6146a88f60492b520a36f8f8f3231e15f3cc6082)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250707/202507071505.e2HFMCH2-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202507071505.e2HFMCH2-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> >> mm/mremap.c:1739:3: error: call to undeclared function 'mremap_userfaultfd_fail'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> 1739 | mremap_userfaultfd_fail(vrm->uf);
> | ^
> mm/mremap.c:1739:3: note: did you mean 'mremap_userfaultfd_prep'?
> include/linux/userfaultfd_k.h:363:20: note: 'mremap_userfaultfd_prep' declared here
> 363 | static inline void mremap_userfaultfd_prep(struct vm_area_struct *vma,
> | ^
> 1 error generated.
>
>
> vim +/mremap_userfaultfd_fail +1739 mm/mremap.c
>
> 1731
> 1732 static void notify_uffd(struct vma_remap_struct *vrm, bool failed)
> 1733 {
> 1734 struct mm_struct *mm = current->mm;
> 1735
> 1736 /* Regardless of success/failure, we always notify of any unmaps. */
> 1737 userfaultfd_unmap_complete(mm, vrm->uf_unmap_early);
> 1738 if (failed)
> > 1739 mremap_userfaultfd_fail(vrm->uf);
> 1740 else
> 1741 mremap_userfaultfd_complete(vrm->uf, vrm->addr,
> 1742 vrm->new_addr, vrm->old_len);
> 1743 userfaultfd_unmap_complete(mm, vrm->uf_unmap);
> 1744 }
> 1745
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
© 2016 - 2025 Red Hat, Inc.