[PATCH 0/2] userfaultfd: validate the whole range before changing write protection

[PATCH 0/2] userfaultfd: validate the whole range before changing write protection

[Umang Pokhriyal]

A UFFDIO_WRITEPROTECT range that spans an unregistered VMA fails, but
the VMAs before it are already (un)protected, and on unprotect no waiter
is woken.  Patch 1 validates the whole range first, as
mrwprotect_range() does; it is open-coded rather than shared with it so
that it applies to stable kernels without RWP.  Patch 2 adds a uffd
selftest.

The problem was found while reviewing the userfaultfd range ioctls with
an LLM, which also helped write the patches.  Tested on mm-new in a VM:
the new selftest and a standalone reproducer (both directions, and a
misaligned hugetlb range) fail without patch 1 and pass with it, and the
rest of uffd-unit-tests gives the same results with and without it.

Umang Pokhriyal (2):
  userfaultfd: validate the whole range before changing write protection
  selftests/mm: test a uffd write-protect range with an unregistered
    page

 mm/userfaultfd.c                             | 20 +++++--
 tools/testing/selftests/mm/uffd-unit-tests.c | 56 ++++++++++++++++++++
 2 files changed, 71 insertions(+), 5 deletions(-)


base-commit: d42dfde37af766d1c272467352c0a56d3169ee58
-- 
2.53.0

[PATCH 1/2] userfaultfd: validate the whole range before changing write protection

[Umang Pokhriyal]

mwriteprotect_range() checks each VMA as it walks the range.  When a later
VMA is not registered for write protection, or is a hugetlb VMA the range
is not aligned to, the call fails after the earlier VMAs were already
changed.  When unprotecting, userfaultfd_writeprotect() then skips
wake_userfault(), so threads blocked on those pages are not woken although
the pages are writable again.

An unprivileged process can trigger this with a range that spans an
unregistered VMA.  It hasn't been seen in practice; it was found while
reviewing the userfaultfd range ioctls with an LLM.

Validate the whole range first, as mrwprotect_range() already does.

