[PATCH] mm/madvise: reject invalid process_madvise() advice for zero-length vectors

[PATCH] mm/madvise: reject invalid process_madvise() advice for zero-length vectors

[fujunjie]

process_madvise() validates the advice while walking the imported iovec.
If the iovec has zero total length, vector_madvise() never enters the
loop and returns 0 without checking whether the advice value is valid.

For a local mm, such as process_madvise(PIDFD_SELF, ...), the remote-only
process_madvise_remote_valid() check is skipped.  As a result, an invalid
advice can be reported as success when the vector has zero total length.
This differs from madvise(), which rejects an invalid advice before
returning success for a zero-length range.

Reject invalid advice before walking the vector.  Valid zero-length
requests remain no-ops and continue to return 0.

Add a selftest that covers invalid advice with a zero-length iovec and an
empty vector, while also checking that a valid zero-length request still
succeeds.

Fixes: 021781b01275 ("mm/madvise: unrestrict process_madvise() for current process")
Signed-off-by: fujunjie <fujunjie1@qq.com>
---
Testing:
- Built bzImage.
- Built tools/testing/selftests/mm/process_madv.
- Ran tools/testing/selftests/mm/process_madv in QEMU:
  # PASSED: 7 / 7 tests passed.

 mm/madvise.c                              |  3 +++
 tools/testing/selftests/mm/process_madv.c | 29 +++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/mm/madvise.c b/mm/madvise.c
index 69708e953cf56..83fe9e651a907 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -2046,6 +2046,9 @@ static ssize_t vector_madvise(struct mm_struct *mm, struct iov_iter *iter,
 
 	total_len = iov_iter_count(iter);
 
+	if (!madvise_behavior_valid(behavior))
+		return -EINVAL;
+
 	ret = madvise_lock(&madv_behavior);
 	if (ret)
 		return ret;
diff --git a/tools/testing/selftests/mm/process_madv.c b/tools/testing/selftests/mm/process_madv.c
index cd4610baf5d7d..9a7e2788fcc50 100644
--- a/tools/testing/selftests/mm/process_madv.c
+++ b/tools/testing/selftests/mm/process_madv.c
@@ -309,6 +309,35 @@ TEST_F(process_madvise, invalid_vlen)
 	ASSERT_EQ(munmap(map, pagesize), 0);
 }
 
+/*
+ * Test that invalid advice is rejected even when the iovec has zero total
+ * length. A zero-length advice is a no-op for valid advice, but invalid
+ * advice should still fail with EINVAL.
+ */
+TEST_F(process_madvise, invalid_advice_zero_length)
+{
+	struct iovec vec = {
+		.iov_base = NULL,
+		.iov_len = 0,
+	};
+	int pidfd = self->pidfd;
+	ssize_t ret;
+
+	errno = 0;
+	ret = sys_process_madvise(pidfd, &vec, 1, -1, 0);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+
+	errno = 0;
+	ret = sys_process_madvise(pidfd, &vec, 1, MADV_DONTNEED, 0);
+	ASSERT_EQ(ret, 0);
+
+	errno = 0;
+	ret = sys_process_madvise(pidfd, NULL, 0, -1, 0);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+}
+
 /*
  * Test process_madvise() with an invalid flag value. Currently, only a flag
  * value of 0 is supported. This test is reserved for the future, e.g., if

base-commit: 1b55f8358e35a67bf3969339ea7b86988af92f66
-- 
2.34.1

Re: [PATCH] mm/madvise: reject invalid process_madvise() advice for zero-length vectors

[Andrew Morton]

On Sun, 26 Apr 2026 11:08:22 +0000 fujunjie <fujunjie1@qq.com> wrote:

> process_madvise() validates the advice while walking the imported iovec.

Seems inefficient to be checking `behavior' repeatedly.  I wonder if
your change will permit us to remove that madvise_behavior_valid()
check from is_valid_madvise().

> If the iovec has zero total length, vector_madvise() never enters the
> loop and returns 0 without checking whether the advice value is valid.
> 
> For a local mm, such as process_madvise(PIDFD_SELF, ...), the remote-only
> process_madvise_remote_valid() check is skipped.  As a result, an invalid
> advice can be reported as success when the vector has zero total length.
> This differs from madvise(), which rejects an invalid advice before
> returning success for a zero-length range.
> 
> Reject invalid advice before walking the vector.  Valid zero-length
> requests remain no-ops and continue to return 0.
> 
> Add a selftest that covers invalid advice with a zero-length iovec and an
> empty vector, while also checking that a valid zero-length request still
> succeeds.

lgtm, thanks.  Slightly non-backward-compatible but I think we can live
with that.

My process_madvise manpage doesn't even anticipate bogus `advice'
parameters.

And grr, the manpage calls it `advice' but the kernel calls it
`behavior'.

Re: [PATCH] mm/madvise: reject invalid process_madvise() advice for zero-length vectors

[fujunjie]

On Sun, Apr 26, 2026 at 12:41:42PM -0700, Andrew Morton wrote:
> Seems inefficient to be checking `behavior' repeatedly.  I wonder if
> your change will permit us to remove that madvise_behavior_valid()
> check from is_valid_madvise().

Yes, I think that makes sense.  I'll look into preparing a follow-up
patch for this in a few days.

Best regards,
Fujunjie

Re: [PATCH] mm/madvise: reject invalid process_madvise() advice for zero-length vectors

[David Hildenbrand (Arm)]

On 4/26/26 13:08, fujunjie wrote:
> process_madvise() validates the advice while walking the imported iovec.
> If the iovec has zero total length, vector_madvise() never enters the
> loop and returns 0 without checking whether the advice value is valid.
> 
> For a local mm, such as process_madvise(PIDFD_SELF, ...), the remote-only
> process_madvise_remote_valid() check is skipped.  As a result, an invalid
> advice can be reported as success when the vector has zero total length.
> This differs from madvise(), which rejects an invalid advice before
> returning success for a zero-length range.
> 
> Reject invalid advice before walking the vector.  Valid zero-length
> requests remain no-ops and continue to return 0.
> 
> Add a selftest that covers invalid advice with a zero-length iovec and an
> empty vector, while also checking that a valid zero-length request still
> succeeds.
> 
> Fixes: 021781b01275 ("mm/madvise: unrestrict process_madvise() for current process")
> Signed-off-by: fujunjie <fujunjie1@qq.com>
> ---
> Testing:
> - Built bzImage.
> - Built tools/testing/selftests/mm/process_madv.
> - Ran tools/testing/selftests/mm/process_madv in QEMU:
>   # PASSED: 7 / 7 tests passed.
> 
>  mm/madvise.c                              |  3 +++
>  tools/testing/selftests/mm/process_madv.c | 29 +++++++++++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 69708e953cf56..83fe9e651a907 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -2046,6 +2046,9 @@ static ssize_t vector_madvise(struct mm_struct *mm, struct iov_iter *iter,
>  
>  	total_len = iov_iter_count(iter);
>  
> +	if (!madvise_behavior_valid(behavior))
> +		return -EINVAL;
> +

I'd suggest performing the check just before the !process_madvise_remote_valid()
check, such that "invalid/unsupported" orders are reported in the same priority,
independent of local vs. non-local MM.

Or is there a good reason to not do it there?

-- 
Cheers,

David

[PATCH v2] mm/madvise: reject invalid process_madvise() advice for zero-length vectors

[fujunjie]

process_madvise() used to validate the advice while walking each
imported iovec. If the vector has zero total length, vector_madvise()
does not enter the loop and can return success without checking whether
the advice value is valid.

For a local mm, such as process_madvise(PIDFD_SELF, ...), the remote-only
process_madvise_remote_valid() check is skipped. As a result, an invalid
advice can be reported as success when the vector has zero total length.
This differs from madvise(), which rejects an invalid advice before
returning success for a zero-length range.

Validate the generic madvise behavior at the syscall-facing entry points
before any vector walk. In process_madvise(), do this before the
remote-only advice restriction so unsupported advice is rejected with the
same priority for local and remote mm. Then keep the per-range helper
focused on address/length validation, avoiding repeated behavior checks
for every iovec.

Valid zero-length requests remain no-ops and continue to return 0. Add a
selftest that covers invalid advice with a zero-length iovec and an empty
vector, while also checking that a valid zero-length request still
succeeds.

Fixes: 021781b01275 ("mm/madvise: unrestrict process_madvise() for current process")
Signed-off-by: fujunjie <fujunjie1@qq.com>
---
v2:
- Validate behavior at the syscall-facing entry points and leave the range
  helper for address/length checks, avoiding repeated behavior checks in the
  iovec loop.
- Put the generic process_madvise() behavior check before
  process_madvise_remote_valid(), as suggested by David.
- Keep the zero-length selftest coverage from v1.

Testing:
Built bzImage and tools/testing/selftests/mm/process_madv. In QEMU, the
process_madv selftest reports 7/7 passed.

 mm/madvise.c                              | 29 ++++++++++++++++-------------
 tools/testing/selftests/mm/process_madv.c | 29 +++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index 69708e953cf56..ce238dd96f158 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1834,13 +1834,10 @@ static void madvise_finish_tlb(struct madvise_behavior *madv_behavior)
 		tlb_finish_mmu(madv_behavior->tlb);
 }
 
-static bool is_valid_madvise(unsigned long start, size_t len_in, int behavior)
+static bool is_valid_madvise_range(unsigned long start, size_t len_in)
 {
 	size_t len;
 
-	if (!madvise_behavior_valid(behavior))
-		return false;
-
 	if (!PAGE_ALIGNED(start))
 		return false;
 	len = PAGE_ALIGN(len_in);
@@ -1859,17 +1856,15 @@ static bool is_valid_madvise(unsigned long start, size_t len_in, int behavior)
  * madvise_should_skip() - Return if the request is invalid or nothing.
  * @start:	Start address of madvise-requested address range.
  * @len_in:	Length of madvise-requested address range.
- * @behavior:	Requested madvise behavior.
  * @err:	Pointer to store an error code from the check.
  *
- * If the specified behaviour is invalid or nothing would occur, we skip the
- * operation.  This function returns true in the cases, otherwise false.  In
- * the former case we store an error on @err.
+ * If the specified range is invalid or nothing would occur, we skip the
+ * operation.  This function returns true in these cases, otherwise false.  In
+ * the former case we store an error in @err.
  */
