Skip to content
Snippets Groups Projects
Unverified Commit 7d94cec9 authored by Ondrej Mosnacek's avatar Ondrej Mosnacek
Browse files

selinux: fix race condition when computing ocontext SIDs

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2083580

commit cbfcd13b
Author: Ondrej Mosnacek <omosnace@redhat.com>
Date:   Wed Jul 28 16:03:13 2021 +0200

    selinux: fix race condition when computing ocontext SIDs

    Current code contains a lot of racy patterns when converting an
    ocontext's context structure to an SID. This is being done in a "lazy"
    fashion, such that the SID is looked up in the SID table only when it's
    first needed and then cached in the "sid" field of the ocontext
    structure. However, this is done without any locking or memory barriers
    and is thus unsafe.

    Between commits 24ed7fda ("selinux: use separate table for initial
    SID lookup") and 66f8e2f0 ("selinux: sidtab reverse lookup hash
    table"), this race condition lead to an actual observable bug, because a
    pointer to the shared sid field was passed directly to
    sidtab_context_to_sid(), which was using this location to also store an
    intermediate value, which could have been read by other threads and
    interpreted as an SID. In practice this caused e.g. new mounts to get a
    wrong (seemingly random) filesystem context, leading to strange denials.
    This bug has been spotted in the wild at least twice, see [1] and [2].

    Fix the race condition by making all the racy functions use a common
    helper that ensures the ocontext::sid accesses are made safely using the
    appropriate SMP constructs.

    Note that security_netif_sid() was populating the sid field of both
    contexts stored in the ocontext, but only the first one was actually
    used. The SELinux wiki's documentation on the "netifcon" policy
    statement [3] suggests that using only the first context is intentional.
    I kept only the handling of the first context here, as there is really
    no point in doing the SID lookup for the unused one.

    I wasn't able to reproduce the bug mentioned above on any kernel that
    includes commit 66f8e2f0, even though it has been reported that the
    issue occurs with that commit, too, just less frequently. Thus, I wasn't
    able to verify that this patch fixes the issue, but it makes sense to
    avoid the race condition regardless.

    [1] https://github.com/containers/container-selinux/issues/89
    [2] https://lists.fedoraproject.org/archives/list/selinux@lists.fedoraproject.org/thread/6DMTAMHIOAOEMUAVTULJD45JZU7IBAFM/
    [3] https://selinuxproject.org/page/NetworkStatements#netifcon



    Cc: stable@vger.kernel.org
    Cc: Xinjie Zheng <xinjie@google.com>
Reported-by: default avatarSujithra Periasamy <sujithra@google.com>
    Fixes: 1da177e4 ("Linux-2.6.12-rc2")
Signed-off-by: default avatarOndrej Mosnacek <omosnace@redhat.com>
Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>

Signed-off-by: default avatarOndrej Mosnacek <omosnace@redhat.com>
parent eaa2c36a
No related branches found
No related tags found
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment