[LTP] [PATCH v2 0/3] fanotify14 on SELinux fix + lib source merge

[LTP] [PATCH v2 0/3] fanotify14 on SELinux fix + lib source merge

[Petr Vorel]

Hi,

changes v1->v2:
* New commit: lib: Merge security related sources
* Add selinux to tst_security.c instead of it's own C file.
* Do not include library header in fanotify14 (not needed)

Mete Durlu (1):
  fanotify14: fix anonymous pipe testcases

Petr Vorel (2):
  lib: Merge security related sources
  lib: Add tst_selinux_enforcing()

 include/tst_fips.h                            | 15 ----
 include/tst_lockdown.h                        | 11 ---
 include/tst_security.h                        | 18 ++++
 include/tst_test.h                            |  4 +-
 lib/tst_fips.c                                | 24 -----
 lib/{tst_lockdown.c => tst_security.c}        | 87 ++++++++++++-------
 .../kernel/syscalls/fanotify/fanotify14.c     | 18 +++-
 7 files changed, 92 insertions(+), 85 deletions(-)
 delete mode 100644 include/tst_fips.h
 delete mode 100644 include/tst_lockdown.h
 create mode 100644 include/tst_security.h
 delete mode 100644 lib/tst_fips.c
 rename lib/{tst_lockdown.c => tst_security.c} (77%)

-- 
2.43.0


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

[LTP] [PATCH v2 1/3] lib: Merge security related sources

[Petr Vorel]

Merge FIPS and lockdown related library sources to new tst_security.[ch]
file to shorten number of the files in the library. More security
related code will be added in next commit.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
changes v1->v2:
* New commit: lib: Merge security related sources

I'll send more cleanup in a different patchset.

Kind regards,
Petr

 include/tst_fips.h                     | 15 ------
 include/tst_lockdown.h                 | 11 ----
 include/tst_security.h                 | 17 ++++++
 include/tst_test.h                     |  4 +-
 lib/tst_fips.c                         | 24 ---------
 lib/{tst_lockdown.c => tst_security.c} | 73 +++++++++++++++-----------
 6 files changed, 62 insertions(+), 82 deletions(-)
 delete mode 100644 include/tst_fips.h
 delete mode 100644 include/tst_lockdown.h
 create mode 100644 include/tst_security.h
 delete mode 100644 lib/tst_fips.c
 rename lib/{tst_lockdown.c => tst_security.c} (86%)

diff --git a/include/tst_fips.h b/include/tst_fips.h
deleted file mode 100644
index 881c32391..000000000
--- a/include/tst_fips.h
+++ /dev/null
@@ -1,15 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * Copyright (c) 2021 Petr Vorel <pvorel@suse.cz>
- */
-
-#ifndef TST_FIPS_H__
-#define TST_FIPS_H__
-
-/*
- * Detect whether FIPS enabled
- * @return 0: FIPS not enabled, 1: FIPS enabled
- */
-int tst_fips_enabled(void);
-
-#endif /* TST_FIPS_H__ */
diff --git a/include/tst_lockdown.h b/include/tst_lockdown.h
deleted file mode 100644
index 07e90c1af..000000000
--- a/include/tst_lockdown.h
+++ /dev/null
@@ -1,11 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later
- * Copyright (c) Linux Test Project, 2020-2021
- */
-
-#ifndef TST_LOCKDOWN_H
-#define TST_LOCKDOWN_H
-
-int tst_secureboot_enabled(void);
-int tst_lockdown_enabled(void);
-
-#endif /* TST_LOCKDOWN_H */
diff --git a/include/tst_security.h b/include/tst_security.h
new file mode 100644
index 000000000..438b16dbb
--- /dev/null
+++ b/include/tst_security.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (c) Linux Test Project, 2020-2024
+ */
+
+#ifndef TST_SECURITY_H__
+#define TST_SECURITY_H__
+
+/*
+ * Detect whether FIPS enabled
+ * @return 0: FIPS not enabled, 1: FIPS enabled
+ */
+int tst_fips_enabled(void);
+
+int tst_lockdown_enabled(void);
+int tst_secureboot_enabled(void);
+
+#endif /* TST_SECURITY_H__ */
diff --git a/include/tst_test.h b/include/tst_test.h
index 47b5902f9..98d74d82e 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -40,8 +40,8 @@
 #include "tst_capability.h"
 #include "tst_hugepage.h"
 #include "tst_assert.h"
