[LTP] [PATCH v1] fanotify14: fix anonymous pipe testcases

[LTP] [PATCH v1] fanotify14: fix anonymous pipe testcases

[Mete Durlu]

When SElinux is configured (comes out of the box on most distros) and
is configured to enforcing (the default configuration), tests related
to anonymous pipes return EACCES instead of the expected errno EINVAL.
Fix the failures caused by the above condition by checking the SElinux
configuration and adjusting the errno accordingly.

Signed-off-by: Mete Durlu <meted@linux.ibm.com>
---
 testcases/kernel/syscalls/fanotify/fanotify14.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
index d02d81495..61ed8c660 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify14.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
@@ -27,12 +27,14 @@
 #define _GNU_SOURCE
 #include "tst_test.h"
 #include <errno.h>
+#include <stdlib.h>
 
 #ifdef HAVE_SYS_FANOTIFY_H
 #include "fanotify.h"
 
 #define MNTPOINT "mntpoint"
 #define FILE1 MNTPOINT"/file1"
+#define SELINUX_STATUS_PATH "/sys/fs/selinux/enforce"
 
 /*
  * List of inode events that are only available when notification group is
@@ -240,6 +242,19 @@ static struct test_case_t {
 	},
 };
 
+static int is_selinux_enforcing(void)
+{
+	char res;
+	int fd;
+
+	fd = open(SELINUX_STATUS_PATH, O_RDONLY);
+	if (fd <= 0)
+		return 0;
+	SAFE_READ(1, fd, &res, 1);
+	SAFE_CLOSE(fd);
+	return atoi(&res);
+}
+
 static void do_test(unsigned int number)
 {
 	struct test_case_t *tc = &test_cases[number];
@@ -279,6 +294,8 @@ static void do_test(unsigned int number)
 	if (tc->pfd) {
 		dirfd = tc->pfd[0];
 		path = NULL;
+		if (is_selinux_enforcing())
+			tc->expected_errno = EACCES;
 	}
 
 	tst_res(TINFO, "Testing %s with %s",
-- 
2.44.0


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

Re: [LTP] [PATCH v1] fanotify14: fix anonymous pipe testcases

[Amir Goldstein]

On Fri, Mar 8, 2024 at 2:43 PM Mete Durlu <meted@linux.ibm.com> wrote:
>
> When SElinux is configured (comes out of the box on most distros) and
> is configured to enforcing (the default configuration), tests related
> to anonymous pipes return EACCES instead of the expected errno EINVAL.
> Fix the failures caused by the above condition by checking the SElinux
> configuration and adjusting the errno accordingly.

Hi Mete,

Isn't the outcome of the test dependent on the SEpolicy rules?
Not only if it is enforced?

Sorry I have very little experience with SELinux.

Thanks,
Amir.

>
> Signed-off-by: Mete Durlu <meted@linux.ibm.com>
> ---
>  testcases/kernel/syscalls/fanotify/fanotify14.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
> index d02d81495..61ed8c660 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify14.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
> @@ -27,12 +27,14 @@
>  #define _GNU_SOURCE
>  #include "tst_test.h"
>  #include <errno.h>
> +#include <stdlib.h>
>
>  #ifdef HAVE_SYS_FANOTIFY_H
>  #include "fanotify.h"
>
>  #define MNTPOINT "mntpoint"
>  #define FILE1 MNTPOINT"/file1"
> +#define SELINUX_STATUS_PATH "/sys/fs/selinux/enforce"
>
>  /*
>   * List of inode events that are only available when notification group is
> @@ -240,6 +242,19 @@ static struct test_case_t {
>         },
>  };
>
> +static int is_selinux_enforcing(void)
> +{
> +       char res;
> +       int fd;
> +
> +       fd = open(SELINUX_STATUS_PATH, O_RDONLY);
> +       if (fd <= 0)
> +               return 0;
> +       SAFE_READ(1, fd, &res, 1);
> +       SAFE_CLOSE(fd);
> +       return atoi(&res);
> +}
> +
>  static void do_test(unsigned int number)
>  {
>         struct test_case_t *tc = &test_cases[number];
> @@ -279,6 +294,8 @@ static void do_test(unsigned int number)
>         if (tc->pfd) {
>                 dirfd = tc->pfd[0];
>                 path = NULL;
> +               if (is_selinux_enforcing())
> +                       tc->expected_errno = EACCES;
>         }
>
>         tst_res(TINFO, "Testing %s with %s",
> --
> 2.44.0
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp

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

Re: [LTP] [PATCH v1] fanotify14: fix anonymous pipe testcases

[Mete Durlu]

On 3/8/24 14:39, Amir Goldstein wrote:
> On Fri, Mar 8, 2024 at 2:43 PM Mete Durlu <meted@linux.ibm.com> wrote:
>>
>> When SElinux is configured (comes out of the box on most distros) and
>> is configured to enforcing (the default configuration), tests related
>> to anonymous pipes return EACCES instead of the expected errno EINVAL.
>> Fix the failures caused by the above condition by checking the SElinux
>> configuration and adjusting the errno accordingly.
> 
> Hi Mete,
> 
> Isn't the outcome of the test dependent on the SEpolicy rules?
> Not only if it is enforced?
> 
> Sorry I have very little experience with SELinux.
> 

Hi Amir,

I don't have SElinux experience either, on my proposed patch I only
considered the default behavior but you are right different SElinux
configurations may lead to different outcomes. I skimmed over SElinux
wiki a little and now I think trying to verify the SElinux policy would
be too cumbersome. Instead I propose two different solutions.

1. We can skip the anonymous pipe test cases when SElinux is in
    enforcing state.

or

2. We can accept both EACESS and EINVAL as valid errnos when SElinux is
    in enforcing state.

Personally option 2 sounds better to me since we would get more coverage
that way. If either way sounds good I can send a v2 right away. How does
that sound?

Thank you.

> 
>>
>> Signed-off-by: Mete Durlu <meted@linux.ibm.com>
>> ---
>>   testcases/kernel/syscalls/fanotify/fanotify14.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
>> index d02d81495..61ed8c660 100644
>> --- a/testcases/kernel/syscalls/fanotify/fanotify14.c
>> +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
>> @@ -27,12 +27,14 @@
>>   #define _GNU_SOURCE
>>   #include "tst_test.h"
>>   #include <errno.h>
>> +#include <stdlib.h>
>>
>>   #ifdef HAVE_SYS_FANOTIFY_H
>>   #include "fanotify.h"
>>
>>   #define MNTPOINT "mntpoint"
>>   #define FILE1 MNTPOINT"/file1"
>> +#define SELINUX_STATUS_PATH "/sys/fs/selinux/enforce"
>>
>>   /*
>>    * List of inode events that are only available when notification group is
>> @@ -240,6 +242,19 @@ static struct test_case_t {
>>          },
>>   };
>>
>> +static int is_selinux_enforcing(void)
>> +{
>> +       char res;
>> +       int fd;
>> +
>> +       fd = open(SELINUX_STATUS_PATH, O_RDONLY);
>> +       if (fd <= 0)
>> +               return 0;
>> +       SAFE_READ(1, fd, &res, 1);
>> +       SAFE_CLOSE(fd);
>> +       return atoi(&res);
>> +}
>> +
>>   static void do_test(unsigned int number)
>>   {
>>          struct test_case_t *tc = &test_cases[number];
>> @@ -279,6 +294,8 @@ static void do_test(unsigned int number)
>>          if (tc->pfd) {
>>                  dirfd = tc->pfd[0];
>>                  path = NULL;
>> +               if (is_selinux_enforcing())
>> +                       tc->expected_errno = EACCES;
>>          }
>>
>>          tst_res(TINFO, "Testing %s with %s",
>> --
>> 2.44.0
>>
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp


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

Re: [LTP] [PATCH v1] fanotify14: fix anonymous pipe testcases

[Amir Goldstein]

On Mon, Mar 11, 2024 at 4:53 PM Mete Durlu <meted@linux.ibm.com> wrote:
>
> On 3/8/24 14:39, Amir Goldstein wrote:
> > On Fri, Mar 8, 2024 at 2:43 PM Mete Durlu <meted@linux.ibm.com> wrote:
> >>
> >> When SElinux is configured (comes out of the box on most distros) and
> >> is configured to enforcing (the default configuration), tests related
> >> to anonymous pipes return EACCES instead of the expected errno EINVAL.
> >> Fix the failures caused by the above condition by checking the SElinux
> >> configuration and adjusting the errno accordingly.
> >
> > Hi Mete,
> >
> > Isn't the outcome of the test dependent on the SEpolicy rules?
> > Not only if it is enforced?
> >
> > Sorry I have very little experience with SELinux.
> >
>
> Hi Amir,
>
> I don't have SElinux experience either, on my proposed patch I only
> considered the default behavior but you are right different SElinux
> configurations may lead to different outcomes. I skimmed over SElinux
> wiki a little and now I think trying to verify the SElinux policy would
> be too cumbersome. Instead I propose two different solutions.
>
> 1. We can skip the anonymous pipe test cases when SElinux is in
>     enforcing state.
>
> or
>
> 2. We can accept both EACESS and EINVAL as valid errnos when SElinux is
>     in enforcing state.
>
> Personally option 2 sounds better to me since we would get more coverage
> that way. If either way sounds good I can send a v2 right away. How does
> that sound?

option 2 sounds good to me.

Thanks,
Amir.

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

Re: [LTP] [PATCH v1] fanotify14: fix anonymous pipe testcases

[Petr Vorel]

Hi Mete, Amir, Li,

[ Cc Li who knows more about SELinux :) ]

> On Mon, Mar 11, 2024 at 4:53 PM Mete Durlu <meted@linux.ibm.com> wrote:

> > On 3/8/24 14:39, Amir Goldstein wrote:
> > > On Fri, Mar 8, 2024 at 2:43 PM Mete Durlu <meted@linux.ibm.com> wrote:

> > >> When SElinux is configured (comes out of the box on most distros) and
> > >> is configured to enforcing (the default configuration), tests related
> > >> to anonymous pipes return EACCES instead of the expected errno EINVAL.
> > >> Fix the failures caused by the above condition by checking the SElinux
> > >> configuration and adjusting the errno accordingly.

> > > Hi Mete,

> > > Isn't the outcome of the test dependent on the SEpolicy rules?
> > > Not only if it is enforced?

> > > Sorry I have very little experience with SELinux.


> > Hi Amir,

> > I don't have SElinux experience either, on my proposed patch I only
> > considered the default behavior but you are right different SElinux
> > configurations may lead to different outcomes. I skimmed over SElinux
> > wiki a little and now I think trying to verify the SElinux policy would
> > be too cumbersome. Instead I propose two different solutions.

> > 1. We can skip the anonymous pipe test cases when SElinux is in
> >     enforcing state.

> > or

> > 2. We can accept both EACESS and EINVAL as valid errnos when SElinux is
> >     in enforcing state.

> > Personally option 2 sounds better to me since we would get more coverage
> > that way. If either way sounds good I can send a v2 right away. How does
> > that sound?

> option 2 sounds good to me.

Yes, EACESS for enforced SELinux is what we want.

Mete, thank you for handling this. I can confirm it's a problem on SELinux
enforced. And I suppose the current code works, but we need some modifications
(please let me know if you don't have time for v2):

* Put tst_selinux_enforcing() function into LTP library: you need to create
lib/tst_selinux.c and include/tst_selinux.c. For inspiration have look at
lib/tst_lockdown.c vv include/tst_lockdown.h. The reason is obvious: sooner or
later we will reuse this functionality.

* use access(), print also TINFO (similarly to lib/tst_lockdown.c)

* /sys/fs/selinux vs. /selinux, selinux=1 vs. security=selinux (/proc/cmdline)
@Li: TL;DR: reading just /sys/fs/selinux/enforce LGTM, but please check

I suppose we can rely on selinuxfs being mounted on /sys/fs/selinux:

$ mount | grep -i selinux
selinuxfs on /sys/fs/selinux type selinuxfs (rw,nosuid,noexec,relatime)

Long time ago the directory was just /selinux (RHEL 5 or 6?), that's why it's
still checked in shell API testcases/lib/tst_security.sh. These systems are
quite old to run newest LTP, right? From d41415eb5edae [1] I see it was kernel
3.0 => way too old to consider.

I guess we cannot rely on selinux=1 or security=selinux to detect enforce mode.
There is SECURITY_SELINUX_BOOTPARAM, when disabled thus there is no selinux=1
variable in /proc/cmdline, thus we cannot rely on it (instead of using
/sys/fs/selinux).

Also, kernel < v5.1 had SECURITY_SELINUX_BOOTPARAM_VALUE (removed in
be6ec88f41ba94 in v5.1 [2]), another reason not to rely on selinux in
/proc/cmdline.

NOTE: as I noted previously we have support for SELinux (and AppArmor) detection
in shell API testcases/lib/tst_security.sh, we might later create simple C
binary in testcases/lib/ which will call function you create in C API (similarly
to testcases/lib/tst_lockdown_enabled.c), but we can ignore it now.

Kind regards,
Petr

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d41415eb5edae
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=be6ec88f41ba94

> Thanks,
> Amir.

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

Re: [LTP] [PATCH v1] fanotify14: fix anonymous pipe testcases

[Mete Durlu]

On 3/12/24 14:10, Petr Vorel wrote:
> Hi Mete, Amir, Li,
> 
> [ Cc Li who knows more about SELinux :) ]
> 
>> On Mon, Mar 11, 2024 at 4:53 PM Mete Durlu <meted@linux.ibm.com> wrote:
> 
>>> On 3/8/24 14:39, Amir Goldstein wrote:
>>>> On Fri, Mar 8, 2024 at 2:43 PM Mete Durlu <meted@linux.ibm.com> wrote:
> 
>>>>> When SElinux is configured (comes out of the box on most distros) and
>>>>> is configured to enforcing (the default configuration), tests related
>>>>> to anonymous pipes return EACCES instead of the expected errno EINVAL.
>>>>> Fix the failures caused by the above condition by checking the SElinux
>>>>> configuration and adjusting the errno accordingly.
> 
>>>> Hi Mete,
> 
>>>> Isn't the outcome of the test dependent on the SEpolicy rules?
>>>> Not only if it is enforced?
> 
>>>> Sorry I have very little experience with SELinux.
> 
> 
>>> Hi Amir,
> 
>>> I don't have SElinux experience either, on my proposed patch I only
>>> considered the default behavior but you are right different SElinux
>>> configurations may lead to different outcomes. I skimmed over SElinux
>>> wiki a little and now I think trying to verify the SElinux policy would
>>> be too cumbersome. Instead I propose two different solutions.
> 
>>> 1. We can skip the anonymous pipe test cases when SElinux is in
>>>      enforcing state.
> 
>>> or
> 
>>> 2. We can accept both EACESS and EINVAL as valid errnos when SElinux is
>>>      in enforcing state.
> 
>>> Personally option 2 sounds better to me since we would get more coverage
>>> that way. If either way sounds good I can send a v2 right away. How does
>>> that sound?
> 
>> option 2 sounds good to me.
> 
> Yes, EACESS for enforced SELinux is what we want.
> 
> Mete, thank you for handling this. I can confirm it's a problem on SELinux
> enforced. And I suppose the current code works, but we need some modifications
> (please let me know if you don't have time for v2):

Hi,

I was hoping to solve this with a quick/small fix but I guess there is
more to do.

> * Put tst_selinux_enforcing() function into LTP library: you need to create
> lib/tst_selinux.c and include/tst_selinux.c. For inspiration have look at
> lib/tst_lockdown.c vv include/tst_lockdown.h. The reason is obvious: sooner or
> later we will reuse this functionality.

If there is no rush for this I can add this in as a separate patch
series, but I am not sure when I can start. If this is urgent then
probably someone else should do it.

> * use access(), print also TINFO (similarly to lib/tst_lockdown.c)
> 
> * /sys/fs/selinux vs. /selinux, selinux=1 vs. security=selinux (/proc/cmdline)
> @Li: TL;DR: reading just /sys/fs/selinux/enforce LGTM, but please check
> 
> I suppose we can rely on selinuxfs being mounted on /sys/fs/selinux:
> 
> $ mount | grep -i selinux
> selinuxfs on /sys/fs/selinux type selinuxfs (rw,nosuid,noexec,relatime)
> 
> Long time ago the directory was just /selinux (RHEL 5 or 6?), that's why it's
> still checked in shell API testcases/lib/tst_security.sh. These systems are
> quite old to run newest LTP, right? From d41415eb5edae [1] I see it was kernel
> 3.0 => way too old to consider.
> 
> I guess we cannot rely on selinux=1 or security=selinux to detect enforce mode.
> There is SECURITY_SELINUX_BOOTPARAM, when disabled thus there is no selinux=1
> variable in /proc/cmdline, thus we cannot rely on it (instead of using
> /sys/fs/selinux).
> 
> Also, kernel < v5.1 had SECURITY_SELINUX_BOOTPARAM_VALUE (removed in
> be6ec88f41ba94 in v5.1 [2]), another reason not to rely on selinux in
> /proc/cmdline.
> NOTE: as I noted previously we have support for SELinux (and AppArmor) detection
> in shell API testcases/lib/tst_security.sh, we might later create simple C
> binary in testcases/lib/ which will call function you create in C API (similarly
> to testcases/lib/tst_lockdown_enabled.c), but we can ignore it now.
> 
> Kind regards,
> Petr
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d41415eb5edae
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=be6ec88f41ba94
> 
>> Thanks,
>> Amir.


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