Re: [PATCH AUTOSEL 5.5 190/542] selinux: ensure we cleanup the internal AVC counters on error in avc_insert()

On 2/14/20 10:43 AM, Sasha Levin wrote:

From: Paul Moore paul@paul-moore.com

[ Upstream commit d8db60cb23e49a92cf8cada3297395c7fa50fdf8 ]

Fix avc_insert() to call avc_node_kill() if we've already allocated an AVC node and the code fails to insert the node in the cache.

Fixes: fa1aa143ac4a ("selinux: extended permissions for ioctls") Reported-by: rsiddoji@codeaurora.org Suggested-by: Stephen Smalley sds@tycho.nsa.gov Acked-by: Stephen Smalley sds@tycho.nsa.gov Signed-off-by: Paul Moore paul@paul-moore.com Signed-off-by: Sasha Levin sashal@kernel.org

You should also apply 030b995ad9ece9fa2d218af4429c1c78c2342096 ("selinux: ensure we cleanup the internal AVC counters on error in avc_update()") which fixes one additional instance of the same kind of bug not addressed by this patch.

---

security/selinux/avc.c | 51 ++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/security/selinux/avc.c b/security/selinux/avc.c index 23dc888ae3056..6646300f7ccb2 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -617,40 +617,37 @@ static struct avc_node *avc_insert(struct selinux_avc *avc, struct avc_node *pos, *node = NULL; int hvalue; unsigned long flag;

• spinlock_t *lock;
• struct hlist_head *head;

if (avc_latest_notif_update(avc, avd->seqno, 1))

• goto out;
  

• return NULL;
  

node = avc_alloc_node(avc);

• if (node) {

• struct hlist_head *head;
  

• spinlock_t *lock;
  

• int rc = 0;
  

• hvalue = avc_hash(ssid, tsid, tclass);
  

• avc_node_populate(node, ssid, tsid, tclass, avd);
  

• rc = avc_xperms_populate(node, xp_node);
  

• if (rc) {
  

• 	kmem_cache_free(avc_node_cachep, node);
  

• 	return NULL;
  

• }
  

• head = &avc->avc_cache.slots[hvalue];
  

• lock = &avc->avc_cache.slots_lock[hvalue];
  

• if (!node)

• return NULL;
  

• spin_lock_irqsave(lock, flag);
  

• hlist_for_each_entry(pos, head, list) {
  

• 	if (pos->ae.ssid == ssid &&
  

• 	    pos->ae.tsid == tsid &&
  

• 	    pos->ae.tclass == tclass) {
  

• 		avc_node_replace(avc, node, pos);
  

• 		goto found;
  

• 	}
  

• avc_node_populate(node, ssid, tsid, tclass, avd);
• if (avc_xperms_populate(node, xp_node)) {

• avc_node_kill(avc, node);
  

• return NULL;
  

• }
• hvalue = avc_hash(ssid, tsid, tclass);
• head = &avc->avc_cache.slots[hvalue];
• lock = &avc->avc_cache.slots_lock[hvalue];
• spin_lock_irqsave(lock, flag);
• hlist_for_each_entry(pos, head, list) {

• if (pos->ae.ssid == ssid &&
  

• 	pos->ae.tsid == tsid &&
  

• 	pos->ae.tclass == tclass) {
  

• 	avc_node_replace(avc, node, pos);
  

• 	goto found;
  

  }

• hlist_add_head_rcu(&node->list, head);
  

-found:

• spin_unlock_irqrestore(lock, flag);
  

  }

-out:

• hlist_add_head_rcu(&node->list, head);

+found:

• spin_unlock_irqrestore(lock, flag); return node; }