Commit d3b990b7 authored by Paul Moore's avatar Paul Moore Committed by David S. Miller

netlabel: fix problems with mapping removal

This patch fixes two main problems seen when removing NetLabel
mappings: memory leaks and potentially extra audit noise.

The memory leaks are caused by not properly free'ing the mapping's
address selector struct when free'ing the entire entry as well as
not properly cleaning up a temporary mapping entry when adding new
address selectors to an existing entry.  This patch fixes both these
problems such that kmemleak reports no NetLabel associated leaks
after running the SELinux test suite.

The potentially extra audit noise was caused by the auditing code in
netlbl_domhsh_remove_entry() being called regardless of the entry's
validity.  If another thread had already marked the entry as invalid,
but not removed/free'd it from the list of mappings, then it was
possible that an additional mapping removal audit record would be
generated.  This patch fixes this by returning early from the removal
function when the entry was previously marked invalid.  This change
also had the side benefit of improving the code by decreasing the
indentation level of large chunk of code by one (accounting for most
of the diffstat).

Fixes: 63c41688 ("netlabel: Add network address selectors to the NetLabel/LSM domain mapping")
Reported-by: default avatarStephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 3106ecb4
...@@ -85,6 +85,7 @@ static void netlbl_domhsh_free_entry(struct rcu_head *entry) ...@@ -85,6 +85,7 @@ static void netlbl_domhsh_free_entry(struct rcu_head *entry)
kfree(netlbl_domhsh_addr6_entry(iter6)); kfree(netlbl_domhsh_addr6_entry(iter6));
} }
#endif /* IPv6 */ #endif /* IPv6 */
kfree(ptr->def.addrsel);
} }
kfree(ptr->domain); kfree(ptr->domain);
kfree(ptr); kfree(ptr);
...@@ -537,6 +538,8 @@ int netlbl_domhsh_add(struct netlbl_dom_map *entry, ...@@ -537,6 +538,8 @@ int netlbl_domhsh_add(struct netlbl_dom_map *entry,
goto add_return; goto add_return;
} }
#endif /* IPv6 */ #endif /* IPv6 */
/* cleanup the new entry since we've moved everything over */
netlbl_domhsh_free_entry(&entry->rcu);
} else } else
ret_val = -EINVAL; ret_val = -EINVAL;
...@@ -580,6 +583,12 @@ int netlbl_domhsh_remove_entry(struct netlbl_dom_map *entry, ...@@ -580,6 +583,12 @@ int netlbl_domhsh_remove_entry(struct netlbl_dom_map *entry,
{ {
int ret_val = 0; int ret_val = 0;
struct audit_buffer *audit_buf; struct audit_buffer *audit_buf;
struct netlbl_af4list *iter4;
struct netlbl_domaddr4_map *map4;
#if IS_ENABLED(CONFIG_IPV6)
struct netlbl_af6list *iter6;
struct netlbl_domaddr6_map *map6;
#endif /* IPv6 */
if (entry == NULL) if (entry == NULL)
return -ENOENT; return -ENOENT;
...@@ -597,6 +606,9 @@ int netlbl_domhsh_remove_entry(struct netlbl_dom_map *entry, ...@@ -597,6 +606,9 @@ int netlbl_domhsh_remove_entry(struct netlbl_dom_map *entry,
ret_val = -ENOENT; ret_val = -ENOENT;
spin_unlock(&netlbl_domhsh_lock); spin_unlock(&netlbl_domhsh_lock);
if (ret_val)
return ret_val;
audit_buf = netlbl_audit_start_common(AUDIT_MAC_MAP_DEL, audit_info); audit_buf = netlbl_audit_start_common(AUDIT_MAC_MAP_DEL, audit_info);
if (audit_buf != NULL) { if (audit_buf != NULL) {
audit_log_format(audit_buf, audit_log_format(audit_buf,
...@@ -606,24 +618,14 @@ int netlbl_domhsh_remove_entry(struct netlbl_dom_map *entry, ...@@ -606,24 +618,14 @@ int netlbl_domhsh_remove_entry(struct netlbl_dom_map *entry,
audit_log_end(audit_buf); audit_log_end(audit_buf);
} }
if (ret_val == 0) {
struct netlbl_af4list *iter4;
struct netlbl_domaddr4_map *map4;
#if IS_ENABLED(CONFIG_IPV6)
struct netlbl_af6list *iter6;
struct netlbl_domaddr6_map *map6;
#endif /* IPv6 */
switch (entry->def.type) { switch (entry->def.type) {
case NETLBL_NLTYPE_ADDRSELECT: case NETLBL_NLTYPE_ADDRSELECT:
netlbl_af4list_foreach_rcu(iter4, netlbl_af4list_foreach_rcu(iter4, &entry->def.addrsel->list4) {
&entry->def.addrsel->list4) {
map4 = netlbl_domhsh_addr4_entry(iter4); map4 = netlbl_domhsh_addr4_entry(iter4);
cipso_v4_doi_putdef(map4->def.cipso); cipso_v4_doi_putdef(map4->def.cipso);
} }
#if IS_ENABLED(CONFIG_IPV6) #if IS_ENABLED(CONFIG_IPV6)
netlbl_af6list_foreach_rcu(iter6, netlbl_af6list_foreach_rcu(iter6, &entry->def.addrsel->list6) {
&entry->def.addrsel->list6) {
map6 = netlbl_domhsh_addr6_entry(iter6); map6 = netlbl_domhsh_addr6_entry(iter6);
calipso_doi_putdef(map6->def.calipso); calipso_doi_putdef(map6->def.calipso);
} }
...@@ -639,7 +641,6 @@ int netlbl_domhsh_remove_entry(struct netlbl_dom_map *entry, ...@@ -639,7 +641,6 @@ int netlbl_domhsh_remove_entry(struct netlbl_dom_map *entry,
#endif /* IPv6 */ #endif /* IPv6 */
} }
call_rcu(&entry->rcu, netlbl_domhsh_free_entry); call_rcu(&entry->rcu, netlbl_domhsh_free_entry);
}
return ret_val; return ret_val;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment