[LTP] [PATCH v2 1/2] lapi: move pkey.h declarations inside the lapi/ folder

[LTP] [PATCH v2 1/2] lapi: move pkey.h declarations inside the lapi/ folder

[Li Wang]

Signed-off-by: Li Wang <liwang@redhat.com>
---
 .../kernel/syscalls/pkeys => include/lapi}/pkey.h      | 10 +++++-----
 testcases/kernel/syscalls/pkeys/pkey01.c               |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)
 rename {testcases/kernel/syscalls/pkeys => include/lapi}/pkey.h (88%)

diff --git a/testcases/kernel/syscalls/pkeys/pkey.h b/include/lapi/pkey.h
similarity index 88%
rename from testcases/kernel/syscalls/pkeys/pkey.h
rename to include/lapi/pkey.h
index 6e32326b6..e0dc20dce 100644
--- a/testcases/kernel/syscalls/pkeys/pkey.h
+++ b/include/lapi/pkey.h
@@ -1,11 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) 2019 Red Hat, Inc.
- * Copyright (c) Linux Test Project, 2019
+ * Copyright (c) 2019-2024 Red Hat, Inc.
+ * Copyright (c) Linux Test Project, 2019-2024
  */
 
-#ifndef PKEYS_H
-#define PKEYS_H
+#ifndef PKEYS_H__
+#define PKEYS_H__
 
 #include "tst_test.h"
 #include "lapi/syscalls.h"
@@ -53,4 +53,4 @@ static inline void check_pkey_support(void)
 	ltp_pkey_free(pkey);
 }
 
-#endif /* PKEYS_H */
+#endif /* PKEYS_H__ */
diff --git a/testcases/kernel/syscalls/pkeys/pkey01.c b/testcases/kernel/syscalls/pkeys/pkey01.c
index 0159822e1..a6a739668 100644
--- a/testcases/kernel/syscalls/pkeys/pkey01.c
+++ b/testcases/kernel/syscalls/pkeys/pkey01.c
@@ -29,7 +29,7 @@
 #include <sys/mman.h>
 #include <sys/wait.h>
 
-#include "pkey.h"
+#include "lapi/pkey.h"
 
 #define TEST_FILE "pkey_testfile"
 #define STR "abcdefghijklmnopqrstuvwxyz12345\n"
-- 
2.45.2


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 2/2] mseal01: handle more possible errnos when pkey_alloc gets fail

[Li Wang]

Signed-off-by: Li Wang <liwang@redhat.com>
---
 testcases/kernel/syscalls/mseal/mseal01.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/testcases/kernel/syscalls/mseal/mseal01.c b/testcases/kernel/syscalls/mseal/mseal01.c
index eb2f4d588..1de9fb723 100644
--- a/testcases/kernel/syscalls/mseal/mseal01.c
+++ b/testcases/kernel/syscalls/mseal/mseal01.c
@@ -25,6 +25,7 @@
 
 #include "tst_test.h"
 #include "lapi/syscalls.h"
+#include "lapi/pkey.h"
 
 #define MEMPAGES 8
 #define MEMSEAL 2
@@ -46,25 +47,21 @@ static void test_mprotect(void)
 
 static void test_pkey_mprotect(void)
 {
-	int ret;
 	int pkey;
 
-	pkey = tst_syscall(__NR_pkey_alloc, 0, 0);
-	if (pkey == -1) {
-		if (errno == EINVAL)
-			tst_brk(TCONF, "pku is not supported on this CPU");
+	check_pkey_support();
 
-		tst_brk(TBROK | TERRNO, "pkey_alloc() error");
-	}
+	pkey = ltp_pkey_alloc(0, 0);
+	if (pkey == -1)
+		tst_brk(TBROK | TERRNO, "pkey_alloc failed");
 
-	TST_EXP_FAIL(tst_syscall(__NR_pkey_mprotect,
+	TST_EXP_FAIL(ltp_pkey_mprotect(
 		mem_addr, mem_size,
 		PROT_NONE,
 		pkey),
 		EPERM);
 
-	ret = tst_syscall(__NR_pkey_free, pkey);
-	if (ret == -1)
+	if (ltp_pkey_free(pkey) == -1)
 		tst_brk(TBROK | TERRNO, "pkey_free() error");
 }
 
@@ -150,7 +147,6 @@ static struct tst_test test = {
 	.test = run,
 	.tcnt = ARRAY_SIZE(tcases),
 	.setup = setup,
-	.min_kver = "6.10",
 	.forks_child = 1,
 };
 
-- 
2.45.2


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 1/2] lapi: move pkey.h declarations inside the lapi/ folder

[Petr Vorel]

Hi Li,

obviously correct, thanks!
Reviewed-by: Petr Vorel <pvorel@suse.cz>

BTW wouldn't be worth to rewrite mseal01.c to support both raw syscall and libc
wrapper via .test_variants?

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 2/2] mseal01: handle more possible errnos when pkey_alloc gets fail

[Petr Vorel]

Hi Li,

...
>  static void test_pkey_mprotect(void)
>  {
> -	int ret;
>  	int pkey;

> -	pkey = tst_syscall(__NR_pkey_alloc, 0, 0);
> -	if (pkey == -1) {
> -		if (errno == EINVAL)
> -			tst_brk(TCONF, "pku is not supported on this CPU");
> +	check_pkey_support();

> -		tst_brk(TBROK | TERRNO, "pkey_alloc() error");
> -	}
> +	pkey = ltp_pkey_alloc(0, 0);
> +	if (pkey == -1)
> +		tst_brk(TBROK | TERRNO, "pkey_alloc failed");

> -	TST_EXP_FAIL(tst_syscall(__NR_pkey_mprotect,
> +	TST_EXP_FAIL(ltp_pkey_mprotect(
>  		mem_addr, mem_size,
>  		PROT_NONE,
>  		pkey),
>  		EPERM);

Reviewed-by: Petr Vorel <pvorel@suse.cz>

But as I wrote in first patch, I would consider better to rewrite test to use
.test_variants = 2 to test both raw syscall and libc wrapper. That would require
to rewrite the lapi header. And it'd be then safer if pkey01.c would use always
raw syscall variant, no need for not standard "ltp_pkey_mprotect()" name.

> -	ret = tst_syscall(__NR_pkey_free, pkey);
> -	if (ret == -1)
> +	if (ltp_pkey_free(pkey) == -1)
>  		tst_brk(TBROK | TERRNO, "pkey_free() error");
>  }

> @@ -150,7 +147,6 @@ static struct tst_test test = {
>  	.test = run,
>  	.tcnt = ARRAY_SIZE(tcases),
>  	.setup = setup,
> -	.min_kver = "6.10",
This change would be worth to mention in the changelog.

Kind regards,
Petr

>  	.forks_child = 1,
>  };

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 1/2] lapi: move pkey.h declarations inside the lapi/ folder

[Li Wang]

Hi Petr,

On Mon, Aug 5, 2024 at 9:55 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> obviously correct, thanks!
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> BTW wouldn't be worth to rewrite mseal01.c to support both raw syscall and
> libc
> wrapper via .test_variants?
>

Yes, we could do that, but as mseal is too new, I haven't seen a wrapper
created in Glibc so far.
Probably we just record this in code comments as TODO?


-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 2/2] mseal01: handle more possible errnos when pkey_alloc gets fail

[Li Wang]