-static bool madvise_should_skip(unsigned long start, size_t len_in,
-		int behavior, int *err)
+static bool madvise_should_skip(unsigned long start, size_t len_in, int *err)
 {
-	if (!is_valid_madvise(start, len_in, behavior)) {
+	if (!is_valid_madvise_range(start, len_in)) {
 		*err = -EINVAL;
 		return true;
 	}
@@ -2013,7 +2008,10 @@ int do_madvise(struct mm_struct *mm, unsigned long start, size_t len_in, int beh
 		.tlb = &tlb,
 	};
 
-	if (madvise_should_skip(start, len_in, behavior, &error))
+	if (!madvise_behavior_valid(behavior))
+		return -EINVAL;
+
+	if (madvise_should_skip(start, len_in, &error))
 		return error;
 	error = madvise_lock(&madv_behavior);
 	if (error)
@@ -2056,7 +2054,7 @@ static ssize_t vector_madvise(struct mm_struct *mm, struct iov_iter *iter,
 		size_t len_in = iter_iov_len(iter);
 		int error;
 
-		if (madvise_should_skip(start, len_in, behavior, &error))
+		if (madvise_should_skip(start, len_in, &error))
 			ret = error;
 		else
 			ret = madvise_do_behavior(start, len_in, &madv_behavior);
@@ -2131,6 +2129,11 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec,
 		goto release_task;
 	}
 
+	if (!madvise_behavior_valid(behavior)) {
+		ret = -EINVAL;
+		goto release_mm;
+	}
+
 	/*
 	 * We need only perform this check if we are attempting to manipulate a
 	 * remote process's address space.
diff --git a/tools/testing/selftests/mm/process_madv.c b/tools/testing/selftests/mm/process_madv.c
index cd4610baf5d7d..9a7e2788fcc50 100644
--- a/tools/testing/selftests/mm/process_madv.c
+++ b/tools/testing/selftests/mm/process_madv.c
@@ -309,6 +309,35 @@ TEST_F(process_madvise, invalid_vlen)
 	ASSERT_EQ(munmap(map, pagesize), 0);
 }
 
+/*
+ * Test that invalid advice is rejected even when the iovec has zero total
+ * length. A zero-length advice is a no-op for valid advice, but invalid
+ * advice should still fail with EINVAL.
+ */
+TEST_F(process_madvise, invalid_advice_zero_length)
+{
+	struct iovec vec = {
+		.iov_base = NULL,
+		.iov_len = 0,
+	};
+	int pidfd = self->pidfd;
+	ssize_t ret;
+
+	errno = 0;
+	ret = sys_process_madvise(pidfd, &vec, 1, -1, 0);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+
+	errno = 0;
+	ret = sys_process_madvise(pidfd, &vec, 1, MADV_DONTNEED, 0);
+	ASSERT_EQ(ret, 0);
+
+	errno = 0;
+	ret = sys_process_madvise(pidfd, NULL, 0, -1, 0);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+}
+
 /*
  * Test process_madvise() with an invalid flag value. Currently, only a flag
  * value of 0 is supported. This test is reserved for the future, e.g., if
base-commit: 1b55f8358e35a67bf3969339ea7b86988af92f66
-- 
2.34.1

Re: [PATCH v2] mm/madvise: reject invalid process_madvise() advice for zero-length vectors

[David Hildenbrand (Arm)]

On 4/27/26 11:43, fujunjie wrote:
> process_madvise() used to validate the advice while walking each
> imported iovec. If the vector has zero total length, vector_madvise()
> does not enter the loop and can return success without checking whether
> the advice value is valid.
> 
> For a local mm, such as process_madvise(PIDFD_SELF, ...), the remote-only
> process_madvise_remote_valid() check is skipped. As a result, an invalid
> advice can be reported as success when the vector has zero total length.
> This differs from madvise(), which rejects an invalid advice before
> returning success for a zero-length range.
> 
> Validate the generic madvise behavior at the syscall-facing entry points
> before any vector walk. In process_madvise(), do this before the
> remote-only advice restriction so unsupported advice is rejected with the
> same priority for local and remote mm. Then keep the per-range helper
> focused on address/length validation, avoiding repeated behavior checks
> for every iovec.
> 
> Valid zero-length requests remain no-ops and continue to return 0. Add a
> selftest that covers invalid advice with a zero-length iovec and an empty
> vector, while also checking that a valid zero-length request still
> succeeds.
> 
> Fixes: 021781b01275 ("mm/madvise: unrestrict process_madvise() for current process")
> Signed-off-by: fujunjie <fujunjie1@qq.com>
> ---


Next time, please don't send new revisions as reply to previous submissions.

Thanks!

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH v2] mm/madvise: reject invalid process_madvise() advice for zero-length vectors

[SeongJae Park]

On Mon, 27 Apr 2026 09:43:30 +0000 fujunjie <fujunjie1@qq.com> wrote:

> process_madvise() used to validate the advice while walking each
> imported iovec. If the vector has zero total length, vector_madvise()
> does not enter the loop and can return success without checking whether
> the advice value is valid.
> 
> For a local mm, such as process_madvise(PIDFD_SELF, ...), the remote-only
> process_madvise_remote_valid() check is skipped. As a result, an invalid
> advice can be reported as success when the vector has zero total length.
> This differs from madvise(), which rejects an invalid advice before
> returning success for a zero-length range.
> 
> Validate the generic madvise behavior at the syscall-facing entry points
> before any vector walk. In process_madvise(), do this before the
> remote-only advice restriction so unsupported advice is rejected with the
> same priority for local and remote mm. Then keep the per-range helper
> focused on address/length validation, avoiding repeated behavior checks
> for every iovec.
> 
> Valid zero-length requests remain no-ops and continue to return 0. Add a
> selftest that covers invalid advice with a zero-length iovec and an empty
> vector, while also checking that a valid zero-length request still
> succeeds.
> 
> Fixes: 021781b01275 ("mm/madvise: unrestrict process_madvise() for current process")
> Signed-off-by: fujunjie <fujunjie1@qq.com>

Looks good to me.  I have trivial comments below, though.  Because those are
really trivial, please feel free to add

Reviewed-by: SeongJae Park <sj@kernel.org>

> ---
[...]
>   *
> - * If the specified behaviour is invalid or nothing would occur, we skip the
> - * operation.  This function returns true in the cases, otherwise false.  In
> - * the former case we store an error on @err.
> + * If the specified range is invalid or nothing would occur, we skip the
> + * operation.  This function returns true in these cases, otherwise false.  In
> + * the former case we store an error in @err.

Maybe we can keep the second and the third lines of the above comment
unchanged?

[...]
> +/*
> + * Test that invalid advice is rejected even when the iovec has zero total
> + * length. A zero-length advice is a no-op for valid advice, but invalid
> + * advice should still fail with EINVAL.
> + */
> +TEST_F(process_madvise, invalid_advice_zero_length)
> +{
> +	struct iovec vec = {
> +		.iov_base = NULL,
> +		.iov_len = 0,
> +	};
> +	int pidfd = self->pidfd;
> +	ssize_t ret;
> +
> +	errno = 0;
> +	ret = sys_process_madvise(pidfd, &vec, 1, -1, 0);
> +	ASSERT_EQ(ret, -1);
> +	ASSERT_EQ(errno, EINVAL);
> +
> +	errno = 0;
> +	ret = sys_process_madvise(pidfd, &vec, 1, MADV_DONTNEED, 0);
> +	ASSERT_EQ(ret, 0);
> +
> +	errno = 0;

The previous sys_process_madvise() is expected to not set errno, correct?
Maybe the above 'errno' reassignment is unnecessary?

> +	ret = sys_process_madvise(pidfd, NULL, 0, -1, 0);
> +	ASSERT_EQ(ret, -1);
> +	ASSERT_EQ(errno, EINVAL);
> +}


Thanks,
SJ

[...]