ANDROID: qtaguid: Fix the UAF probelm with tag_ref_tree
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 Git-repo: https://android.googlesource.com/kernel/msm Git-commit: a6661da56dc61b67cc65222b71896a775ceb17be Signed-off-by: Dennis Cagle <dcagle@codeaurora.org>
This commit is contained in:
parent
3425f556fd
commit
33566fe241
|
@ -518,13 +518,11 @@ static struct tag_ref *get_tag_ref(tag_t full_tag,
|
|||
|
||||
DR_DEBUG("qtaguid: get_tag_ref(0x%llx)\n",
|
||||
full_tag);
|
||||
spin_lock_bh(&uid_tag_data_tree_lock);
|
||||
tr_entry = lookup_tag_ref(full_tag, &utd_entry);
|
||||
BUG_ON(IS_ERR_OR_NULL(utd_entry));
|
||||
if (!tr_entry)
|
||||
tr_entry = new_tag_ref(full_tag, utd_entry);
|
||||
|
||||
spin_unlock_bh(&uid_tag_data_tree_lock);
|
||||
if (utd_res)
|
||||
*utd_res = utd_entry;
|
||||
DR_DEBUG("qtaguid: get_tag_ref(0x%llx) utd=%p tr=%p\n",
|
||||
|
@ -2017,6 +2015,7 @@ static int ctrl_cmd_delete(const char *input)
|
|||
|
||||
/* Delete socket tags */
|
||||
spin_lock_bh(&sock_tag_list_lock);
|
||||
spin_lock_bh(&uid_tag_data_tree_lock);
|
||||
node = rb_first(&sock_tag_tree);
|
||||
while (node) {
|
||||
st_entry = rb_entry(node, struct sock_tag, sock_node);
|
||||
|
@ -2046,6 +2045,7 @@ static int ctrl_cmd_delete(const char *input)
|
|||
list_del(&st_entry->list);
|
||||
}
|
||||
}
|
||||
spin_unlock_bh(&uid_tag_data_tree_lock);
|
||||
spin_unlock_bh(&sock_tag_list_lock);
|
||||
|
||||
sock_tag_tree_erase(&st_to_free_tree);
|
||||
|
@ -2250,10 +2250,12 @@ static int ctrl_cmd_tag(const char *input)
|
|||
full_tag = combine_atag_with_uid(acct_tag, uid);
|
||||
|
||||
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);
|
||||
tag_ref_entry = get_tag_ref(full_tag, &uid_tag_data_entry);
|
||||
if (IS_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);
|
||||
goto err_put;
|
||||
}
|
||||
|
@ -2287,16 +2289,20 @@ static int ctrl_cmd_tag(const char *input)
|
|||
pr_err("qtaguid: ctrl_tag(%s): "
|
||||
"socket tag alloc failed\n",
|
||||
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);
|
||||
res = -ENOMEM;
|
||||
goto err_tag_unref_put;
|
||||
goto err_put;
|
||||
}
|
||||
sock_tag_entry->sk = el_socket->sk;
|
||||
sock_tag_entry->socket = el_socket;
|
||||
sock_tag_entry->pid = current->tgid;
|
||||
sock_tag_entry->tag = combine_atag_with_uid(acct_tag,
|
||||
uid);
|
||||
spin_lock_bh(&uid_tag_data_tree_lock);
|
||||
pqd_entry = proc_qtu_data_tree_search(
|
||||
&proc_qtu_data_tree, current->tgid);
|
||||
/*
|
||||
|
@ -2314,11 +2320,11 @@ static int ctrl_cmd_tag(const char *input)
|
|||
else
|
||||
list_add(&sock_tag_entry->list,
|
||||
&pqd_entry->sock_tag_list);
|
||||
spin_unlock_bh(&uid_tag_data_tree_lock);
|
||||
|
||||
sock_tag_tree_insert(sock_tag_entry, &sock_tag_tree);
|
||||
atomic64_inc(&qtu_events.sockets_tagged);
|
||||
}
|
||||
spin_unlock_bh(&uid_tag_data_tree_lock);
|
||||
spin_unlock_bh(&sock_tag_list_lock);
|
||||
/* We keep the ref to the socket (file) until it is untagged */
|
||||
CT_DEBUG("qtaguid: ctrl_tag(%s): done st@%p ...->f_count=%ld\n",
|
||||
|
@ -2326,10 +2332,6 @@ static int ctrl_cmd_tag(const char *input)
|
|||
atomic_long_read(&el_socket->file->f_count));
|
||||
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:
|
||||
CT_DEBUG("qtaguid: ctrl_tag(%s): done. ...->f_count=%ld\n",
|
||||
input, atomic_long_read(&el_socket->file->f_count) - 1);
|
||||
|
|
Loading…
Reference in New Issue