-#include "tst_lockdown.h"
-#include "tst_fips.h"
+#include "tst_security.h"
+#include "tst_security.h"
 #include "tst_taint.h"
 #include "tst_memutils.h"
 #include "tst_arch.h"
diff --git a/lib/tst_fips.c b/lib/tst_fips.c
deleted file mode 100644
index 82dafef7a..000000000
--- a/lib/tst_fips.c
+++ /dev/null
@@ -1,24 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * Copyright (c) 2021 Petr Vorel <pvorel@suse.cz>
- */
-
-#define TST_NO_DEFAULT_MAIN
-
-#define PATH_FIPS	"/proc/sys/crypto/fips_enabled"
-
-#include "tst_test.h"
-#include "tst_safe_macros.h"
-#include "tst_fips.h"
-
-int tst_fips_enabled(void)
-{
-	int fips = 0;
-
-	if (access(PATH_FIPS, R_OK) == 0) {
-		SAFE_FILE_SCANF(PATH_FIPS, "%d", &fips);
-	}
-
-	tst_res(TINFO, "FIPS: %s", fips ? "on" : "off");
-	return fips;
-}
diff --git a/lib/tst_lockdown.c b/lib/tst_security.c
similarity index 86%
rename from lib/tst_lockdown.c
rename to lib/tst_security.c
index 3126d67bd..0fc704dfa 100644
--- a/lib/tst_lockdown.c
+++ b/lib/tst_security.c
@@ -1,12 +1,21 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) Linux Test Project, 2020-2023
+ * Copyright (c) Linux Test Project, 2020-2024
  */
 
 #define TST_NO_DEFAULT_MAIN
 
+#define PATH_FIPS	"/proc/sys/crypto/fips_enabled"
 #define PATH_LOCKDOWN	"/sys/kernel/security/lockdown"
 
+#if defined(__powerpc64__) || defined(__ppc64__)
+# define SECUREBOOT_VAR "/proc/device-tree/ibm,secure-boot"
+# define VAR_DATA_SIZE 4
+#else
+# define SECUREBOOT_VAR "/sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c"
+# define VAR_DATA_SIZE 5
+#endif
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/mount.h>
@@ -14,41 +23,19 @@
 #include "tst_test.h"
 #include "tst_safe_macros.h"
 #include "tst_safe_stdio.h"
-#include "tst_lockdown.h"
+#include "tst_security.h"
 #include "tst_private.h"
 
-#if defined(__powerpc64__) || defined(__ppc64__)
-# define SECUREBOOT_VAR "/proc/device-tree/ibm,secure-boot"
-# define VAR_DATA_SIZE 4
-#else
-# define SECUREBOOT_VAR "/sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c"
-# define VAR_DATA_SIZE 5
-#endif
-
-int tst_secureboot_enabled(void)
+int tst_fips_enabled(void)
 {
-	int fd;
-	char data[5];
+	int fips = 0;
 
-	if (access(SECUREBOOT_VAR, F_OK)) {
-		tst_res(TINFO, "SecureBoot sysfs file not available");
-		return -1;
+	if (access(PATH_FIPS, R_OK) == 0) {
+		SAFE_FILE_SCANF(PATH_FIPS, "%d", &fips);
 	}
 
-	fd = open(SECUREBOOT_VAR, O_RDONLY);
-
-	if (fd == -1) {
-		tst_res(TINFO | TERRNO,
-			"Cannot open SecureBoot file");
-		return -1;
-	} else if (fd < 0) {
-		tst_brk(TBROK | TERRNO, "Invalid open() return value %d", fd);
-		return -1;
-	}
-	SAFE_READ(1, fd, data, VAR_DATA_SIZE);
-	SAFE_CLOSE(fd);
-	tst_res(TINFO, "SecureBoot: %s", data[VAR_DATA_SIZE - 1] ? "on" : "off");
-	return data[VAR_DATA_SIZE - 1];
+	tst_res(TINFO, "FIPS: %s", fips ? "on" : "off");
+	return fips;
 }
 
 int tst_lockdown_enabled(void)
@@ -86,3 +73,29 @@ int tst_lockdown_enabled(void)
 
 	return ret;
 }
