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

[Hongfu Li <hongfu.li@linux.dev>]

在 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