2023-10-27 14:22:33

by Alexey Dobriyan

[permalink] [raw]
Subject: [PATCH 1/2] proc: fix proc-empty-vm test with vsyscall

* fix embarassing /proc/*/smaps test bug due to a typo in variable name
it tested only the first line of the output if vsyscall is enabled:

ffffffffff600000-ffffffffff601000 r-xp ...

so test passed but tested only VMA location and permissions.

* add "KSM" entry, unnoticed because (1)

* swap "r-xp" and "--xp" vsyscall test strings,
also unnoticed because (1)

Signed-off-by: Alexey Dobriyan <[email protected]>
---

tools/testing/selftests/proc/proc-empty-vm.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

--- a/tools/testing/selftests/proc/proc-empty-vm.c
+++ b/tools/testing/selftests/proc/proc-empty-vm.c
@@ -60,7 +60,7 @@ static const char proc_pid_maps_vsyscall_2[] =
static const char proc_pid_smaps_vsyscall_0[] = "";

static const char proc_pid_smaps_vsyscall_1[] =
-"ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall]\n"
+"ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0 [vsyscall]\n"
"Size: 4 kB\n"
"KernelPageSize: 4 kB\n"
"MMUPageSize: 4 kB\n"
@@ -73,6 +73,7 @@ static const char proc_pid_smaps_vsyscall_1[] =
"Private_Dirty: 0 kB\n"
"Referenced: 0 kB\n"
"Anonymous: 0 kB\n"
+"KSM: 0 kB\n"
"LazyFree: 0 kB\n"
"AnonHugePages: 0 kB\n"
"ShmemPmdMapped: 0 kB\n"
@@ -90,7 +91,7 @@ static const char proc_pid_smaps_vsyscall_1[] =
;

static const char proc_pid_smaps_vsyscall_2[] =
-"ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0 [vsyscall]\n"
+"ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall]\n"
"Size: 4 kB\n"
"KernelPageSize: 4 kB\n"
"MMUPageSize: 4 kB\n"
@@ -103,6 +104,7 @@ static const char proc_pid_smaps_vsyscall_2[] =
"Private_Dirty: 0 kB\n"
"Referenced: 0 kB\n"
"Anonymous: 0 kB\n"
+"KSM: 0 kB\n"
"LazyFree: 0 kB\n"
"AnonHugePages: 0 kB\n"
"ShmemPmdMapped: 0 kB\n"
@@ -244,10 +246,10 @@ static int test_proc_pid_smaps(pid_t pid)
if (g_vsyscall == 0) {
assert(rv == 0);
} else {
- size_t len = strlen(g_proc_pid_maps_vsyscall);
+ size_t len = strlen(g_proc_pid_smaps_vsyscall);
/* TODO "ProtectionKey:" */
assert(rv > len);
- assert(memcmp(buf, g_proc_pid_maps_vsyscall, len) == 0);
+ assert(memcmp(buf, g_proc_pid_smaps_vsyscall, len) == 0);
}
return EXIT_SUCCESS;
}


2023-10-27 14:26:52

by Alexey Dobriyan

[permalink] [raw]
Subject: [PATCH 2/2] proc: test ProtectionKey in proc-empty-vm test

From: Swarup Laxman Kotiaklapudi <[email protected]>

Check ProtectionKey field in /proc/*/smaps output, if system supports
protection keys feature.

[test support in the beginning of the program,
use syscall, not glibc pkey_alloc(3) which may not compile,
--adobriyan]

Signed-off-by: Swarup Laxman Kotiaklapudi <[email protected]>
Signed-off-by: Alexey Dobriyan <[email protected]>
---

tools/testing/selftests/proc/proc-empty-vm.c | 79 ++++++++++++++++++++-------
1 file changed, 61 insertions(+), 18 deletions(-)

--- a/tools/testing/selftests/proc/proc-empty-vm.c
+++ b/tools/testing/selftests/proc/proc-empty-vm.c
@@ -23,6 +23,9 @@
* /proc/${pid}/smaps
* /proc/${pid}/smaps_rollup
*/
+#undef _GNU_SOURCE
+#define _GNU_SOURCE
+
#undef NDEBUG
#include <assert.h>
#include <errno.h>
@@ -34,6 +37,7 @@
#include <sys/mman.h>
#include <sys/ptrace.h>
#include <sys/resource.h>
+#include <sys/syscall.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
@@ -42,6 +46,43 @@
#define TEST_VSYSCALL
#endif

+#if defined __amd64__
+ #ifndef SYS_pkey_alloc
+ #define SYS_pkey_alloc 330
+ #endif
+ #ifndef SYS_pkey_free
+ #define SYS_pkey_free 331
+ #endif
+#elif defined __i386__
+ #ifndef SYS_pkey_alloc
+ #define SYS_pkey_alloc 381
+ #endif
+ #ifndef SYS_pkey_free
+ #define SYS_pkey_free 382
+ #endif
+#else
+ #error "SYS_pkey_alloc"
+#endif
+
+static int g_protection_key_support;
+
+static int protection_key_support(void)
+{
+ long rv = syscall(SYS_pkey_alloc, 0, 0);
+ if (rv > 0) {
+ syscall(SYS_pkey_free, (int)rv);
+ return 1;
+ } else if (rv == -1 && errno == ENOSYS) {
+ return 0;
+ } else if (rv == -1 && errno == EINVAL) {
+ // ospke=n
+ return 0;
+ } else {
+ fprintf(stderr, "%s: error: rv %ld, errno %d\n", __func__, rv, errno);
+ exit(EXIT_FAILURE);
+ }
+}
+
/*
* 0: vsyscall VMA doesn't exist vsyscall=none
* 1: vsyscall VMA is --xp vsyscall=xonly
@@ -84,10 +125,6 @@ static const char proc_pid_smaps_vsyscall_1[] =
"SwapPss: 0 kB\n"
"Locked: 0 kB\n"
"THPeligible: 0\n"
-/*
- * "ProtectionKey:" field is conditional. It is possible to check it as well,
- * but I don't have such machine.
- */
;

static const char proc_pid_smaps_vsyscall_2[] =
@@ -115,10 +152,6 @@ static const char proc_pid_smaps_vsyscall_2[] =
"SwapPss: 0 kB\n"
"Locked: 0 kB\n"
"THPeligible: 0\n"
-/*
- * "ProtectionKey:" field is conditional. It is possible to check it as well,
- * but I'm too tired.
- */
;

static void sigaction_SIGSEGV(int _, siginfo_t *__, void *___)
@@ -240,19 +273,27 @@ static int test_proc_pid_smaps(pid_t pid)
}
perror("open /proc/${pid}/smaps");
return EXIT_FAILURE;
+ }
+ ssize_t rv = read(fd, buf, sizeof(buf));
+ close(fd);
+
+ assert(0 <= rv);
+ assert(rv <= sizeof(buf));
+
+ if (g_vsyscall == 0) {
+ assert(rv == 0);
} else {
- ssize_t rv = read(fd, buf, sizeof(buf));
- close(fd);
- if (g_vsyscall == 0) {
- assert(rv == 0);
- } else {
- size_t len = strlen(g_proc_pid_smaps_vsyscall);
- /* TODO "ProtectionKey:" */
- assert(rv > len);
- assert(memcmp(buf, g_proc_pid_smaps_vsyscall, len) == 0);
+ size_t len = strlen(g_proc_pid_smaps_vsyscall);
+ assert(rv > len);
+ assert(memcmp(buf, g_proc_pid_smaps_vsyscall, len) == 0);
+
+ if (g_protection_key_support) {
+#define PROTECTION_KEY "ProtectionKey: 0\n"
+ assert(memmem(buf, rv, PROTECTION_KEY, strlen(PROTECTION_KEY)));
}
- return EXIT_SUCCESS;
}
+
+ return EXIT_SUCCESS;
}

static const char g_smaps_rollup[] =
@@ -419,6 +460,8 @@ int main(void)
abort();
}

