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

[Mete Durlu <meted@linux.ibm.com>]

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