| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <fd720337-ebc8-4039-b9bf-062be642f5d3@igalia.com>
Date: Thu, 4 Sep 2025 12:23:53 -0300
From: André Almeida <andrealmeid@...lia.com>
To: Waiman Long <llong@...hat.com>
Cc: linux-kernel@...r.kernel.org, Darren Hart <dvhart@...radead.org>,
Davidlohr Bueso <dave@...olabs.net>, Ingo Molnar <mingo@...hat.com>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Juri Lelli <juri.lelli@...hat.com>, Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Valentin Schneider <vschneid@...hat.com>, Borislav Petkov <bp@...en8.de>,
kernel-dev@...lia.com
Subject: Re: [PATCH 1/2] selftest/futex: Make the error check more precise for
futex_numa_mpol
Hi Waiman,
Thanks for the feedback!
Em 03/09/2025 14:53, Waiman Long escreveu:
> On 9/1/25 4:33 PM, André Almeida wrote:
>> Instead of just checking if the syscall failed as expected, check as
>> well what is the error code returned, to check if it's match the
>> expectation and it's failing in the correct error path inside the
>> kernel.
>>
>> Signed-off-by: André Almeida <andrealmeid@...lia.com>
>> ---
>> This patch is aimed for 6.18
>> ---
>> .../futex/functional/futex_numa_mpol.c | 36 +++++++++++--------
>> 1 file changed, 21 insertions(+), 15 deletions(-)
>>
>> diff --git a/tools/testing/selftests/futex/functional/
>> futex_numa_mpol.c b/tools/testing/selftests/futex/functional/
>> futex_numa_mpol.c
>> index 802c15c82190..c84441751235 100644
>> --- a/tools/testing/selftests/futex/functional/futex_numa_mpol.c
>> +++ b/tools/testing/selftests/futex/functional/futex_numa_mpol.c
>> @@ -77,7 +77,7 @@ static void join_max_threads(void)
>> }
>> }
>> -static void __test_futex(void *futex_ptr, int must_fail, unsigned int
>> futex_flags)
>> +static void __test_futex(void *futex_ptr, int err_value, unsigned int
>> futex_flags)
>> {
>> int to_wake, ret, i, need_exit = 0;
>> @@ -88,11 +88,17 @@ static void __test_futex(void *futex_ptr, int
>> must_fail, unsigned int futex_flag
>> do {
>> ret = futex2_wake(futex_ptr, to_wake, futex_flags);
>> - if (must_fail) {
>> - if (ret < 0)
>> - break;
>> - ksft_exit_fail_msg("futex2_wake(%d, 0x%x) should fail,
>> but didn't\n",
>> - to_wake, futex_flags);
>> +
>> + if (err_value) {
>> + if (ret >= 0)
>> + ksft_exit_fail_msg("futex2_wake(%d, 0x%x) should
>> fail, but didn't\n",
>> + to_wake, futex_flags);
>> +
>> + if (errno != err_value)
>> + ksft_exit_fail_msg("futex2_wake(%d, 0x%x) expected
>> error was %d, but returned %d (%s)\n",
>> + to_wake, futex_flags, err_value, errno,
>> strerror(errno));
>> +
>> + break;
>
> If (ret >= 0), the 2nd (errno != err_value) failure message will likely
> be printed too. Should we use "else if" so that only one error message
> will be printed?
>
>
ksft_exit_fail_msg() calls exit(), so the code will exit before
executing the second failure message.
If this was a ksft_test_result_error() call, then the message would be
printed twice.
>> }
>> if (ret < 0) {
>> ksft_exit_fail_msg("Failed futex2_wake(%d, 0x%x): %m\n",
>> @@ -106,12 +112,12 @@ static void __test_futex(void *futex_ptr, int
>> must_fail, unsigned int futex_flag
>> join_max_threads();
>> for (i = 0; i < MAX_THREADS; i++) {
>> - if (must_fail && thread_args[i].result != -1) {
>> + if (err_value && thread_args[i].result != -1) {
>> ksft_print_msg("Thread %d should fail but succeeded
>> (%d)\n",
>> i, thread_args[i].result);
>> need_exit = 1;
>> }
>> - if (!must_fail && thread_args[i].result != 0) {
>> + if (!err_value && thread_args[i].result != 0) {
>> ksft_print_msg("Thread %d failed (%d)\n", i,
>> thread_args[i].result);
>> need_exit = 1;
>> }
>> @@ -120,14 +126,14 @@ static void __test_futex(void *futex_ptr, int
>> must_fail, unsigned int futex_flag
>> ksft_exit_fail_msg("Aborting due to earlier errors.\n");
>> }
>> -static void test_futex(void *futex_ptr, int must_fail)
>> +static void test_futex(void *futex_ptr, int err_value)
>> {
>> - __test_futex(futex_ptr, must_fail, FUTEX2_SIZE_U32 |
>> FUTEX_PRIVATE_FLAG | FUTEX2_NUMA);
>> + __test_futex(futex_ptr, err_value, FUTEX2_SIZE_U32 |
>> FUTEX_PRIVATE_FLAG | FUTEX2_NUMA);
>> }
>> -static void test_futex_mpol(void *futex_ptr, int must_fail)
>> +static void test_futex_mpol(void *futex_ptr, int err_value)
>> {
>> - __test_futex(futex_ptr, must_fail, FUTEX2_SIZE_U32 |
>> FUTEX_PRIVATE_FLAG | FUTEX2_NUMA | FUTEX2_MPOL);
>> + __test_futex(futex_ptr, err_value, FUTEX2_SIZE_U32 |
>> FUTEX_PRIVATE_FLAG | FUTEX2_NUMA | FUTEX2_MPOL);
>> }
>> static void usage(char *prog)
>> @@ -184,16 +190,16 @@ int main(int argc, char *argv[])
>> /* FUTEX2_NUMA futex must be 8-byte aligned */
>> ksft_print_msg("Mis-aligned futex\n");
>> - test_futex(futex_ptr + mem_size - 4, 1);
>> + test_futex(futex_ptr + mem_size - 4, 22);
>> futex_numa->numa = FUTEX_NO_NODE;
>> mprotect(futex_ptr, mem_size, PROT_READ);
>> ksft_print_msg("Memory, RO\n");
>> - test_futex(futex_ptr, 1);
>> + test_futex(futex_ptr, 14);
>> mprotect(futex_ptr, mem_size, PROT_NONE);
>> ksft_print_msg("Memory, no access\n");
>> - test_futex(futex_ptr, 1);
>> + test_futex(futex_ptr, 14);
>> mprotect(futex_ptr, mem_size, PROT_READ | PROT_WRITE);
>> ksft_print_msg("Memory back to RW\n");
>
> I believe it is better to use the error number mnemonic (EINVAL &
> EFAULT) instead of 22 and 14 as argument to make the code easier to read.
>
Good call, applied.
Thanks!
André
> Cheers,
> Longman
>
Powered by blists - more mailing lists