+ g_protection_key_support = protection_key_support();
+
pid_t pid = fork();
if (pid == -1) {
perror("fork");

2023-10-30 17:28:36

by swarup

[permalink] [raw]
Subject: Re: [PATCH 2/2] proc: test ProtectionKey in proc-empty-vm test

esOn Fri, Oct 27, 2023 at 05:26:24PM +0300, Alexey Dobriyan wrote:
> From: Swarup Laxman Kotiaklapudi <[email protected]>
>
> Check ProtectionKey field in /proc/*/smaps output, if system supports
> protection keys feature.
>
> [test support in the beginning of the program,
> use syscall, not glibc pkey_alloc(3) which may not compile,
> --adobriyan]
>
> Signed-off-by: Swarup Laxman Kotiaklapudi <[email protected]>
> Signed-off-by: Alexey Dobriyan <[email protected]>
> ---
>
> tools/testing/selftests/proc/proc-empty-vm.c | 79 ++++++++++++++++++++-------
> 1 file changed, 61 insertions(+), 18 deletions(-)
>
> --- a/tools/testing/selftests/proc/proc-empty-vm.c
> +++ b/tools/testing/selftests/proc/proc-empty-vm.c
> @@ -23,6 +23,9 @@
> * /proc/${pid}/smaps
> * /proc/${pid}/smaps_rollup
> */
> +#undef _GNU_SOURCE
> +#define _GNU_SOURCE
> +
> #undef NDEBUG
> #include <assert.h>
> #include <errno.h>
> @@ -34,6 +37,7 @@
> #include <sys/mman.h>
> #include <sys/ptrace.h>
> #include <sys/resource.h>
> +#include <sys/syscall.h>
> #include <sys/types.h>
> #include <sys/wait.h>
> #include <unistd.h>
> @@ -42,6 +46,43 @@
> #define TEST_VSYSCALL
> #endif
>
> +#if defined __amd64__
> + #ifndef SYS_pkey_alloc
> + #define SYS_pkey_alloc 330
> + #endif
> + #ifndef SYS_pkey_free
> + #define SYS_pkey_free 331
> + #endif
> +#elif defined __i386__
> + #ifndef SYS_pkey_alloc
> + #define SYS_pkey_alloc 381
> + #endif
> + #ifndef SYS_pkey_free
> + #define SYS_pkey_free 382
> + #endif
> +#else
> + #error "SYS_pkey_alloc"
> +#endif
> +
> +static int g_protection_key_support;
> +
> +static int protection_key_support(void)
> +{
> + long rv = syscall(SYS_pkey_alloc, 0, 0);
> + if (rv > 0) {
> + syscall(SYS_pkey_free, (int)rv);
> + return 1;
> + } else if (rv == -1 && errno == ENOSYS) {
> + return 0;
> + } else if (rv == -1 && errno == EINVAL) {
> + // ospke=n
> + return 0;
> + } else {
> + fprintf(stderr, "%s: error: rv %ld, errno %d\n", __func__, rv, errno);
> + exit(EXIT_FAILURE);
> + }
> +}
> +
> /*
> * 0: vsyscall VMA doesn't exist vsyscall=none
> * 1: vsyscall VMA is --xp vsyscall=xonly
> @@ -84,10 +125,6 @@ static const char proc_pid_smaps_vsyscall_1[] =
> "SwapPss: 0 kB\n"
> "Locked: 0 kB\n"
> "THPeligible: 0\n"
> -/*
> - * "ProtectionKey:" field is conditional. It is possible to check it as well,
> - * but I don't have such machine.
> - */
> ;
>
> static const char proc_pid_smaps_vsyscall_2[] =
> @@ -115,10 +152,6 @@ static const char proc_pid_smaps_vsyscall_2[] =
> "SwapPss: 0 kB\n"
> "Locked: 0 kB\n"
> "THPeligible: 0\n"
> -/*
> - * "ProtectionKey:" field is conditional. It is possible to check it as well,
> - * but I'm too tired.
> - */
> ;
>
> static void sigaction_SIGSEGV(int _, siginfo_t *__, void *___)
> @@ -240,19 +273,27 @@ static int test_proc_pid_smaps(pid_t pid)
> }
> perror("open /proc/${pid}/smaps");
> return EXIT_FAILURE;
> + }
> + ssize_t rv = read(fd, buf, sizeof(buf));
> + close(fd);
> +
> + assert(0 <= rv);
> + assert(rv <= sizeof(buf));
> +
> + if (g_vsyscall == 0) {
> + assert(rv == 0);
> } else {
> - ssize_t rv = read(fd, buf, sizeof(buf));
> - close(fd);
> - if (g_vsyscall == 0) {
> - assert(rv == 0);
> - } else {
> - size_t len = strlen(g_proc_pid_smaps_vsyscall);
> - /* TODO "ProtectionKey:" */
> - assert(rv > len);
> - assert(memcmp(buf, g_proc_pid_smaps_vsyscall, len) == 0);
> + size_t len = strlen(g_proc_pid_smaps_vsyscall);
> + assert(rv > len);
> + assert(memcmp(buf, g_proc_pid_smaps_vsyscall, len) == 0);
> +
> + if (g_protection_key_support) {
> +#define PROTECTION_KEY "ProtectionKey: 0\n"
> + assert(memmem(buf, rv, PROTECTION_KEY, strlen(PROTECTION_KEY)));
> }
> - return EXIT_SUCCESS;
> }
> +
> + return EXIT_SUCCESS;
> }
>
> static const char g_smaps_rollup[] =
> @@ -419,6 +460,8 @@ int main(void)
> abort();
> }
>
> + g_protection_key_support = protection_key_support();
> +
> pid_t pid = fork();
> if (pid == -1) {
> perror("fork");

Reviewed-by: Swarup Laxman Kotikalapudi<[email protected]>
Tested-by: Swarup Laxman Kotikalapudi<[email protected]>

Hi Alexey,
Thanks a lot for correcting the fix.
Thanks,
Swarup