Re: [PATCH testsuite] tests/task_setscheduler: add cgroup v2 case for moving proc to root cgroup

Re: [PATCH testsuite] tests/task_setscheduler: add cgroup v2 case for moving proc to root cgroup

[Gong Ruiqi]

Ping.

On 2024/07/02 17:54, GONG, Ruiqi wrote:
> Currently for systems that only enable cgroup v2, the test script would
> fail to move the target process into the root cgroup since the cgroup v1
> path is used, which therefore makes the testcase fail. Add cgroup v2
> handling here so that no matter which cgroup version the CPU controller
> is bound to, the target process can always be moved to the root CPU
> cgroup.
> 
> Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com>
> ---
>  tests/task_setscheduler/test | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/task_setscheduler/test b/tests/task_setscheduler/test
> index c2fe8c6..fa1efb1 100755
> --- a/tests/task_setscheduler/test
> +++ b/tests/task_setscheduler/test
> @@ -20,12 +20,17 @@ vec( $rin, fileno($f), 1 ) = 1;
>  select( $rin, undef, undef, 5 );
>  close($f);
>  
> -$cgroup_cpu = "/sys/fs/cgroup/cpu/tasks";
> -if ( -w $cgroup_cpu ) {
> -
> -    # We can only set the scheduler policy fo SCHED_{RR,FIFO} in the root
> -    # cgroup so move our target process to the root cgroup.
> -    open( my $fd, ">>", $cgroup_cpu );
> +# We can only set the scheduler policy fo SCHED_{RR,FIFO} in the root
> +# cgroup so move our target process to the root cgroup.
> +$cgroup_v1_cpu = "/sys/fs/cgroup/cpu/tasks";
> +if ( -w $cgroup_v1_cpu ) {
> +    open( my $fd, ">>", $cgroup_v1_cpu );
> +    print $fd $pid;
> +    close $fd;
> +}
> +$cgroup_v2 = "/sys/fs/cgroup/cgroup.procs";
> +if ( -w $cgroup_v2 ) {
> +    open( my $fd, ">>", $cgroup_v2 );
>      print $fd $pid;
>      close $fd;
>  }

Re: [PATCH testsuite] tests/task_setscheduler: add cgroup v2 case for moving proc to root cgroup

[Paul Moore]

On Tue, Jul 16, 2024 at 10:19 PM Gong Ruiqi <gongruiqi1@huawei.com> wrote:
>
> Ping.

Dropping the LSM mailing list to cut down on the noise as it isn't a
relevant mailing list.

Ondrej currently maintains the selinux-testsuite project so I'd prefer
to give him time to review/test/etc. the patch, but I see it has
already been a couple of weeks without response.  If Ondrej doesn't
get to this patch by the end of the Linux v6.11 merge window I'll take
a look then.

Where (what distribution, version, etc.) did you see this problem?

Thank you for sending this out :)

> On 2024/07/02 17:54, GONG, Ruiqi wrote:
> > Currently for systems that only enable cgroup v2, the test script would
> > fail to move the target process into the root cgroup since the cgroup v1
> > path is used, which therefore makes the testcase fail. Add cgroup v2
> > handling here so that no matter which cgroup version the CPU controller
> > is bound to, the target process can always be moved to the root CPU
> > cgroup.
> >
> > Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com>
> > ---
> >  tests/task_setscheduler/test | 17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/tests/task_setscheduler/test b/tests/task_setscheduler/test
> > index c2fe8c6..fa1efb1 100755
> > --- a/tests/task_setscheduler/test
> > +++ b/tests/task_setscheduler/test
> > @@ -20,12 +20,17 @@ vec( $rin, fileno($f), 1 ) = 1;
> >  select( $rin, undef, undef, 5 );
> >  close($f);
> >
> > -$cgroup_cpu = "/sys/fs/cgroup/cpu/tasks";
> > -if ( -w $cgroup_cpu ) {
> > -
> > -    # We can only set the scheduler policy fo SCHED_{RR,FIFO} in the root
> > -    # cgroup so move our target process to the root cgroup.
> > -    open( my $fd, ">>", $cgroup_cpu );
> > +# We can only set the scheduler policy fo SCHED_{RR,FIFO} in the root
> > +# cgroup so move our target process to the root cgroup.
> > +$cgroup_v1_cpu = "/sys/fs/cgroup/cpu/tasks";
> > +if ( -w $cgroup_v1_cpu ) {
> > +    open( my $fd, ">>", $cgroup_v1_cpu );
> > +    print $fd $pid;
> > +    close $fd;
> > +}
> > +$cgroup_v2 = "/sys/fs/cgroup/cgroup.procs";
> > +if ( -w $cgroup_v2 ) {
> > +    open( my $fd, ">>", $cgroup_v2 );
> >      print $fd $pid;
> >      close $fd;
> >  }
>


-- 
paul-moore.com

Re: [PATCH testsuite] tests/task_setscheduler: add cgroup v2 case for moving proc to root cgroup

[Gong Ruiqi]

On 2024/07/18 0:17, Paul Moore wrote:
> On Tue, Jul 16, 2024 at 10:19 PM Gong Ruiqi <gongruiqi1@huawei.com> wrote:
>>
>> Ping.
> 
> Dropping the LSM mailing list to cut down on the noise as it isn't a
> relevant mailing list.
> 
> Ondrej currently maintains the selinux-testsuite project so I'd prefer
> to give him time to review/test/etc. the patch, but I see it has
> already been a couple of weeks without response.  If Ondrej doesn't
> get to this patch by the end of the Linux v6.11 merge window I'll take
> a look then.

Thanks for your help!

> 
> Where (what distribution, version, etc.) did you see this problem?

The problem occurred when I ran the testsuite on Fedora 40 with v6.6
kernel, and it was the only failed testcase.

> 
> Thank you for sending this out :)
> 
>> On 2024/07/02 17:54, GONG, Ruiqi wrote:
>>> Currently for systems that only enable cgroup v2, the test script would
>>> fail to move the target process into the root cgroup since the cgroup v1
>>> path is used, which therefore makes the testcase fail. Add cgroup v2
>>> handling here so that no matter which cgroup version the CPU controller
>>> is bound to, the target process can always be moved to the root CPU
>>> cgroup.
>>>
>>> Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com>
>>> ---
>>>  tests/task_setscheduler/test | 17 +++++++++++------
>>>  1 file changed, 11 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tests/task_setscheduler/test b/tests/task_setscheduler/test
>>> index c2fe8c6..fa1efb1 100755
>>> --- a/tests/task_setscheduler/test
>>> +++ b/tests/task_setscheduler/test
>>> @@ -20,12 +20,17 @@ vec( $rin, fileno($f), 1 ) = 1;
>>>  select( $rin, undef, undef, 5 );
>>>  close($f);
>>>
>>> -$cgroup_cpu = "/sys/fs/cgroup/cpu/tasks";
>>> -if ( -w $cgroup_cpu ) {
>>> -
>>> -    # We can only set the scheduler policy fo SCHED_{RR,FIFO} in the root
>>> -    # cgroup so move our target process to the root cgroup.
>>> -    open( my $fd, ">>", $cgroup_cpu );
>>> +# We can only set the scheduler policy fo SCHED_{RR,FIFO} in the root
>>> +# cgroup so move our target process to the root cgroup.
>>> +$cgroup_v1_cpu = "/sys/fs/cgroup/cpu/tasks";
>>> +if ( -w $cgroup_v1_cpu ) {
>>> +    open( my $fd, ">>", $cgroup_v1_cpu );
>>> +    print $fd $pid;
>>> +    close $fd;
>>> +}
>>> +$cgroup_v2 = "/sys/fs/cgroup/cgroup.procs";
>>> +if ( -w $cgroup_v2 ) {
>>> +    open( my $fd, ">>", $cgroup_v2 );
>>>      print $fd $pid;
>>>      close $fd;
>>>  }
>>
> 
> 

Re: [PATCH testsuite] tests/task_setscheduler: add cgroup v2 case for moving proc to root cgroup

[Ondrej Mosnacek]

On Thu, Jul 18, 2024 at 2:34 PM Gong Ruiqi <gongruiqi1@huawei.com> wrote:
>
>
> On 2024/07/18 0:17, Paul Moore wrote:
> > On Tue, Jul 16, 2024 at 10:19 PM Gong Ruiqi <gongruiqi1@huawei.com> wrote:
> >>
> >> Ping.
> >
> > Dropping the LSM mailing list to cut down on the noise as it isn't a
> > relevant mailing list.
> >
> > Ondrej currently maintains the selinux-testsuite project so I'd prefer
> > to give him time to review/test/etc. the patch, but I see it has
> > already been a couple of weeks without response.  If Ondrej doesn't
> > get to this patch by the end of the Linux v6.11 merge window I'll take
> > a look then.
>
> Thanks for your help!
>
> >
> > Where (what distribution, version, etc.) did you see this problem?
>
> The problem occurred when I ran the testsuite on Fedora 40 with v6.6
> kernel, and it was the only failed testcase.

Sorry for the delay... For some reason the test passes for me even
with cgroup v2 only and without the patch (also when run from a
regular user account with sudo). Do you happen to know what
circumstances are needed for it to fail when the cgroup is not
switched?

-- 
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.

Re: [PATCH testsuite] tests/task_setscheduler: add cgroup v2 case for moving proc to root cgroup

[Gong Ruiqi]

On 2024/07/26 21:43, Ondrej Mosnacek wrote:
> On Thu, Jul 18, 2024 at 2:34 PM Gong Ruiqi <gongruiqi1@huawei.com> wrote:
>>
>>
>> On 2024/07/18 0:17, Paul Moore wrote:
>>> ...
>>>
>>> Where (what distribution, version, etc.) did you see this problem?
>>
>> The problem occurred when I ran the testsuite on Fedora 40 with v6.6
>> kernel, and it was the only failed testcase.
> 
> Sorry for the delay... For some reason the test passes for me even
> with cgroup v2 only and without the patch (also when run from a
> regular user account with sudo). Do you happen to know what
> circumstances are needed for it to fail when the cgroup is not
> switched?
> 

As the comment in the script says, a process need to be in the root
cgroup in order to switch its scheduler policy to SCHED_{RR,FIFO}. So
maybe in your case the shell process is already in the root cgroup?

In my case I need to ssh to a Fedora VM, and that makes my shell process
to be in a sub cgroup called /user.slice/.../XXX.scope (looks like some
systemd stuff). And since /sys/fs/cgroup/cpu/tasks doesn't exit in the
system with cgroup v2 only, the script skips moving the target process
to the root cgroup, and therefore the subsequent test fails.

Re: [PATCH testsuite] tests/task_setscheduler: add cgroup v2 case for moving proc to root cgroup

[Ondrej Mosnacek]

On Sat, Jul 27, 2024 at 4:55 AM Gong Ruiqi <gongruiqi1@huawei.com> wrote:
>
>
> On 2024/07/26 21:43, Ondrej Mosnacek wrote:
> > On Thu, Jul 18, 2024 at 2:34 PM Gong Ruiqi <gongruiqi1@huawei.com> wrote:
> >>
> >>
> >> On 2024/07/18 0:17, Paul Moore wrote:
> >>> ...
> >>>
> >>> Where (what distribution, version, etc.) did you see this problem?
> >>
> >> The problem occurred when I ran the testsuite on Fedora 40 with v6.6
> >> kernel, and it was the only failed testcase.
> >
> > Sorry for the delay... For some reason the test passes for me even
> > with cgroup v2 only and without the patch (also when run from a
> > regular user account with sudo). Do you happen to know what
> > circumstances are needed for it to fail when the cgroup is not
> > switched?
> >
>
> As the comment in the script says, a process need to be in the root
> cgroup in order to switch its scheduler policy to SCHED_{RR,FIFO}. So
> maybe in your case the shell process is already in the root cgroup?
>
> In my case I need to ssh to a Fedora VM, and that makes my shell process
> to be in a sub cgroup called /user.slice/.../XXX.scope (looks like some
> systemd stuff). And since /sys/fs/cgroup/cpu/tasks doesn't exit in the
> system with cgroup v2 only, the script skips moving the target process
> to the root cgroup, and therefore the subsequent test fails.

In my case I ssh as root and end up in
/user.slice/user-0.slice/session-1.scope cgroup,
/sys/fs/cgroup/cpu/tasks also doesn't exist, and yet the test passes.
The same also happens when I ssh as a regular user (with cgroup
/user.slice/user-1000.slice/session-3.scope) and run the testsuite
with sudo. So there must be something more to it... maybe some kernel
config or sysctl setting?

-- 
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.

Re: [PATCH testsuite] tests/task_setscheduler: add cgroup v2 case for moving proc to root cgroup

[Stephen Smalley]

On Mon, Jul 29, 2024 at 6:29 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> On Sat, Jul 27, 2024 at 4:55 AM Gong Ruiqi <gongruiqi1@huawei.com> wrote:
> >
> >
> > On 2024/07/26 21:43, Ondrej Mosnacek wrote:
> > > On Thu, Jul 18, 2024 at 2:34 PM Gong Ruiqi <gongruiqi1@huawei.com> wrote:
> > >>
> > >>
> > >> On 2024/07/18 0:17, Paul Moore wrote:
> > >>> ...
> > >>>
> > >>> Where (what distribution, version, etc.) did you see this problem?
> > >>
> > >> The problem occurred when I ran the testsuite on Fedora 40 with v6.6
> > >> kernel, and it was the only failed testcase.
> > >
> > > Sorry for the delay... For some reason the test passes for me even
> > > with cgroup v2 only and without the patch (also when run from a
> > > regular user account with sudo). Do you happen to know what
> > > circumstances are needed for it to fail when the cgroup is not
> > > switched?
> > >
> >
> > As the comment in the script says, a process need to be in the root
> > cgroup in order to switch its scheduler policy to SCHED_{RR,FIFO}. So
> > maybe in your case the shell process is already in the root cgroup?
> >
> > In my case I need to ssh to a Fedora VM, and that makes my shell process
> > to be in a sub cgroup called /user.slice/.../XXX.scope (looks like some
> > systemd stuff). And since /sys/fs/cgroup/cpu/tasks doesn't exit in the
> > system with cgroup v2 only, the script skips moving the target process
> > to the root cgroup, and therefore the subsequent test fails.
>
> In my case I ssh as root and end up in
> /user.slice/user-0.slice/session-1.scope cgroup,
> /sys/fs/cgroup/cpu/tasks also doesn't exist, and yet the test passes.
> The same also happens when I ssh as a regular user (with cgroup
> /user.slice/user-1000.slice/session-3.scope) and run the testsuite
> with sudo. So there must be something more to it... maybe some kernel
> config or sysctl setting?

As a further data point, I also have been unable to reproduce the
reported behavior.
That said, since tasks doesn't exist, isn't the passing test a false
negative (i.e. it isn't truly testing as intended)?

Re: [PATCH testsuite] tests/task_setscheduler: add cgroup v2 case for moving proc to root cgroup

[Ondrej Mosnacek]

On Mon, Jul 29, 2024 at 1:55 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Mon, Jul 29, 2024 at 6:29 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > On Sat, Jul 27, 2024 at 4:55 AM Gong Ruiqi <gongruiqi1@huawei.com> wrote:
> > >
> > >
> > > On 2024/07/26 21:43, Ondrej Mosnacek wrote:
> > > > On Thu, Jul 18, 2024 at 2:34 PM Gong Ruiqi <gongruiqi1@huawei.com> wrote:
> > > >>
> > > >>
> > > >> On 2024/07/18 0:17, Paul Moore wrote:
> > > >>> ...
> > > >>>
> > > >>> Where (what distribution, version, etc.) did you see this problem?
> > > >>
> > > >> The problem occurred when I ran the testsuite on Fedora 40 with v6.6
> > > >> kernel, and it was the only failed testcase.
> > > >
> > > > Sorry for the delay... For some reason the test passes for me even
> > > > with cgroup v2 only and without the patch (also when run from a
> > > > regular user account with sudo). Do you happen to know what
> > > > circumstances are needed for it to fail when the cgroup is not
> > > > switched?
> > > >
> > >
> > > As the comment in the script says, a process need to be in the root
> > > cgroup in order to switch its scheduler policy to SCHED_{RR,FIFO}. So
> > > maybe in your case the shell process is already in the root cgroup?
> > >
> > > In my case I need to ssh to a Fedora VM, and that makes my shell process
> > > to be in a sub cgroup called /user.slice/.../XXX.scope (looks like some
> > > systemd stuff). And since /sys/fs/cgroup/cpu/tasks doesn't exit in the
> > > system with cgroup v2 only, the script skips moving the target process
> > > to the root cgroup, and therefore the subsequent test fails.
> >
> > In my case I ssh as root and end up in
> > /user.slice/user-0.slice/session-1.scope cgroup,
> > /sys/fs/cgroup/cpu/tasks also doesn't exist, and yet the test passes.
> > The same also happens when I ssh as a regular user (with cgroup
> > /user.slice/user-1000.slice/session-3.scope) and run the testsuite
> > with sudo. So there must be something more to it... maybe some kernel
> > config or sysctl setting?
>
> As a further data point, I also have been unable to reproduce the
> reported behavior.
> That said, since tasks doesn't exist, isn't the passing test a false
> negative (i.e. it isn't truly testing as intended)?

I don't think it is. The test wants to verify that it is possible to
change the scheduling policy with the SELinux permission and that it
is not possible without it - and it tests basically identical
conditions with the permission allowed and denied, so it indeed
verifies it correctly. The cgroup switch is just a preparation step so
that changing the policy to realtime policies can succeed at all. When
the test fully passes without switching the cgroup, then it just means
that the switch wasn't necessary for whatever reason.

-- 
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.

Re: [PATCH testsuite] tests/task_setscheduler: add cgroup v2 case for moving proc to root cgroup

[Stephen Smalley]

On Tue, Jul 30, 2024 at 4:15 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> On Mon, Jul 29, 2024 at 1:55 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Mon, Jul 29, 2024 at 6:29 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > >
> > > On Sat, Jul 27, 2024 at 4:55 AM Gong Ruiqi <gongruiqi1@huawei.com> wrote:
> > > >
> > > >
> > > > On 2024/07/26 21:43, Ondrej Mosnacek wrote:
> > > > > On Thu, Jul 18, 2024 at 2:34 PM Gong Ruiqi <gongruiqi1@huawei.com> wrote:
> > > > >>
> > > > >>
> > > > >> On 2024/07/18 0:17, Paul Moore wrote:
> > > > >>> ...
> > > > >>>
> > > > >>> Where (what distribution, version, etc.) did you see this problem?
> > > > >>
> > > > >> The problem occurred when I ran the testsuite on Fedora 40 with v6.6
> > > > >> kernel, and it was the only failed testcase.
> > > > >
> > > > > Sorry for the delay... For some reason the test passes for me even
> > > > > with cgroup v2 only and without the patch (also when run from a
> > > > > regular user account with sudo). Do you happen to know what
> > > > > circumstances are needed for it to fail when the cgroup is not
> > > > > switched?
> > > > >
> > > >
> > > > As the comment in the script says, a process need to be in the root
> > > > cgroup in order to switch its scheduler policy to SCHED_{RR,FIFO}. So
> > > > maybe in your case the shell process is already in the root cgroup?
> > > >
> > > > In my case I need to ssh to a Fedora VM, and that makes my shell process
> > > > to be in a sub cgroup called /user.slice/.../XXX.scope (looks like some
> > > > systemd stuff). And since /sys/fs/cgroup/cpu/tasks doesn't exit in the
> > > > system with cgroup v2 only, the script skips moving the target process
> > > > to the root cgroup, and therefore the subsequent test fails.
> > >
> > > In my case I ssh as root and end up in
> > > /user.slice/user-0.slice/session-1.scope cgroup,
> > > /sys/fs/cgroup/cpu/tasks also doesn't exist, and yet the test passes.
> > > The same also happens when I ssh as a regular user (with cgroup
> > > /user.slice/user-1000.slice/session-3.scope) and run the testsuite
> > > with sudo. So there must be something more to it... maybe some kernel
> > > config or sysctl setting?
> >
> > As a further data point, I also have been unable to reproduce the
> > reported behavior.
> > That said, since tasks doesn't exist, isn't the passing test a false
> > negative (i.e. it isn't truly testing as intended)?
>
> I don't think it is. The test wants to verify that it is possible to
> change the scheduling policy with the SELinux permission and that it
> is not possible without it - and it tests basically identical
> conditions with the permission allowed and denied, so it indeed
> verifies it correctly. The cgroup switch is just a preparation step so
> that changing the policy to realtime policies can succeed at all. When
> the test fully passes without switching the cgroup, then it just means
> that the switch wasn't necessary for whatever reason.

Ok, thank you for clarifying.