Re: [LTP] [PATCH v2] security/dirtyc0w_shmem: Add new test for CVE-2022-2590

David Hildenbrand
2022-11-25T10:18:10Z

On 25.11.22 10:53, Martin Doucha wrote:
> Hi,
>

Hi Martin,

> On 23. 11. 22 11:35, David Hildenbrand wrote:
>> + pid = SAFE_FORK();
>> + if (!pid) {
>> + SAFE_SETGID(nobody_gid);
>> + SAFE_SETUID(nobody_uid);
>> + SAFE_EXECLP("dirtyc0w_shmem_child", "dirtyc0w_shmem_child", NULL);
>
> Manpage says that the last argument of execlp() must be (char*)NULL,
> including the explicit typecast.

$ git grep SAFE_EXECLP | grep NULL
testcases/kernel/connectors/pec/event_generator.c: SAFE_EXECLP(prog_name, prog_name, "-e", "exec", "-n", buf, NULL);
testcases/kernel/security/dirtyc0w/dirtyc0w.c: SAFE_EXECLP("dirtyc0w_child", "dirtyc0w_child",NULL);
testcases/kernel/security/dirtyc0w_shmem/dirtyc0w_shmem.c: SAFE_EXECLP("dirtyc0w_shmem_child", "dirtyc0w_shmem_child", NULL);
testcases/kernel/syscalls/pipe2/pipe2_02.c: SAFE_EXECLP(TESTBIN, TESTBIN, buf, NULL);
testcases/kernel/syscalls/setpgid/setpgid03.c: SAFE_EXECLP(TEST_APP, TEST_APP, NULL);
testcases/kernel/syscalls/setrlimit/setrlimit04.c: SAFE_EXECLP("/bin/true", "/bin/true", NULL);

>
>> +#else /* UFFD_FEATURE_MINOR_SHMEM */
>> +#include "tst_test.h"
>> +TST_TEST_TCONF("System does not have userfaultfd minor fault support for shmem");
>> +#endif /* UFFD_FEATURE_MINOR_SHMEM */
>
> When the child exits through this TST_TEST_TCONF(), the
> TST_CHECKPOINT_WAIT() in parent will fail. The parent process should not
> even fork() when UFFD_FEATURE_MINOR_SHMEM is not defined in config.h.

Thanks, you're right, that's the remaining case that doesn't
make the checkpoint happy.

I tried handling TCONF in the parent and it got all very ugly.
The following should do the trick:

From fb13df0ea9e477b8e903d3ef4df317e548200a86 Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@redhat.com>
Date: Fri, 25 Nov 2022 05:12:26 -0500
Subject: [PATCH v1] security/dirtyc0w_shmem: Fix test result when
UFFD_FEATURE_MINOR_SHMEM is missing

We have make the checkpoint happy, otherwise our parent process will run
into a timeout.

Reported-by: Martin Doucha <mdoucha@suse.cz>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
.../security/dirtyc0w_shmem/dirtyc0w_shmem_child.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/security/dirtyc0w_shmem/dirtyc0w_shmem_child.c b/testcases/kernel/security/dirtyc0w_shmem/dirtyc0w_shmem_child.c
index cb2e9df0c..eac128e5d 100644
--- a/testcases/kernel/security/dirtyc0w_shmem/dirtyc0w_shmem_child.c
+++ b/testcases/kernel/security/dirtyc0w_shmem/dirtyc0w_shmem_child.c
@@ -24,12 +24,12 @@
#include <linux/userfaultfd.h>
#endif

-#ifdef UFFD_FEATURE_MINOR_SHMEM
-
#define TST_NO_DEFAULT_MAIN
#include "tst_test.h"
#include "tst_safe_macros.h"
#include "tst_safe_pthread.h"
+
+#ifdef UFFD_FEATURE_MINOR_SHMEM
#include "lapi/syscalls.h"

#define TMP_DIR "tmp_dirtyc0w_shmem"
@@ -236,6 +236,10 @@ int main(void)
return 0;
}
#else /* UFFD_FEATURE_MINOR_SHMEM */
-#include "tst_test.h"
-TST_TEST_TCONF("System does not have userfaultfd minor fault support for shmem");
+int main(void)
+{
+ tst_reinit();
+ TST_CHECKPOINT_WAKE(0);
+ tst_brk(TCONF, "System does not have userfaultfd minor fault support for shmem");
+}
#endif /* UFFD_FEATURE_MINOR_SHMEM */
--
2.38.1

Which gives me:

# ./dirtyc0w_shmem
tst_test.c:1552: TINFO: Timeout per run is 0h 02m 30s
dirtyc0w_shmem_child.c:243: TCONF: System does not have userfaultfd minor fault support for shmem
dirtyc0w_shmem.c:95: TINFO: Early child process exit

Summary:
passed 0
failed 0
broken 0
skipped 1
warnings 0

--
Thanks,

David / dhildenb