TechPanelGM/kernel_xiaomi_sm8250
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ANDROID: qtaguid: Fix the UAF probelm with tag_ref_tree

Browse Source 
When multiple threads is trying to tag/delete the same socket at the
same time, there is a chance the tag_ref_entry of the target socket to
be null before the uid_tag_data entry is freed. It is caused by the
ctrl_cmd_tag function where it doesn't correctly grab the spinlocks
when tagging a socket.

Signed-off-by: Chenbo Feng <fengc@google.com>
Bug: 65853158
Change-Id: I5d89885918054cf835370a52bff2d693362ac5f0
This commit is contained in:
Chenbo Feng
2017-11-28 18:22:11 -08:00
committed by Amit Pundir
parent 2f6e1d62d4
commit b4d74821e0
1 changed files with 11 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
net/netfilter/xt_qtaguid.c
View File

@@ -520,13 +520,11 @@ static struct tag_ref *get_tag_ref(tag_t full_tag,
DR_DEBUG("qtaguid: get_tag_ref(0x%llx)\n", DR_DEBUG("qtaguid: get_tag_ref(0x%llx)\n",
full_tag); full_tag);
spin_lock_bh(&uid_tag_data_tree_lock);
tr_entry = lookup_tag_ref(full_tag, &utd_entry); tr_entry = lookup_tag_ref(full_tag, &utd_entry);
BUG_ON(IS_ERR_OR_NULL(utd_entry)); BUG_ON(IS_ERR_OR_NULL(utd_entry));
if (!tr_entry) if (!tr_entry)
tr_entry = new_tag_ref(full_tag, utd_entry); tr_entry = new_tag_ref(full_tag, utd_entry);
spin_unlock_bh(&uid_tag_data_tree_lock);
if (utd_res) if (utd_res)
*utd_res = utd_entry; *utd_res = utd_entry;
DR_DEBUG("qtaguid: get_tag_ref(0x%llx) utd=%p tr=%p\n", DR_DEBUG("qtaguid: get_tag_ref(0x%llx) utd=%p tr=%p\n",
@@ -2033,6 +2031,7 @@ static int ctrl_cmd_delete(const char *input)
/* Delete socket tags */ /* Delete socket tags */
spin_lock_bh(&sock_tag_list_lock); spin_lock_bh(&sock_tag_list_lock);
spin_lock_bh(&uid_tag_data_tree_lock);
node = rb_first(&sock_tag_tree); node = rb_first(&sock_tag_tree);
while (node) { while (node) {
st_entry = rb_entry(node, struct sock_tag, sock_node); st_entry = rb_entry(node, struct sock_tag, sock_node);
@@ -2062,6 +2061,7 @@ static int ctrl_cmd_delete(const char *input)
list_del(&st_entry->list); list_del(&st_entry->list);
} }
} }
spin_unlock_bh(&uid_tag_data_tree_lock);
spin_unlock_bh(&sock_tag_list_lock); spin_unlock_bh(&sock_tag_list_lock);
sock_tag_tree_erase(&st_to_free_tree); sock_tag_tree_erase(&st_to_free_tree);
@@ -2271,10 +2271,12 @@ static int ctrl_cmd_tag(const char *input)
full_tag = combine_atag_with_uid(acct_tag, uid_int); full_tag = combine_atag_with_uid(acct_tag, uid_int);
spin_lock_bh(&sock_tag_list_lock); spin_lock_bh(&sock_tag_list_lock);
spin_lock_bh(&uid_tag_data_tree_lock);
sock_tag_entry = get_sock_stat_nl(el_socket->sk); sock_tag_entry = get_sock_stat_nl(el_socket->sk);
tag_ref_entry = get_tag_ref(full_tag, &uid_tag_data_entry); tag_ref_entry = get_tag_ref(full_tag, &uid_tag_data_entry);
if (IS_ERR(tag_ref_entry)) { if (IS_ERR(tag_ref_entry)) {
res = PTR_ERR(tag_ref_entry); res = PTR_ERR(tag_ref_entry);
spin_unlock_bh(&uid_tag_data_tree_lock);
spin_unlock_bh(&sock_tag_list_lock); spin_unlock_bh(&sock_tag_list_lock);
goto err_put; goto err_put;
} }
@@ -2301,9 +2303,14 @@ static int ctrl_cmd_tag(const char *input)
pr_err("qtaguid: ctrl_tag(%s): " pr_err("qtaguid: ctrl_tag(%s): "
"socket tag alloc failed\n", "socket tag alloc failed\n",
input); input);
BUG_ON(tag_ref_entry->num_sock_tags <= 0);
tag_ref_entry->num_sock_tags--;
free_tag_ref_from_utd_entry(tag_ref_entry,
uid_tag_data_entry);
spin_unlock_bh(&uid_tag_data_tree_lock);
spin_unlock_bh(&sock_tag_list_lock); spin_unlock_bh(&sock_tag_list_lock);
res = -ENOMEM; res = -ENOMEM;
goto err_tag_unref_put; goto err_put;
} }
/* /*
* Hold the sk refcount here to make sure the sk pointer cannot * Hold the sk refcount here to make sure the sk pointer cannot
@@ -2313,7 +2320,6 @@ static int ctrl_cmd_tag(const char *input)
sock_tag_entry->sk = el_socket->sk; sock_tag_entry->sk = el_socket->sk;
sock_tag_entry->pid = current->tgid; sock_tag_entry->pid = current->tgid;
sock_tag_entry->tag = combine_atag_with_uid(acct_tag, uid_int); sock_tag_entry->tag = combine_atag_with_uid(acct_tag, uid_int);
spin_lock_bh(&uid_tag_data_tree_lock);
pqd_entry = proc_qtu_data_tree_search( pqd_entry = proc_qtu_data_tree_search(
&proc_qtu_data_tree, current->tgid); &proc_qtu_data_tree, current->tgid);
/* /*
@@ -2331,11 +2337,11 @@ static int ctrl_cmd_tag(const char *input)
else else
list_add(&sock_tag_entry->list, list_add(&sock_tag_entry->list,
&pqd_entry->sock_tag_list); &pqd_entry->sock_tag_list);
spin_unlock_bh(&uid_tag_data_tree_lock);
sock_tag_tree_insert(sock_tag_entry, &sock_tag_tree); sock_tag_tree_insert(sock_tag_entry, &sock_tag_tree);
atomic64_inc(&qtu_events.sockets_tagged); atomic64_inc(&qtu_events.sockets_tagged);
} }
spin_unlock_bh(&uid_tag_data_tree_lock);
spin_unlock_bh(&sock_tag_list_lock); spin_unlock_bh(&sock_tag_list_lock);
/* We keep the ref to the sk until it is untagged */ /* We keep the ref to the sk until it is untagged */
CT_DEBUG("qtaguid: ctrl_tag(%s): done st@%p ...->sk_refcnt=%d\n", CT_DEBUG("qtaguid: ctrl_tag(%s): done st@%p ...->sk_refcnt=%d\n",
@@ -2344,10 +2350,6 @@ static int ctrl_cmd_tag(const char *input)
sockfd_put(el_socket); sockfd_put(el_socket);
return 0; return 0;
err_tag_unref_put:
BUG_ON(tag_ref_entry->num_sock_tags <= 0);
tag_ref_entry->num_sock_tags--;
free_tag_ref_from_utd_entry(tag_ref_entry, uid_tag_data_entry);
err_put: err_put:
CT_DEBUG("qtaguid: ctrl_tag(%s): done. ...->sk_refcnt=%d\n", CT_DEBUG("qtaguid: ctrl_tag(%s): done. ...->sk_refcnt=%d\n",
input, refcount_read(&el_socket->sk->sk_refcnt) - 1); input, refcount_read(&el_socket->sk->sk_refcnt) - 1);