working-selinuxns rebase

working-selinuxns rebase

[Stephen Smalley]

I did a re-base of the working-selinuxns branch on top of latest next;
this required manual conflict fixes due to the encapsulation of the
policy state and refactoring of policy reload.  The rebase can be
found at:
https://github.com/stephensmalley/selinux-kernel/tree/working-selinuxns-rebase

It boots, passes the selinux-testsuite, and passes the following
trivial exercising of the unshare mechanism:
$ sudo bash
# echo 1 > /sys/fs/selinux/unshare
# unshare -m -n
# umount /sys/fs/selinux
# mount -t selinuxfs none /sys/fs/selinux
# id
uid=0(root) gid=0(root) groups=0(root) context=kernel
# getenforce
Permissive
# load_policy
# id
uid=0(root) gid=0(root) groups=0(root) context=system_u:system_r:kernel_t:s0

All the same caveats apply - this is still not safe to use and has
many unresolved issues as noted in the patch descriptions.

Re: working-selinuxns rebase

[Paul Moore]

On Tue, Aug 18, 2020 at 9:37 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> I did a re-base of the working-selinuxns branch on top of latest next;
> this required manual conflict fixes due to the encapsulation of the
> policy state and refactoring of policy reload.  The rebase can be
> found at:
> https://github.com/stephensmalley/selinux-kernel/tree/working-selinuxns-rebase
>
> It boots, passes the selinux-testsuite, and passes the following
> trivial exercising of the unshare mechanism:
> $ sudo bash
> # echo 1 > /sys/fs/selinux/unshare
> # unshare -m -n
> # umount /sys/fs/selinux
> # mount -t selinuxfs none /sys/fs/selinux
> # id
> uid=0(root) gid=0(root) groups=0(root) context=kernel
> # getenforce
> Permissive
> # load_policy
> # id
> uid=0(root) gid=0(root) groups=0(root) context=system_u:system_r:kernel_t:s0
>
> All the same caveats apply - this is still not safe to use and has
> many unresolved issues as noted in the patch descriptions.

Thanks Stephen, do you mind if I pull that into the working-selinuxns
branch in the main SELinux repo?

-- 
paul moore
www.paul-moore.com

Re: working-selinuxns rebase

[Stephen Smalley]

On Wed, Aug 19, 2020 at 9:28 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Tue, Aug 18, 2020 at 9:37 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > I did a re-base of the working-selinuxns branch on top of latest next;
> > this required manual conflict fixes due to the encapsulation of the
> > policy state and refactoring of policy reload.  The rebase can be
> > found at:
> > https://github.com/stephensmalley/selinux-kernel/tree/working-selinuxns-rebase
> >
> > It boots, passes the selinux-testsuite, and passes the following
> > trivial exercising of the unshare mechanism:
> > $ sudo bash
> > # echo 1 > /sys/fs/selinux/unshare
> > # unshare -m -n
> > # umount /sys/fs/selinux
> > # mount -t selinuxfs none /sys/fs/selinux
> > # id
> > uid=0(root) gid=0(root) groups=0(root) context=kernel
> > # getenforce
> > Permissive
> > # load_policy
> > # id
> > uid=0(root) gid=0(root) groups=0(root) context=system_u:system_r:kernel_t:s0
> >
> > All the same caveats apply - this is still not safe to use and has
> > many unresolved issues as noted in the patch descriptions.
>
> Thanks Stephen, do you mind if I pull that into the working-selinuxns
> branch in the main SELinux repo?

Unfortunately I need to re-base it again and manually fix conflicts
with my patch to avoid deferencing the policy prior to initialization.
And I'll need to do it again when/if the patch to convert the policy
rwlock to rcu lands.  So you might want to wait. I'm starting to
wonder if the first patch in the series to rename selinux_state/state
to selinux_ns/ns throughout is a mistake because it produces a lot of
unnecessary conflicts.  Originally I did it because that was the
original naming since the encapsulation started to support namespacing
and then I did a mass rename to selinux_state/state for upstreaming
since I wasn't yet upstreaming the actual namespace support. Renaming
it back again reduces conflicts in the later patches but makes the
first one a pain.  But if I just do a mass rename on all the later
patches then I can drop the first one and avoid these unnecessary
conflicts.  Thoughts?

Re: working-selinuxns rebase

[Paul Moore]

On Thu, Aug 20, 2020 at 8:10 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> Unfortunately I need to re-base it again and manually fix conflicts
> with my patch to avoid deferencing the policy prior to initialization.
> And I'll need to do it again when/if the patch to convert the policy
> rwlock to rcu lands.  So you might want to wait. I'm starting to
> wonder if the first patch in the series to rename selinux_state/state
> to selinux_ns/ns throughout is a mistake because it produces a lot of
> unnecessary conflicts.  Originally I did it because that was the
> original naming since the encapsulation started to support namespacing
> and then I did a mass rename to selinux_state/state for upstreaming
> since I wasn't yet upstreaming the actual namespace support. Renaming
> it back again reduces conflicts in the later patches but makes the
> first one a pain.  But if I just do a mass rename on all the later
> patches then I can drop the first one and avoid these unnecessary
> conflicts.  Thoughts?

I agree, the first patch is the one that always causes me the most
pain; considering the work-in-progress state of the patches I think it
would make the most sense to drop that initial cosmetic patch for now
and we can always reinstate it at the end when this work finally
lands.

-- 
paul moore
www.paul-moore.com

Re: working-selinuxns rebase

[Stephen Smalley]

On Thu, Aug 20, 2020 at 9:17 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Thu, Aug 20, 2020 at 8:10 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> > Unfortunately I need to re-base it again and manually fix conflicts
> > with my patch to avoid deferencing the policy prior to initialization.
> > And I'll need to do it again when/if the patch to convert the policy
> > rwlock to rcu lands.  So you might want to wait. I'm starting to
> > wonder if the first patch in the series to rename selinux_state/state
> > to selinux_ns/ns throughout is a mistake because it produces a lot of
> > unnecessary conflicts.  Originally I did it because that was the
> > original naming since the encapsulation started to support namespacing
> > and then I did a mass rename to selinux_state/state for upstreaming
> > since I wasn't yet upstreaming the actual namespace support. Renaming
> > it back again reduces conflicts in the later patches but makes the
> > first one a pain.  But if I just do a mass rename on all the later
> > patches then I can drop the first one and avoid these unnecessary
> > conflicts.  Thoughts?
>
> I agree, the first patch is the one that always causes me the most
> pain; considering the work-in-progress state of the patches I think it
> would make the most sense to drop that initial cosmetic patch for now
> and we can always reinstate it at the end when this work finally
> lands.

I've made a pass at this and force-pushed it to my
working-selinuxns-rebase branch.
It turned out that the first patch did two things: it renamed state to
ns and it changed all direct references to &selinux_state to use a new
current_selinux_state pointer to a static init_selinux_state variable
(in preparation for multiple states/namespaces).  I had to retain the
latter so I just dropped the renaming part of it, rewrote the
description, and did a mass rename in all the subsequent patches back
to state. So the first patch may still produce some conflicts but
there should be fewer of them.  This is relative to your current next
branch but it will need to be manually re-based again when/if the
policy rcu patches land, so feel free to wait if you want.  Since
every patch required modification and many of them required manual
fixups, I dropped all of your Signed-off-by lines and rewrote all of
mine with my current preferred email address.

Re: working-selinuxns rebase

[Stephen Smalley]

On Fri, Aug 21, 2020 at 5:00 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Thu, Aug 20, 2020 at 9:17 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > On Thu, Aug 20, 2020 at 8:10 AM Stephen Smalley
> > <stephen.smalley.work@gmail.com> wrote:
> > > Unfortunately I need to re-base it again and manually fix conflicts
> > > with my patch to avoid deferencing the policy prior to initialization.
> > > And I'll need to do it again when/if the patch to convert the policy
> > > rwlock to rcu lands.  So you might want to wait. I'm starting to
> > > wonder if the first patch in the series to rename selinux_state/state
> > > to selinux_ns/ns throughout is a mistake because it produces a lot of
> > > unnecessary conflicts.  Originally I did it because that was the
> > > original naming since the encapsulation started to support namespacing
> > > and then I did a mass rename to selinux_state/state for upstreaming
> > > since I wasn't yet upstreaming the actual namespace support. Renaming
> > > it back again reduces conflicts in the later patches but makes the
> > > first one a pain.  But if I just do a mass rename on all the later
> > > patches then I can drop the first one and avoid these unnecessary
> > > conflicts.  Thoughts?
> >
> > I agree, the first patch is the one that always causes me the most
> > pain; considering the work-in-progress state of the patches I think it
> > would make the most sense to drop that initial cosmetic patch for now
> > and we can always reinstate it at the end when this work finally
> > lands.
>
> I've made a pass at this and force-pushed it to my
> working-selinuxns-rebase branch.
> It turned out that the first patch did two things: it renamed state to
> ns and it changed all direct references to &selinux_state to use a new
> current_selinux_state pointer to a static init_selinux_state variable
> (in preparation for multiple states/namespaces).  I had to retain the
> latter so I just dropped the renaming part of it, rewrote the
> description, and did a mass rename in all the subsequent patches back
> to state. So the first patch may still produce some conflicts but
> there should be fewer of them.  This is relative to your current next
> branch but it will need to be manually re-based again when/if the
> policy rcu patches land, so feel free to wait if you want.  Since
> every patch required modification and many of them required manual
> fixups, I dropped all of your Signed-off-by lines and rewrote all of
> mine with my current preferred email address.

Re-based again on latest next and it was much easier this time around.
Will need to do it again once Ondrej's patches and my policy_mutex
patch land but hopefully not too much work.

Re: working-selinuxns rebase

[Stephen Smalley]

On Wed, Aug 26, 2020 at 4:50 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Fri, Aug 21, 2020 at 5:00 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Thu, Aug 20, 2020 at 9:17 PM Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > On Thu, Aug 20, 2020 at 8:10 AM Stephen Smalley
> > > <stephen.smalley.work@gmail.com> wrote:
> > > > Unfortunately I need to re-base it again and manually fix conflicts
> > > > with my patch to avoid deferencing the policy prior to initialization.
> > > > And I'll need to do it again when/if the patch to convert the policy
> > > > rwlock to rcu lands.  So you might want to wait. I'm starting to
> > > > wonder if the first patch in the series to rename selinux_state/state
> > > > to selinux_ns/ns throughout is a mistake because it produces a lot of
> > > > unnecessary conflicts.  Originally I did it because that was the
> > > > original naming since the encapsulation started to support namespacing
> > > > and then I did a mass rename to selinux_state/state for upstreaming
> > > > since I wasn't yet upstreaming the actual namespace support. Renaming
> > > > it back again reduces conflicts in the later patches but makes the
> > > > first one a pain.  But if I just do a mass rename on all the later
> > > > patches then I can drop the first one and avoid these unnecessary
> > > > conflicts.  Thoughts?
> > >
> > > I agree, the first patch is the one that always causes me the most
> > > pain; considering the work-in-progress state of the patches I think it
> > > would make the most sense to drop that initial cosmetic patch for now
> > > and we can always reinstate it at the end when this work finally
> > > lands.
> >
> > I've made a pass at this and force-pushed it to my
> > working-selinuxns-rebase branch.
> > It turned out that the first patch did two things: it renamed state to
> > ns and it changed all direct references to &selinux_state to use a new
> > current_selinux_state pointer to a static init_selinux_state variable
> > (in preparation for multiple states/namespaces).  I had to retain the
> > latter so I just dropped the renaming part of it, rewrote the
> > description, and did a mass rename in all the subsequent patches back
> > to state. So the first patch may still produce some conflicts but
> > there should be fewer of them.  This is relative to your current next
> > branch but it will need to be manually re-based again when/if the
> > policy rcu patches land, so feel free to wait if you want.  Since
> > every patch required modification and many of them required manual
> > fixups, I dropped all of your Signed-off-by lines and rewrote all of
> > mine with my current preferred email address.
>
> Re-based again on latest next and it was much easier this time around.
> Will need to do it again once Ondrej's patches and my policy_mutex
> patch land but hopefully not too much work.

I'm also considering whether to drop the two patches that were
externally contributed.
The one from James to mark init_selinux_state/ns as ro_after_init
seems kind of pointless; it isn't really used after init except for a
check in the runtime disable code so I don't see any gain from making
it read-only.  The one from Peter to introduce a lockdep class for
what used to be the services (security server) locks is partly
obsoleted by the elimination of the policy rwlock and the description
no longer fits since the status lock got moved up earlier from the
selinux_ss to the selinux_state.

Re: working-selinuxns rebase

[Stephen Smalley]

On Wed, Aug 26, 2020 at 4:54 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Wed, Aug 26, 2020 at 4:50 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Fri, Aug 21, 2020 at 5:00 PM Stephen Smalley
> > <stephen.smalley.work@gmail.com> wrote:
> > >
> > > On Thu, Aug 20, 2020 at 9:17 PM Paul Moore <paul@paul-moore.com> wrote:
> > > >
> > > > On Thu, Aug 20, 2020 at 8:10 AM Stephen Smalley
> > > > <stephen.smalley.work@gmail.com> wrote:
> > > > > Unfortunately I need to re-base it again and manually fix conflicts
> > > > > with my patch to avoid deferencing the policy prior to initialization.
> > > > > And I'll need to do it again when/if the patch to convert the policy
> > > > > rwlock to rcu lands.  So you might want to wait. I'm starting to
> > > > > wonder if the first patch in the series to rename selinux_state/state
> > > > > to selinux_ns/ns throughout is a mistake because it produces a lot of
> > > > > unnecessary conflicts.  Originally I did it because that was the
> > > > > original naming since the encapsulation started to support namespacing
> > > > > and then I did a mass rename to selinux_state/state for upstreaming
> > > > > since I wasn't yet upstreaming the actual namespace support. Renaming
> > > > > it back again reduces conflicts in the later patches but makes the
> > > > > first one a pain.  But if I just do a mass rename on all the later
> > > > > patches then I can drop the first one and avoid these unnecessary
> > > > > conflicts.  Thoughts?
> > > >
> > > > I agree, the first patch is the one that always causes me the most
> > > > pain; considering the work-in-progress state of the patches I think it
> > > > would make the most sense to drop that initial cosmetic patch for now
> > > > and we can always reinstate it at the end when this work finally
> > > > lands.
> > >
> > > I've made a pass at this and force-pushed it to my
> > > working-selinuxns-rebase branch.
> > > It turned out that the first patch did two things: it renamed state to
> > > ns and it changed all direct references to &selinux_state to use a new
> > > current_selinux_state pointer to a static init_selinux_state variable
> > > (in preparation for multiple states/namespaces).  I had to retain the
> > > latter so I just dropped the renaming part of it, rewrote the
> > > description, and did a mass rename in all the subsequent patches back
> > > to state. So the first patch may still produce some conflicts but
> > > there should be fewer of them.  This is relative to your current next
> > > branch but it will need to be manually re-based again when/if the
> > > policy rcu patches land, so feel free to wait if you want.  Since
> > > every patch required modification and many of them required manual
> > > fixups, I dropped all of your Signed-off-by lines and rewrote all of
> > > mine with my current preferred email address.
> >
> > Re-based again on latest next and it was much easier this time around.
> > Will need to do it again once Ondrej's patches and my policy_mutex
> > patch land but hopefully not too much work.
>
> I'm also considering whether to drop the two patches that were
> externally contributed.
> The one from James to mark init_selinux_state/ns as ro_after_init
> seems kind of pointless; it isn't really used after init except for a
> check in the runtime disable code so I don't see any gain from making
> it read-only.  The one from Peter to introduce a lockdep class for
> what used to be the services (security server) locks is partly
> obsoleted by the elimination of the policy rwlock and the description
> no longer fits since the status lock got moved up earlier from the
> selinux_ss to the selinux_state.

Re-based again and dropped the two externally contributed patches; we
can always revive those later and add them on top if desired but I
don't think they are essential.

Re: working-selinuxns rebase

[Paul Moore]

On Mon, Aug 31, 2020 at 11:52 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> Re-based again and dropped the two externally contributed patches; we
> can always revive those later and add them on top if desired but I
> don't think they are essential.

Thanks Stephen, I just updated selinux/working-selinuxns with your
tree.  I think the rationale for dropping James' and Peter's patches
make sense; if we decide we need them back I keep all of the previous
working-selinuxns branches in my local tree so it should be easy
enough to restore/port them.

-- 
paul moore
www.paul-moore.com