[PATCH] selftests/mm: fix ptrace PEEKDATA check in memfd_secret test

[PATCH] selftests/mm: fix ptrace PEEKDATA check in memfd_secret test

[Hongfu Li]

From: Hongfu Li <lihongfu@kylinos.cn>

try_ptrace() treats PTRACE_PEEKDATA return value as a boolean
check. A successful read returns non-zero data (memory filled with
0x55), causing the test to incorrectly report PASS when secret memory
protection is broken.

Check the return value against -1 instead. The test should only pass
when PTRACE_PEEKDATA fails, which means secret memory protection works.

Fixes: 76fe17ef588a ("secretmem: test: add basic selftest for memfd_secret(2)")
Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
---
 tools/testing/selftests/mm/memfd_secret.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/memfd_secret.c b/tools/testing/selftests/mm/memfd_secret.c
index c55d84c5e613..dd08a3a1ef14 100644
--- a/tools/testing/selftests/mm/memfd_secret.c
+++ b/tools/testing/selftests/mm/memfd_secret.c
@@ -145,7 +145,7 @@ static void try_ptrace(int fd, int pipefd[2])
 		exit(KSFT_FAIL);
 	}
 
-	if (ptrace(PTRACE_PEEKDATA, ppid, mem, 0))
+	if (ptrace(PTRACE_PEEKDATA, ppid, mem, 0) == -1)
 		exit(KSFT_PASS);
 
 	exit(KSFT_FAIL);
-- 
2.54.0

Re: [PATCH] selftests/mm: fix ptrace PEEKDATA check in memfd_secret test

[Mike Rapoport]

On Tue, Sep 08, 2026 at 10:51:11AM +0800, Hongfu Li wrote:
> From: Hongfu Li <lihongfu@kylinos.cn>
> 
> try_ptrace() treats PTRACE_PEEKDATA return value as a boolean
> check. A successful read returns non-zero data (memory filled with
> 0x55), causing the test to incorrectly report PASS when secret memory
> protection is broken.
> 
> Check the return value against -1 instead. The test should only pass
> when PTRACE_PEEKDATA fails, which means secret memory protection works.
> 
> Fixes: 76fe17ef588a ("secretmem: test: add basic selftest for memfd_secret(2)")
> Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>

Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  tools/testing/selftests/mm/memfd_secret.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/mm/memfd_secret.c b/tools/testing/selftests/mm/memfd_secret.c
> index c55d84c5e613..dd08a3a1ef14 100644
> --- a/tools/testing/selftests/mm/memfd_secret.c
> +++ b/tools/testing/selftests/mm/memfd_secret.c
> @@ -145,7 +145,7 @@ static void try_ptrace(int fd, int pipefd[2])
>  		exit(KSFT_FAIL);
>  	}
>  
> -	if (ptrace(PTRACE_PEEKDATA, ppid, mem, 0))
> +	if (ptrace(PTRACE_PEEKDATA, ppid, mem, 0) == -1)
>  		exit(KSFT_PASS);
>  
>  	exit(KSFT_FAIL);
> -- 
> 2.54.0
> 
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH] selftests/mm: fix ptrace PEEKDATA check in memfd_secret test

[Lorenzo Stoakes (ARM)]

On Tue, Sep 08, 2026 at 10:51:11AM +0800, Hongfu Li wrote:
> From: Hongfu Li <lihongfu@kylinos.cn>
>
> try_ptrace() treats PTRACE_PEEKDATA return value as a boolean
> check. A successful read returns non-zero data (memory filled with
> 0x55), causing the test to incorrectly report PASS when secret memory
> protection is broken.
>
> Check the return value against -1 instead. The test should only pass
> when PTRACE_PEEKDATA fails, which means secret memory protection works.
>
> Fixes: 76fe17ef588a ("secretmem: test: add basic selftest for memfd_secret(2)")

Not sure if a fixes is warranted? But I also definitely don't think a backport
is in any case in case :P

> Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>

The change LGTM afaict. Though I think a comment should be added. With that
addressed:

Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
>  tools/testing/selftests/mm/memfd_secret.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/mm/memfd_secret.c b/tools/testing/selftests/mm/memfd_secret.c
> index c55d84c5e613..dd08a3a1ef14 100644
> --- a/tools/testing/selftests/mm/memfd_secret.c
> +++ b/tools/testing/selftests/mm/memfd_secret.c
> @@ -145,7 +145,7 @@ static void try_ptrace(int fd, int pipefd[2])
>  		exit(KSFT_FAIL);
>  	}
>
> -	if (ptrace(PTRACE_PEEKDATA, ppid, mem, 0))
> +	if (ptrace(PTRACE_PEEKDATA, ppid, mem, 0) == -1)
>  		exit(KSFT_PASS);

