linux/security/selinux/ss
Ondrej Mosnacek 39a706fbcf selinux: fix sidtab string cache locking
Avoiding taking a lock in an IRQ context is not enough to prevent
deadlocks, as discovered by syzbot:

===
WARNING: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected
5.5.0-syzkaller #0 Not tainted
-----------------------------------------------------
syz-executor.0/8927 [HC0[0]:SC0[2]:HE1:SE0] is trying to acquire:
ffff888027c94098 (&(&s->cache_lock)->rlock){+.+.}, at: spin_lock include/linux/spinlock.h:338 [inline]
ffff888027c94098 (&(&s->cache_lock)->rlock){+.+.}, at: sidtab_sid2str_put.part.0+0x36/0x880 security/selinux/ss/sidtab.c:533

and this task is already holding:
ffffffff898639b0 (&(&nf_conntrack_locks[i])->rlock){+.-.}, at: spin_lock include/linux/spinlock.h:338 [inline]
ffffffff898639b0 (&(&nf_conntrack_locks[i])->rlock){+.-.}, at: nf_conntrack_lock+0x17/0x70 net/netfilter/nf_conntrack_core.c:91
which would create a new lock dependency:
 (&(&nf_conntrack_locks[i])->rlock){+.-.} -> (&(&s->cache_lock)->rlock){+.+.}

but this new dependency connects a SOFTIRQ-irq-safe lock:
 (&(&nf_conntrack_locks[i])->rlock){+.-.}

[...]

other info that might help us debug this:

 Possible interrupt unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&(&s->cache_lock)->rlock);
                               local_irq_disable();
                               lock(&(&nf_conntrack_locks[i])->rlock);
                               lock(&(&s->cache_lock)->rlock);
  <Interrupt>
    lock(&(&nf_conntrack_locks[i])->rlock);

 *** DEADLOCK ***
[...]
===

Fix this by simply locking with irqsave/irqrestore and stop giving up on
!in_task(). It makes the locking a bit slower, but it shouldn't make a
big difference in real workloads. Under the scenario from [1] (only
cache hits) it only increased the runtime overhead from the
security_secid_to_secctx() function from ~2% to ~3% (it was ~5-65%
before introducing the cache).

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1733259

Fixes: d97bd23c2d ("selinux: cache the SID -> context string translation")
Reported-by: syzbot+61cba5033e2072d61806@syzkaller.appspotmail.com
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: Paul Moore <paul@paul-moore.com>
2020-02-05 18:31:10 -05:00
..
avtab.c selinux: convert to kvmalloc 2019-03-12 10:04:02 -07:00
avtab.h treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 372 2019-06-05 17:37:10 +02:00
conditional.c treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 372 2019-06-05 17:37:10 +02:00
conditional.h treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 372 2019-06-05 17:37:10 +02:00
constraint.h License cleanup: add SPDX GPL-2.0 license identifier to files with no license 2017-11-02 11:10:55 +01:00
context.h selinux: sidtab reverse lookup hash table 2019-12-09 16:14:51 -05:00
ebitmap.c selinux: default_range glblub implementation 2019-10-07 19:01:35 -04:00
ebitmap.h selinux: default_range glblub implementation 2019-10-07 19:01:35 -04:00
hashtab.c selinux: wrap global selinux state 2018-03-01 18:48:02 -05:00
hashtab.h selinux: wrap global selinux state 2018-03-01 18:48:02 -05:00
mls.c selinux: default_range glblub implementation 2019-10-07 19:01:35 -04:00
mls.h selinux: overhaul sidtab to fix bug and improve performance 2018-12-05 16:12:32 -05:00
mls_types.h License cleanup: add SPDX GPL-2.0 license identifier to files with no license 2017-11-02 11:10:55 +01:00
policydb.c selinux: fix wrong buffer types in policydb.c 2020-01-16 14:31:05 -05:00
policydb.h selinux: randomize layout of key structures 2019-12-18 21:26:06 -05:00
services.c selinux: do not allocate ancillary buffer on first load 2020-01-16 16:05:25 -05:00
services.h selinux: randomize layout of key structures 2019-12-18 21:26:06 -05:00
sidtab.c selinux: fix sidtab string cache locking 2020-02-05 18:31:10 -05:00
sidtab.h selinux: cache the SID -> context string translation 2019-12-09 16:14:51 -05:00
status.c treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 500 2019-06-19 17:09:55 +02:00
symtab.c License cleanup: add SPDX GPL-2.0 license identifier to files with no license 2017-11-02 11:10:55 +01:00
symtab.h License cleanup: add SPDX GPL-2.0 license identifier to files with no license 2017-11-02 11:10:55 +01:00