+
+int tst_secureboot_enabled(void)
+{
+	int fd;
+	char data[5];
+
+	if (access(SECUREBOOT_VAR, F_OK)) {
+		tst_res(TINFO, "SecureBoot sysfs file not available");
+		return -1;
+	}
+
+	fd = open(SECUREBOOT_VAR, O_RDONLY);
+
+	if (fd == -1) {
+		tst_res(TINFO | TERRNO,
+			"Cannot open SecureBoot file");
+		return -1;
+	} else if (fd < 0) {
+		tst_brk(TBROK | TERRNO, "Invalid open() return value %d", fd);
+		return -1;
+	}
+	SAFE_READ(1, fd, data, VAR_DATA_SIZE);
+	SAFE_CLOSE(fd);
+	tst_res(TINFO, "SecureBoot: %s", data[VAR_DATA_SIZE - 1] ? "on" : "off");
+	return data[VAR_DATA_SIZE - 1];
+}
-- 
2.43.0


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

Re: [LTP] [PATCH v2 1/3] lib: Merge security related sources

[Cyril Hrubis]

Hi!
> Merge FIPS and lockdown related library sources to new tst_security.[ch]
> file to shorten number of the files in the library. More security
> related code will be added in next commit.

Sound good.

> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> changes v1->v2:
> * New commit: lib: Merge security related sources
> 
> I'll send more cleanup in a different patchset.
> 
> Kind regards,
> Petr
> 
>  include/tst_fips.h                     | 15 ------
>  include/tst_lockdown.h                 | 11 ----
>  include/tst_security.h                 | 17 ++++++
>  include/tst_test.h                     |  4 +-
>  lib/tst_fips.c                         | 24 ---------
>  lib/{tst_lockdown.c => tst_security.c} | 73 +++++++++++++++-----------
>  6 files changed, 62 insertions(+), 82 deletions(-)
>  delete mode 100644 include/tst_fips.h
>  delete mode 100644 include/tst_lockdown.h
>  create mode 100644 include/tst_security.h
>  delete mode 100644 lib/tst_fips.c
>  rename lib/{tst_lockdown.c => tst_security.c} (86%)
> 
> diff --git a/include/tst_fips.h b/include/tst_fips.h
> deleted file mode 100644
> index 881c32391..000000000
> --- a/include/tst_fips.h
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-or-later
> -/*
> - * Copyright (c) 2021 Petr Vorel <pvorel@suse.cz>
> - */
> -
> -#ifndef TST_FIPS_H__
> -#define TST_FIPS_H__
> -
> -/*
> - * Detect whether FIPS enabled
> - * @return 0: FIPS not enabled, 1: FIPS enabled
> - */
> -int tst_fips_enabled(void);
> -
> -#endif /* TST_FIPS_H__ */
> diff --git a/include/tst_lockdown.h b/include/tst_lockdown.h
> deleted file mode 100644
> index 07e90c1af..000000000
> --- a/include/tst_lockdown.h
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later
> - * Copyright (c) Linux Test Project, 2020-2021
> - */
> -
> -#ifndef TST_LOCKDOWN_H
> -#define TST_LOCKDOWN_H
> -
> -int tst_secureboot_enabled(void);
> -int tst_lockdown_enabled(void);
> -
> -#endif /* TST_LOCKDOWN_H */
> diff --git a/include/tst_security.h b/include/tst_security.h
> new file mode 100644
> index 000000000..438b16dbb
> --- /dev/null
> +++ b/include/tst_security.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (c) Linux Test Project, 2020-2024
> + */
> +
> +#ifndef TST_SECURITY_H__
> +#define TST_SECURITY_H__
> +
> +/*
> + * Detect whether FIPS enabled
> + * @return 0: FIPS not enabled, 1: FIPS enabled
> + */
> +int tst_fips_enabled(void);
> +
> +int tst_lockdown_enabled(void);
> +int tst_secureboot_enabled(void);
> +
> +#endif /* TST_SECURITY_H__ */
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 47b5902f9..98d74d82e 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -40,8 +40,8 @@
>  #include "tst_capability.h"
>  #include "tst_hugepage.h"
>  #include "tst_assert.h"
> -#include "tst_lockdown.h"
> -#include "tst_fips.h"
> +#include "tst_security.h"
> +#include "tst_security.h"

Huh, included twice?


Other than that:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 1/3] lib: Merge security related sources

[Petr Vorel]

Hi Cyril,

...

> > +++ b/include/tst_test.h
> > @@ -40,8 +40,8 @@
> >  #include "tst_capability.h"
> >  #include "tst_hugepage.h"
> >  #include "tst_assert.h"
> > -#include "tst_lockdown.h"
> > -#include "tst_fips.h"
> > +#include "tst_security.h"
> > +#include "tst_security.h"

> Huh, included twice?

Good catch, I'll remove it before merge.
Also note tst_security.h is in both tst_security.c and tst_test.h
(because it's also needed in tst_test.c).

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 1/3] lib: Merge security related sources

[Petr Vorel]

Hi all,

FYI merged this independent cleanup.

Kind regards,
Petr

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

[LTP] [PATCH v2 2/3] lib: Add tst_selinux_enforcing()

[Petr Vorel]

Reviewed-by: Li Wang <liwang@redhat.com>
Co-developed-by: Mete Durlu <meted@linux.ibm.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
changes v1->v2:
* Add selinux to tst_security.c instead of it's own C file.

 include/tst_security.h |  1 +
 lib/tst_security.c     | 18 ++++++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/include/tst_security.h b/include/tst_security.h
index 438b16dbb..5d91f8a98 100644
--- a/include/tst_security.h
+++ b/include/tst_security.h
@@ -13,5 +13,6 @@ int tst_fips_enabled(void);
 
 int tst_lockdown_enabled(void);
 int tst_secureboot_enabled(void);
+int tst_selinux_enforcing(void);
 
 #endif /* TST_SECURITY_H__ */
diff --git a/lib/tst_security.c b/lib/tst_security.c
index 0fc704dfa..7d929fafe 100644
--- a/lib/tst_security.c
+++ b/lib/tst_security.c
@@ -7,6 +7,7 @@
 
 #define PATH_FIPS	"/proc/sys/crypto/fips_enabled"
 #define PATH_LOCKDOWN	"/sys/kernel/security/lockdown"
+#define SELINUX_STATUS_PATH "/sys/fs/selinux/enforce"
 
 #if defined(__powerpc64__) || defined(__ppc64__)
 # define SECUREBOOT_VAR "/proc/device-tree/ibm,secure-boot"
@@ -16,6 +17,7 @@
 # define VAR_DATA_SIZE 5
 #endif
 
+#include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/mount.h>
@@ -30,11 +32,11 @@ int tst_fips_enabled(void)
 {
 	int fips = 0;
 
-	if (access(PATH_FIPS, R_OK) == 0) {
+	if (access(PATH_FIPS, R_OK) == 0)
 		SAFE_FILE_SCANF(PATH_FIPS, "%d", &fips);
-	}
 
 	tst_res(TINFO, "FIPS: %s", fips ? "on" : "off");
+
 	return fips;
 }
 
@@ -99,3 +101,15 @@ int tst_secureboot_enabled(void)
 	tst_res(TINFO, "SecureBoot: %s", data[VAR_DATA_SIZE - 1] ? "on" : "off");
 	return data[VAR_DATA_SIZE - 1];
 }
+
+int tst_selinux_enforcing(void)
+{
+	int res = 0;
+
+	if (access(SELINUX_STATUS_PATH, F_OK) == 0)
+		SAFE_FILE_SCANF(SELINUX_STATUS_PATH, "%d", &res);
+
+	tst_res(TINFO, "SELinux enforcing: %s", res ? "on" : "off");
+
+	return res;
+}
-- 
2.43.0


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

Re: [LTP] [PATCH v2 2/3] lib: Add tst_selinux_enforcing()

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH v2 3/3] fanotify14: fix anonymous pipe testcases

[Petr Vorel]

From: Mete Durlu <meted@linux.ibm.com>

When SElinux is in enforcing state and SEpolicies disallow anonymous
pipe usage with fanotify_mark(), related fanotify14 testcases fail with
EACCES instead of EINVAL. Accept both errnos when SElinux is in
enforcing state to correctly evaluate test results.

Replace TST_EXP_FD_OR_FAIL with TST_EXP_FAIL when testing
fanotify_mark() as it returns -1 on failure and 0 on success not a file
descriptor.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Co-developed-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Mete Durlu <meted@linux.ibm.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
changes v1->v2:
* Do not include library header in fanotify14 (not needed)

 .../kernel/syscalls/fanotify/fanotify14.c      | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
index d02d81495..82725f317 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify14.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
@@ -47,6 +47,7 @@ static int pipes[2] = {-1, -1};
 static int fanotify_fd;
 static int ignore_mark_unsupported;
 static int filesystem_mark_unsupported;
+static int se_enforcing;
 static unsigned int supported_init_flags;
 
 struct test_case_flags_t {
@@ -283,9 +284,18 @@ static void do_test(unsigned int number)
 
 	tst_res(TINFO, "Testing %s with %s",
 		tc->mark.desc, tc->mask.desc);
-	TST_EXP_FD_OR_FAIL(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
-					 tc->mask.flags, dirfd, path),
-					 tc->expected_errno);
+
+	if (tc->pfd && se_enforcing) {
+		const int exp_errs[] = {tc->expected_errno, EACCES};
+
+		TST_EXP_FAIL_ARR(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
+				 tc->mask.flags, dirfd, path),
+				 exp_errs);
+	} else {
+		TST_EXP_FAIL(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
+						 tc->mask.flags, dirfd, path),
+						 tc->expected_errno);
+	}
 
 	/*
 	 * ENOTDIR are errors for events/flags not allowed on a non-dir inode.
@@ -334,6 +344,8 @@ static void do_setup(void)
 	SAFE_FILE_PRINTF(FILE1, "0");
 	/* Create anonymous pipes to place marks on */
 	SAFE_PIPE2(pipes, O_CLOEXEC);
+
+	se_enforcing = tst_selinux_enforcing();
 }
 
 static void do_cleanup(void)
-- 
2.43.0


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

Re: [LTP] [PATCH v2 3/3] fanotify14: fix anonymous pipe testcases

[Cyril Hrubis]

Hi!
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
> Co-developed-by: Petr Vorel <pvorel@suse.cz>
> Signed-off-by: Mete Durlu <meted@linux.ibm.com>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> changes v1->v2:
> * Do not include library header in fanotify14 (not needed)
> 
>  .../kernel/syscalls/fanotify/fanotify14.c      | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
> index d02d81495..82725f317 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify14.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
> @@ -47,6 +47,7 @@ static int pipes[2] = {-1, -1};
>  static int fanotify_fd;
>  static int ignore_mark_unsupported;
>  static int filesystem_mark_unsupported;
> +static int se_enforcing;
>  static unsigned int supported_init_flags;
>  
>  struct test_case_flags_t {
> @@ -283,9 +284,18 @@ static void do_test(unsigned int number)
>  
>  	tst_res(TINFO, "Testing %s with %s",
>  		tc->mark.desc, tc->mask.desc);
> -	TST_EXP_FD_OR_FAIL(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
> -					 tc->mask.flags, dirfd, path),
> -					 tc->expected_errno);
> +
> +	if (tc->pfd && se_enforcing) {
> +		const int exp_errs[] = {tc->expected_errno, EACCES};
> +
> +		TST_EXP_FAIL_ARR(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
> +				 tc->mask.flags, dirfd, path),
> +				 exp_errs);
> +	} else {
> +		TST_EXP_FAIL(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
> +						 tc->mask.flags, dirfd, path),
> +						 tc->expected_errno);
> +	}

This looks like the test library is not flexible enough to make this
simpler. Maybe having the ARRAY_SIZE() in the TST_EXP_FAIL_ARR wasn't a
good idea after all. Or maybe we just need TST_EXP_FAIL_ARR2() that
would take the array size explicitly, then we could do:


	const int exp_errs[] = {tc->expected_errno, EACESS}

	TST_EXP_FAIL_ARR2(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
                           tc->mask.flags, dirfd, path), exp_errs,
			   se_enforcing ? 1 : 2);

>  	/*
>  	 * ENOTDIR are errors for events/flags not allowed on a non-dir inode.
> @@ -334,6 +344,8 @@ static void do_setup(void)
>  	SAFE_FILE_PRINTF(FILE1, "0");
>  	/* Create anonymous pipes to place marks on */
>  	SAFE_PIPE2(pipes, O_CLOEXEC);
> +
> +	se_enforcing = tst_selinux_enforcing();
>  }
>  
>  static void do_cleanup(void)
> -- 
> 2.43.0
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 3/3] fanotify14: fix anonymous pipe testcases

[Petr Vorel]

> Hi!
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > Reviewed-by: Jan Kara <jack@suse.cz>
> > Co-developed-by: Petr Vorel <pvorel@suse.cz>
> > Signed-off-by: Mete Durlu <meted@linux.ibm.com>
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > changes v1->v2:
> > * Do not include library header in fanotify14 (not needed)

> >  .../kernel/syscalls/fanotify/fanotify14.c      | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)

> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
> > index d02d81495..82725f317 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify14.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
> > @@ -47,6 +47,7 @@ static int pipes[2] = {-1, -1};
> >  static int fanotify_fd;
> >  static int ignore_mark_unsupported;
> >  static int filesystem_mark_unsupported;
> > +static int se_enforcing;
> >  static unsigned int supported_init_flags;

> >  struct test_case_flags_t {
> > @@ -283,9 +284,18 @@ static void do_test(unsigned int number)

> >  	tst_res(TINFO, "Testing %s with %s",
> >  		tc->mark.desc, tc->mask.desc);
> > -	TST_EXP_FD_OR_FAIL(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
> > -					 tc->mask.flags, dirfd, path),
> > -					 tc->expected_errno);
> > +
> > +	if (tc->pfd && se_enforcing) {
> > +		const int exp_errs[] = {tc->expected_errno, EACCES};
> > +
> > +		TST_EXP_FAIL_ARR(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
> > +				 tc->mask.flags, dirfd, path),
> > +				 exp_errs);
> > +	} else {
> > +		TST_EXP_FAIL(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
> > +						 tc->mask.flags, dirfd, path),
> > +						 tc->expected_errno);
> > +	}

> This looks like the test library is not flexible enough to make this
> simpler. Maybe having the ARRAY_SIZE() in the TST_EXP_FAIL_ARR wasn't a
> good idea after all. Or maybe we just need TST_EXP_FAIL_ARR2() that
> would take the array size explicitly, then we could do:


> 	const int exp_errs[] = {tc->expected_errno, EACESS}

> 	TST_EXP_FAIL_ARR2(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
>                            tc->mask.flags, dirfd, path), exp_errs,
> 			   se_enforcing ? 1 : 2);

This LGTM, although it makes tst_test_macros.h even harder to read.
I'll send another version.

Kind regards,
Petr

> >  	/*
> >  	 * ENOTDIR are errors for events/flags not allowed on a non-dir inode.
> > @@ -334,6 +344,8 @@ static void do_setup(void)
> >  	SAFE_FILE_PRINTF(FILE1, "0");
> >  	/* Create anonymous pipes to place marks on */
> >  	SAFE_PIPE2(pipes, O_CLOEXEC);
> > +
> > +	se_enforcing = tst_selinux_enforcing();
> >  }

> >  static void do_cleanup(void)
> > -- 
> > 2.43.0

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

Re: [LTP] [PATCH v2 3/3] fanotify14: fix anonymous pipe testcases

[Petr Vorel]

H
> Hi!
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > Reviewed-by: Jan Kara <jack@suse.cz>
> > Co-developed-by: Petr Vorel <pvorel@suse.cz>
> > Signed-off-by: Mete Durlu <meted@linux.ibm.com>
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > changes v1->v2:
> > * Do not include library header in fanotify14 (not needed)

> >  .../kernel/syscalls/fanotify/fanotify14.c      | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)

> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
> > index d02d81495..82725f317 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify14.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
> > @@ -47,6 +47,7 @@ static int pipes[2] = {-1, -1};
> >  static int fanotify_fd;
> >  static int ignore_mark_unsupported;
> >  static int filesystem_mark_unsupported;
> > +static int se_enforcing;
> >  static unsigned int supported_init_flags;

> >  struct test_case_flags_t {
> > @@ -283,9 +284,18 @@ static void do_test(unsigned int number)

> >  	tst_res(TINFO, "Testing %s with %s",
> >  		tc->mark.desc, tc->mask.desc);
> > -	TST_EXP_FD_OR_FAIL(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
> > -					 tc->mask.flags, dirfd, path),
> > -					 tc->expected_errno);
> > +
> > +	if (tc->pfd && se_enforcing) {
> > +		const int exp_errs[] = {tc->expected_errno, EACCES};
> > +
> > +		TST_EXP_FAIL_ARR(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
> > +				 tc->mask.flags, dirfd, path),
> > +				 exp_errs);
> > +	} else {
> > +		TST_EXP_FAIL(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
> > +						 tc->mask.flags, dirfd, path),
> > +						 tc->expected_errno);
> > +	}

> This looks like the test library is not flexible enough to make this
> simpler. Maybe having the ARRAY_SIZE() in the TST_EXP_FAIL_ARR wasn't a
> good idea after all. Or maybe we just need TST_EXP_FAIL_ARR2() that
> would take the array size explicitly, then we could do:

[ Removing Jan and Amir from the LTP specific discussion ]

@Cyril, @Li: How about add 2 macros with '_SIZE' in the name?
(TST_EXP_FAIL_ARR_SIZE and TST_EXP_FAIL2_ARR_SIZE, see diff at the end).

Other option would be to add '2' (TST_EXP_FAIL_ARR2 and TST_EXP_FAIL2_ARR2),
not sure what is more readable.

Kind regards,
Petr

> 	const int exp_errs[] = {tc->expected_errno, EACESS}

> 	TST_EXP_FAIL_ARR2(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags,
>                            tc->mask.flags, dirfd, path), exp_errs,
> 			   se_enforcing ? 1 : 2);

+++ include/tst_test_macros.h
@@ -246,6 +246,10 @@ const char *tst_errno_names(char *buf, const int *exp_errs, int exp_errs_cnt);
 		TST_EXP_FAIL_ARR_(SCALL, #SCALL, EXP_ERRS,                     \
 				  ARRAY_SIZE(EXP_ERRS), ##__VA_ARGS__);
 
+#define TST_EXP_FAIL_ARR_SIZE(SCALL, EXP_ERRS, EXP_ERRS_CNT, ...)                   \
+		TST_EXP_FAIL_ARR_(SCALL, #SCALL, EXP_ERRS,                     \
+				  EXP_ERRS_CNT, ##__VA_ARGS__);
+
 #define TST_EXP_FAIL2_ARR_(SCALL, SSCALL, EXP_ERRS, EXP_ERRS_CNT, ...)         \
 	do {                                                                   \
 		TST_EXP_FAIL_SILENT_(TST_RET >= 0, SCALL, SSCALL,              \
@@ -258,6 +262,10 @@ const char *tst_errno_names(char *buf, const int *exp_errs, int exp_errs_cnt);
 		TST_EXP_FAIL2_ARR_(SCALL, #SCALL, EXP_ERRS,                    \
 		                  ARRAY_SIZE(EXP_ERRS), ##__VA_ARGS__);
 
+#define TST_EXP_FAIL2_ARR_SIZE(SCALL, EXP_ERRS, EXP_ERRS_CNT...)                \
+		TST_EXP_FAIL2_ARR_(SCALL, #SCALL, EXP_ERRS,                    \
+		                  EXP_ERRS_CNT, ##__VA_ARGS__);
+
 #define TST_EXP_FAIL2(SCALL, EXP_ERR, ...)                                     \
 	do {                                                                   \
 		int tst_exp_err__ = EXP_ERR;                                   \

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

Re: [LTP] [PATCH v2 3/3] fanotify14: fix anonymous pipe testcases

[Cyril Hrubis]

Hi!
> @Cyril, @Li: How about add 2 macros with '_SIZE' in the name?
> (TST_EXP_FAIL_ARR_SIZE and TST_EXP_FAIL2_ARR_SIZE, see diff at the end).

Maybe just _SZ instead of full _SIZE?

Also the TST_EXP_FAIL_ARR() is rarely used so we may as well to change
it to include the size argument explicitly, the worst case is that user
would have to pass down ARRAY_SIZE(errnos), which is just one more
argument. The macro is used by a single test at the moment so it's easy
now. Doing that would at least slown down the combinatoric explosion of
the test macros.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 3/3] fanotify14: fix anonymous pipe testcases

[Petr Vorel]

Hi Cyril,

> Hi!
> > @Cyril, @Li: How about add 2 macros with '_SIZE' in the name?
> > (TST_EXP_FAIL_ARR_SIZE and TST_EXP_FAIL2_ARR_SIZE, see diff at the end).

> Maybe just _SZ instead of full _SIZE?

> Also the TST_EXP_FAIL_ARR() is rarely used so we may as well to change
> it to include the size argument explicitly, the worst case is that user
> would have to pass down ARRAY_SIZE(errnos), which is just one more
> argument. The macro is used by a single test at the moment so it's easy
> now. Doing that would at least slown down the combinatoric explosion of
> the test macros.

Good point, I just replace ARRAY_SIZE(errnos) with size parameter in the next
version. Thanks a lot for your input.

Kind regards,
Petr

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