On Mon, Aug 5, 2024 at 9:59 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> ...
> >  static void test_pkey_mprotect(void)
> >  {
> > -     int ret;
> >       int pkey;
>
> > -     pkey = tst_syscall(__NR_pkey_alloc, 0, 0);
> > -     if (pkey == -1) {
> > -             if (errno == EINVAL)
> > -                     tst_brk(TCONF, "pku is not supported on this CPU");
> > +     check_pkey_support();
>
> > -             tst_brk(TBROK | TERRNO, "pkey_alloc() error");
> > -     }
> > +     pkey = ltp_pkey_alloc(0, 0);
> > +     if (pkey == -1)
> > +             tst_brk(TBROK | TERRNO, "pkey_alloc failed");
>
> > -     TST_EXP_FAIL(tst_syscall(__NR_pkey_mprotect,
> > +     TST_EXP_FAIL(ltp_pkey_mprotect(
> >               mem_addr, mem_size,
> >               PROT_NONE,
> >               pkey),
> >               EPERM);
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> But as I wrote in first patch, I would consider better to rewrite test to
> use
> .test_variants = 2 to test both raw syscall and libc wrapper. That would
> require
>

Sure, as I reply in 1/2 we can delay the work.



> to rewrite the lapi header. And it'd be then safer if pkey01.c would use
> always
> raw syscall variant, no need for not standard "ltp_pkey_mprotect()" name.
>

Good point, I will send a follow-up patch to resolve that.



>
> > -     ret = tst_syscall(__NR_pkey_free, pkey);
> > -     if (ret == -1)
> > +     if (ltp_pkey_free(pkey) == -1)
> >               tst_brk(TBROK | TERRNO, "pkey_free() error");
> >  }
>
> > @@ -150,7 +147,6 @@ static struct tst_test test = {
> >       .test = run,
> >       .tcnt = ARRAY_SIZE(tcases),
> >       .setup = setup,
> > -     .min_kver = "6.10",
> This change would be worth to mention in the changelog.
>

+1


> Kind regards,
> Petr
>
> >       .forks_child = 1,
> >  };
>
>

-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH] pkey: remove the ltp_ prefix from pkey functions

[Li Wang]

Suggested-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Li Wang <liwang@redhat.com>
---
 include/lapi/pkey.h                       | 14 +++++---------
 testcases/kernel/syscalls/mseal/mseal01.c |  6 +++---
 testcases/kernel/syscalls/pkeys/pkey01.c  |  8 ++++----
 3 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/include/lapi/pkey.h b/include/lapi/pkey.h
index e0dc20dce..933dff34a 100644
--- a/include/lapi/pkey.h
+++ b/include/lapi/pkey.h
@@ -17,29 +17,25 @@
 #endif
 
 #ifndef HAVE_PKEY_MPROTECT
-inline int ltp_pkey_mprotect(void *addr, size_t len, int prot, int pkey)
+inline int pkey_mprotect(void *addr, size_t len, int prot, int pkey)
 {
 	return tst_syscall(__NR_pkey_mprotect, addr, len, prot, pkey);
 }
 
-inline int ltp_pkey_alloc(unsigned int flags, unsigned int access_rights)
+inline int pkey_alloc(unsigned int flags, unsigned int access_rights)
 {
 	return tst_syscall(__NR_pkey_alloc, flags, access_rights);
 }
 
-inline int ltp_pkey_free(int pkey)
+inline int pkey_free(int pkey)
 {
 	return tst_syscall(__NR_pkey_free, pkey);
 }
-#else
-#define ltp_pkey_alloc pkey_alloc
-#define ltp_pkey_free pkey_free
-#define ltp_pkey_mprotect pkey_mprotect
 #endif /* HAVE_PKEY_MPROTECT */
 
 static inline void check_pkey_support(void)
 {
-	int pkey = ltp_pkey_alloc(0, 0);
+	int pkey = tst_syscall(__NR_pkey_alloc, 0, 0);
 
 	if (pkey == -1) {
 		if (errno == ENOSYS)
@@ -50,7 +46,7 @@ static inline void check_pkey_support(void)
 			tst_brk(TCONF, "pkeys are not available for test");
 	}
 
-	ltp_pkey_free(pkey);
+	tst_syscall(__NR_pkey_free, pkey);
 }
 
 #endif /* PKEYS_H__ */
diff --git a/testcases/kernel/syscalls/mseal/mseal01.c b/testcases/kernel/syscalls/mseal/mseal01.c
index 1de9fb723..94e3e4b23 100644
--- a/testcases/kernel/syscalls/mseal/mseal01.c
+++ b/testcases/kernel/syscalls/mseal/mseal01.c
@@ -51,17 +51,17 @@ static void test_pkey_mprotect(void)
 
 	check_pkey_support();
 
-	pkey = ltp_pkey_alloc(0, 0);
+	pkey = pkey_alloc(0, 0);
 	if (pkey == -1)
 		tst_brk(TBROK | TERRNO, "pkey_alloc failed");
 
-	TST_EXP_FAIL(ltp_pkey_mprotect(
+	TST_EXP_FAIL(pkey_mprotect(
 		mem_addr, mem_size,
 		PROT_NONE,
 		pkey),
 		EPERM);
 
-	if (ltp_pkey_free(pkey) == -1)
+	if (pkey_free(pkey) == -1)
 		tst_brk(TBROK | TERRNO, "pkey_free() error");
 }
 
diff --git a/testcases/kernel/syscalls/pkeys/pkey01.c b/testcases/kernel/syscalls/pkeys/pkey01.c
index a6a739668..60f36706c 100644
--- a/testcases/kernel/syscalls/pkeys/pkey01.c
+++ b/testcases/kernel/syscalls/pkeys/pkey01.c
@@ -142,12 +142,12 @@ static void pkey_test(struct tcase *tc, struct mmap_param *mpa)
 
 	buffer = SAFE_MMAP(NULL, size, mpa->prot, mpa->flags, fd, 0);
 
-	pkey = ltp_pkey_alloc(tc->flags, tc->access_rights);
+	pkey = pkey_alloc(tc->flags, tc->access_rights);
 	if (pkey == -1)
 		tst_brk(TBROK | TERRNO, "pkey_alloc failed");
 
 	tst_res(TINFO, "Set %s on (%s) buffer", tc->name, flag_to_str(mpa->flags));
-	if (ltp_pkey_mprotect(buffer, size, mpa->prot, pkey) == -1)
+	if (pkey_mprotect(buffer, size, mpa->prot, pkey) == -1)
 		tst_brk(TBROK | TERRNO, "pkey_mprotect failed");
 
 	pid = SAFE_FORK();
@@ -176,7 +176,7 @@ static void pkey_test(struct tcase *tc, struct mmap_param *mpa)
                 tst_res(TFAIL, "Child: %s", tst_strstatus(status));
 
 	tst_res(TINFO, "Remove %s from the buffer", tc->name);
-	if (ltp_pkey_mprotect(buffer, size, mpa->prot, 0x0) == -1)
+	if (pkey_mprotect(buffer, size, mpa->prot, 0x0) == -1)
 		tst_brk(TBROK | TERRNO, "pkey_mprotect failed");
 
 	switch (mpa->prot) {
@@ -199,7 +199,7 @@ static void pkey_test(struct tcase *tc, struct mmap_param *mpa)
 
 	SAFE_MUNMAP(buffer, size);
 
-	if (ltp_pkey_free(pkey) == -1)
+	if (pkey_free(pkey) == -1)
 		tst_brk(TBROK | TERRNO, "pkey_free failed");
 }
 
-- 
2.45.2


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] pkey: remove the ltp_ prefix from pkey functions

[Petr Vorel]

Hi Li,

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Thanks!

Other thing which should follow is to use .test_variants.
Feel free to send a patch or I'll do it later.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] pkey: remove the ltp_ prefix from pkey functions

[Li Wang]

Hi Petr

On Tue, Aug 6, 2024 at 2:19 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Thanks!
>
> Other thing which should follow is to use .test_variants.
>

As I pointed out in the last email, mseal() is too new to libc, there is no
wrapper in Glibc so far.
Should we add .test_variants now?


-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] pkey: remove the ltp_ prefix from pkey functions

