On 12/9/19 8:53 PM, Paul Moore wrote:
In AVC insert we don't call avc_node_kill() when avc_xperms_populate()
fails, resulting in the avc->avc_cache.active_nodes counter having a
false value.
incorrect value?
This patch corrects this problem and does some cleanup
in avc_insert() while we are there.
submitting-patches.rst recommends describing in imperative mood and
avoiding the words "patch" in what will eventually just be a commit log,
ala "Correct this problem and perform some cleanup..."
Should probably add a:
Fixes: fa1aa143ac4a ("selinux: extended permissions for ioctls")
Might be easier to back port if you split the cleanup from the fix, but
your call of course.
Reported-by: rsiddoji@xxxxxxxxxxxxxx
Suggested-by: Stephen Smalley <sds@xxxxxxxxxxxxx>
Signed-off-by: Paul Moore <paul@xxxxxxxxxxxxxx>
Acked-by: Stephen Smalley <sds@xxxxxxxxxxxxx>
---
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 23dc888ae305..6646300f7ccb 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;
}