From https://man7.org/linux/man-pages/man2/ptrace.2.html#RETURN_VALUE :

       On success, the PTRACE_PEEK* operations return the requested data
       (but see NOTES)...

       On error, all operations return -1, ...

       ...

       PTRACE_PEEKTEXT
       PTRACE_PEEKDATA
              Read a word at the address addr in the tracee's memory,
              returning the word as the result of the ptrace() call.
              Linux does not have separate text and data address spaces,
              so these two operations are currently equivalent.  (data is
              ignored; but see NOTES.)

OK so we expect this to fail as otherwise that'd be a violation of secretmem.

Feels like maybe we should add a comment to that effect? :)


>
>  	exit(KSFT_FAIL);
> --
> 2.54.0
>

--
Cheers, Lorenzo

Re: [PATCH] selftests/mm: fix ptrace PEEKDATA check in memfd_secret test

[Hongfu Li]

在 2026/9/8 17:03, Lorenzo Stoakes (ARM) 写道:
> On Tue, Sep 08, 2026 at 10:51:11AM +0800, Hongfu Li wrote:
>> From: Hongfu Li <lihongfu@kylinos.cn>
>>
>> try_ptrace() treats PTRACE_PEEKDATA return value as a boolean
>> check. A successful read returns non-zero data (memory filled with
>> 0x55), causing the test to incorrectly report PASS when secret memory
>> protection is broken.
>>
>> Check the return value against -1 instead. The test should only pass
>> when PTRACE_PEEKDATA fails, which means secret memory protection works.
>>
>> Fixes: 76fe17ef588a ("secretmem: test: add basic selftest for memfd_secret(2)")
> Not sure if a fixes is warranted? But I also definitely don't think a backport
> is in any case in case :P
>
>> Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
> The change LGTM afaict. Though I think a comment should be added. With that
> addressed:
>
> Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>
>> ---
>>   tools/testing/selftests/mm/memfd_secret.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/mm/memfd_secret.c b/tools/testing/selftests/mm/memfd_secret.c
>> index c55d84c5e613..dd08a3a1ef14 100644
>> --- a/tools/testing/selftests/mm/memfd_secret.c
>> +++ b/tools/testing/selftests/mm/memfd_secret.c
>> @@ -145,7 +145,7 @@ static void try_ptrace(int fd, int pipefd[2])
>>   		exit(KSFT_FAIL);
>>   	}
>>
>> -	if (ptrace(PTRACE_PEEKDATA, ppid, mem, 0))
>> +	if (ptrace(PTRACE_PEEKDATA, ppid, mem, 0) == -1)
>>   		exit(KSFT_PASS);
>  From https://man7.org/linux/man-pages/man2/ptrace.2.html#RETURN_VALUE :
>
>         On success, the PTRACE_PEEK* operations return the requested data
>         (but see NOTES)...
>
>         On error, all operations return -1, ...
>
>         ...
>
>         PTRACE_PEEKTEXT
>         PTRACE_PEEKDATA
>                Read a word at the address addr in the tracee's memory,
>                returning the word as the result of the ptrace() call.
>                Linux does not have separate text and data address spaces,
>                so these two operations are currently equivalent.  (data is
>                ignored; but see NOTES.)
>
> OK so we expect this to fail as otherwise that'd be a violation of secretmem.
>
> Feels like maybe we should add a comment to that effect? :)

Thanks for the review, and agreed on the comment: the test only passes
when PEEKDATA fails, which is easy to misread, so I'll make that
explicit in v2:

--- a/tools/testing/selftests/mm/memfd_secret.c
+++ b/tools/testing/selftests/mm/memfd_secret.c
@@ -145,7 +145,8 @@ static void try_ptrace(int fd, int pipefd[2])
          exit(KSFT_FAIL);
      }

-    if (ptrace(PTRACE_PEEKDATA, ppid, mem, 0))
+    /* PEEKDATA on secret memory must fail, else protection is broken. */
+    if (ptrace(PTRACE_PEEKDATA, ppid, mem, 0) == -1)
          exit(KSFT_PASS);


I'll also drop the Fixes: tag in v2 since a backport is not intended.

>>   	exit(KSFT_FAIL);
>> --
>> 2.54.0
>>
>> --
>> Cheers, Lorenzo

-- 
Best regards,
Hongfu