[Li Wang]

On Tue, Aug 6, 2024 at 3:37 PM Li Wang <liwang@redhat.com> wrote:

> Hi Petr
>
> On Tue, Aug 6, 2024 at 2:19 PM Petr Vorel <pvorel@suse.cz> wrote:
>
>> Hi Li,
>>
>> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>> Thanks!
>>
>> Other thing which should follow is to use .test_variants.
>>
>
> As I pointed out in the last email, mseal() is too new to libc, there is
> no wrapper in Glibc so far.
> Should we add .test_variants now?
>

Anyway, I boldly pushed the patches and added a TODO in the comments.
@Petr, I would be happy to review if you plan to write more to cover libc
wrapper.

>
--- a/testcases/kernel/syscalls/mseal/mseal01.c
+++ b/testcases/kernel/syscalls/mseal/mseal01.c
@@ -19,12 +19,15 @@
  *   when users don’t have write permission to the memory
  *
  * Any of the described actions is recognized via EPERM errno.
+ *
+ * TODO: support both raw syscall and libc wrapper via .test_variants.
  */


-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] pkey: remove the ltp_ prefix from pkey functions

[Petr Vorel]

> On Tue, Aug 6, 2024 at 3:37 PM Li Wang <liwang@redhat.com> wrote:

> > Hi Petr

> > On Tue, Aug 6, 2024 at 2:19 PM Petr Vorel <pvorel@suse.cz> wrote:

> >> Hi Li,

> >> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> >> Thanks!

> >> Other thing which should follow is to use .test_variants.


> > As I pointed out in the last email, mseal() is too new to libc, there is
> > no wrapper in Glibc so far.
> > Should we add .test_variants now?

> Anyway, I boldly pushed the patches and added a TODO in the comments.
> @Petr, I would be happy to review if you plan to write more to cover libc
> wrapper.

I'm sorry this was meant only for functions in lapi/pkey.h, which have
their fallback variants guarded by #ifndef HAVE_PKEY_MPROTECT.
I'll send a patch for it.

Also, once mseal() gets to any libc, it might need to be guarded (depending if
we load header where will be added).

Kind regards,
Petr

> --- a/testcases/kernel/syscalls/mseal/mseal01.c
> +++ b/testcases/kernel/syscalls/mseal/mseal01.c
> @@ -19,12 +19,15 @@
>   *   when users don’t have write permission to the memory
>   *
>   * Any of the described actions is recognized via EPERM errno.
> + *
> + * TODO: support both raw syscall and libc wrapper via .test_variants.
>   */

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp