On Wed, Apr 7, 2021 at 3:24 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
>
> Since commit 1b8b31a2e612 ("selinux: convert policy read-write lock to
> RCU"), there is a small window during policy load where the new policy
> pointer has already been installed, but some threads may still be
> holding the old policy pointer in their read-side RCU critical sections.
> This means that there may be conflicting attempts to add a new SID entry
> to both tables via sidtab_context_to_sid().
>
> See also (and the rest of the thread):
> https://lore.kernel.org/selinux/CAFqZXNvfux46_f8gnvVvRYMKoes24nwm2n3sPbMjrB8vKTW00g@xxxxxxxxxxxxxx/
>
> Fix this by installing the new policy pointer under the old sidtab's
> spinlock along with marking the old sidtab as "frozen". Then, if an
> attempt to add new entry to a "frozen" sidtab is detected, make
> sidtab_context_to_sid() return -ESTALE to indicate that a new policy
> has been installed and that the caller will have to abort the policy
> transaction and try again after re-taking the policy pointer (which is
> guaranteed to be a newer policy). This requires adding a retry-on-ESTALE
> logic to all callers of sidtab_context_to_sid(), but fortunately these
> are easy to determine and aren't that many.
>
> This seems to be the simplest solution for this problem, even if it
> looks somewhat ugly. Note that other places in the kernel (e.g.
> do_mknodat() in fs/namei.c) use similar stale-retry patterns, so I think
> it's reasonable.
>
> Fixes: 1b8b31a2e612 ("selinux: convert policy read-write lock to RCU")
> Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> ---
>
> v3: correctly handle initial allocation in security_get_user_sids()
> v2: reset cladatum to NULL on retry in security_compute_sid()
>
> security/selinux/ss/services.c | 157 +++++++++++++++++++++++++--------
> security/selinux/ss/sidtab.c | 21 +++++
> security/selinux/ss/sidtab.h | 4 +
> 3 files changed, 145 insertions(+), 37 deletions(-)
Third time's the charm :) Thanks Ondrej, I've merged this into
selinux/stable-5.12 with the stable CC; assuming testing goes well
I'll send this up to Linus later this week.
--
paul moore
www.paul-moore.com