Fixes: a1b92a3f1498 ("mm/userfaultfd: support WP on multiple VMAs")
Cc: stable@vger.kernel.org
Signed-off-by: Umang Pokhriyal <umangpokhriyall@gmail.com>
---
 mm/userfaultfd.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 87da92526..b8a9d9574 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -1105,21 +1105,31 @@ static int mwriteprotect_range(struct userfaultfd_ctx *ctx, unsigned long start,
 	if (atomic_read(&ctx->mmap_changing))
 		goto out_unlock;
 
+	/*
+	 * Validate every spanned VMA before changing any of them, so an
+	 * invalid range is rejected without changing part of it.
+	 */
 	err = -ENOENT;
 	for_each_vma_range(vmi, dst_vma, end) {
-
 		if (!userfaultfd_wp(dst_vma)) {
 			err = -ENOENT;
-			break;
+			goto out_unlock;
 		}
 
 		if (vma_is_hugetlb(dst_vma)) {
-			err = -EINVAL;
 			page_mask = vma_kernel_pagesize(dst_vma) - 1;
-			if ((start & page_mask) || (len & page_mask))
-				break;
+			if ((start & page_mask) || (len & page_mask)) {
+				err = -EINVAL;
+				goto out_unlock;
+			}
 		}
+		err = 0;
+	}
+	if (err)
+		goto out_unlock;
 
+	vma_iter_set(&vmi, start);
+	for_each_vma_range(vmi, dst_vma, end) {
 		_start = max(dst_vma->vm_start, start);
 		_end = min(dst_vma->vm_end, end);
 
-- 
2.53.0

Re: [PATCH 1/2] userfaultfd: validate the whole range before changing write protection

[Mike Rapoport]

On Mon, Nov 02, 2026 at 04:06:20PM +0530, Umang Pokhriyal wrote:
> mwriteprotect_range() checks each VMA as it walks the range.  When a later
> VMA is not registered for write protection, or is a hugetlb VMA the range
> is not aligned to, the call fails after the earlier VMAs were already
> changed.  When unprotecting, userfaultfd_writeprotect() then skips
> wake_userfault(), so threads blocked on those pages are not woken although
> the pages are writable again.
> 
> An unprivileged process can trigger this with a range that spans an
> unregistered VMA.  It hasn't been seen in practice; it was found while
> reviewing the userfaultfd range ioctls with an LLM.
> 
> Validate the whole range first, as mrwprotect_range() already does.
> 
> Fixes: a1b92a3f1498 ("mm/userfaultfd: support WP on multiple VMAs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Umang Pokhriyal <umangpokhriyall@gmail.com>
> ---
>  mm/userfaultfd.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> index 87da92526..b8a9d9574 100644
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
> @@ -1105,21 +1105,31 @@ static int mwriteprotect_range(struct userfaultfd_ctx *ctx, unsigned long start,
>  	if (atomic_read(&ctx->mmap_changing))
>  		goto out_unlock;
>  
> +	/*
> +	 * Validate every spanned VMA before changing any of them, so an
> +	 * invalid range is rejected without changing part of it.
> +	 */
>  	err = -ENOENT;

Nit: this assignment is now redundant, isn't it?

>  	for_each_vma_range(vmi, dst_vma, end) {
> -
>  		if (!userfaultfd_wp(dst_vma)) {
>  			err = -ENOENT;
> -			break;
> +			goto out_unlock;
>  		}
>  
>  		if (vma_is_hugetlb(dst_vma)) {
> -			err = -EINVAL;

Same here.

>  			page_mask = vma_kernel_pagesize(dst_vma) - 1;
> -			if ((start & page_mask) || (len & page_mask))
> -				break;
> +			if ((start & page_mask) || (len & page_mask)) {
> +				err = -EINVAL;
> +				goto out_unlock;
> +			}
>  		}
> +		err = 0;
> +	}
> +	if (err)
> +		goto out_unlock;

If we a breaking on any error to out_unlock, why do we need err = 0 and
this check?

>  
> +	vma_iter_set(&vmi, start);
> +	for_each_vma_range(vmi, dst_vma, end) {
>  		_start = max(dst_vma->vm_start, start);
>  		_end = min(dst_vma->vm_end, end);
>  
> -- 
> 2.53.0
> 

-- 
Sincerely yours,
Mike.

[PATCH 2/2] selftests/mm: test a uffd write-protect range with an unregistered page

[Umang Pokhriyal]

Register only the first of two pages and check that UFFDIO_WRITEPROTECT
over both pages fails with ENOENT without changing the first page, when
protecting and when unprotecting.

Signed-off-by: Umang Pokhriyal <umangpokhriyall@gmail.com>
---
 tools/testing/selftests/mm/uffd-unit-tests.c | 56 ++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
index 298609544..57e442fd2 100644
--- a/tools/testing/selftests/mm/uffd-unit-tests.c
+++ b/tools/testing/selftests/mm/uffd-unit-tests.c
@@ -375,6 +375,56 @@ static void uffd_wp_unpopulated_test(uffd_global_test_opts_t *gopts, uffd_test_a
 	uffd_test_pass();
 }
 
+/*
+ * UFFDIO_WRITEPROTECT over a range whose second page is not registered must
+ * fail without changing the first page, in both directions.
+ */
+static void uffd_wp_partial_range_test(uffd_global_test_opts_t *gopts,
+				       uffd_test_args_t *args)
+{
+	struct uffdio_writeprotect prms = {
+		.range.start = (uint64_t)gopts->area_dst,
+		.range.len = 2 * gopts->page_size,
+	};
+	uint64_t value;
+	int pagemap_fd;
+
+	/* Only the first page is registered; the second is a separate VMA */
+	if (uffd_register(gopts->uffd, gopts->area_dst, gopts->page_size,
+			  false, true, false))
+		err("register failed");
+
+	pagemap_fd = pagemap_open();
+	*gopts->area_dst = 1;
+
+	prms.mode = UFFDIO_WRITEPROTECT_MODE_WP;
+	if (!ioctl(gopts->uffd, UFFDIO_WRITEPROTECT, &prms) || errno != ENOENT) {
+		uffd_test_fail("protect over an unregistered page did not fail with ENOENT");
+		goto out;
+	}
+	value = pagemap_get_entry(pagemap_fd, gopts->area_dst);
+	if (value & PM_UFFD_WP) {
+		uffd_test_fail("failed protect changed the first page");
+		goto out;
+	}
+
+	wp_range(gopts->uffd, (uint64_t)gopts->area_dst, gopts->page_size, true);
+	prms.mode = 0;
+	if (!ioctl(gopts->uffd, UFFDIO_WRITEPROTECT, &prms) || errno != ENOENT) {
+		uffd_test_fail("unprotect over an unregistered page did not fail with ENOENT");
+		goto out;
+	}
+	value = pagemap_get_entry(pagemap_fd, gopts->area_dst);
+	if (!(value & PM_UFFD_WP)) {
+		uffd_test_fail("failed unprotect changed the first page");
+		goto out;
+	}
+
+	uffd_test_pass();
+out:
+	close(pagemap_fd);
+}
+
 static void uffd_wp_fork_test_common(uffd_global_test_opts_t *gopts, uffd_test_args_t *args,
 				     bool with_event)
 {
@@ -2403,6 +2453,12 @@ uffd_test_case_t uffd_tests[] = {
 		.uffd_feature_required =
 		UFFD_FEATURE_PAGEFAULT_FLAG_WP | UFFD_FEATURE_WP_UNPOPULATED,
 	},
+	{
+		.name = "wp-partial-range",
+		.uffd_fn = uffd_wp_partial_range_test,
+		.mem_targets = MEM_ANON,
+		.uffd_feature_required = UFFD_FEATURE_PAGEFAULT_FLAG_WP,
+	},
 	{
 		.name = "minor",
 		.uffd_fn = uffd_minor_test,
-- 
2.53.0

Re: [PATCH 2/2] selftests/mm: test a uffd write-protect range with an unregistered page

[Mike Rapoport]

Hi Umang,

On Mon, Nov 02, 2026 at 04:06:21PM +0530, Umang Pokhriyal wrote:
> Register only the first of two pages and check that UFFDIO_WRITEPROTECT
> over both pages fails with ENOENT without changing the first page, when
> protecting and when unprotecting.

I don't think it's worth keeping the test for a regression we already
fixed.
 
> Signed-off-by: Umang Pokhriyal <umangpokhriyall@gmail.com>
> ---
>  tools/testing/selftests/mm/uffd-unit-tests.c | 56 ++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c
> index 298609544..57e442fd2 100644
> --- a/tools/testing/selftests/mm/uffd-unit-tests.c
> +++ b/tools/testing/selftests/mm/uffd-unit-tests.c
> @@ -375,6 +375,56 @@ static void uffd_wp_unpopulated_test(uffd_global_test_opts_t *gopts, uffd_test_a
>  	uffd_test_pass();
>  }
>  
> +/*
> + * UFFDIO_WRITEPROTECT over a range whose second page is not registered must
> + * fail without changing the first page, in both directions.
> + */
> +static void uffd_wp_partial_range_test(uffd_global_test_opts_t *gopts,
> +				       uffd_test_args_t *args)
> +{
> +	struct uffdio_writeprotect prms = {
> +		.range.start = (uint64_t)gopts->area_dst,
> +		.range.len = 2 * gopts->page_size,
> +	};
> +	uint64_t value;
> +	int pagemap_fd;
> +
> +	/* Only the first page is registered; the second is a separate VMA */
> +	if (uffd_register(gopts->uffd, gopts->area_dst, gopts->page_size,
> +			  false, true, false))
> +		err("register failed");
> +
> +	pagemap_fd = pagemap_open();
> +	*gopts->area_dst = 1;
> +
> +	prms.mode = UFFDIO_WRITEPROTECT_MODE_WP;
> +	if (!ioctl(gopts->uffd, UFFDIO_WRITEPROTECT, &prms) || errno != ENOENT) {
> +		uffd_test_fail("protect over an unregistered page did not fail with ENOENT");
> +		goto out;
> +	}
> +	value = pagemap_get_entry(pagemap_fd, gopts->area_dst);
> +	if (value & PM_UFFD_WP) {
> +		uffd_test_fail("failed protect changed the first page");
> +		goto out;
> +	}
> +
> +	wp_range(gopts->uffd, (uint64_t)gopts->area_dst, gopts->page_size, true);
> +	prms.mode = 0;
> +	if (!ioctl(gopts->uffd, UFFDIO_WRITEPROTECT, &prms) || errno != ENOENT) {
> +		uffd_test_fail("unprotect over an unregistered page did not fail with ENOENT");
> +		goto out;
> +	}
> +	value = pagemap_get_entry(pagemap_fd, gopts->area_dst);
> +	if (!(value & PM_UFFD_WP)) {
> +		uffd_test_fail("failed unprotect changed the first page");
> +		goto out;
> +	}
> +
> +	uffd_test_pass();
> +out:
> +	close(pagemap_fd);
> +}
> +
>  static void uffd_wp_fork_test_common(uffd_global_test_opts_t *gopts, uffd_test_args_t *args,
>  				     bool with_event)
>  {
> @@ -2403,6 +2453,12 @@ uffd_test_case_t uffd_tests[] = {
>  		.uffd_feature_required =
>  		UFFD_FEATURE_PAGEFAULT_FLAG_WP | UFFD_FEATURE_WP_UNPOPULATED,
>  	},
> +	{
> +		.name = "wp-partial-range",
> +		.uffd_fn = uffd_wp_partial_range_test,
> +		.mem_targets = MEM_ANON,
> +		.uffd_feature_required = UFFD_FEATURE_PAGEFAULT_FLAG_WP,
> +	},
>  	{
>  		.name = "minor",
>  		.uffd_fn = uffd_minor_test,
> -- 
> 2.53.0
> 

-- 
Sincerely yours,
Mike.