ANDROID: binder: fix transaction leak.
PATCH 1/1 ANDROID: binder: fix transaction leak.
If a call to put_user() fails, we failed to
properly free a transaction and send a failed
reply (if necessary).
Signed-off-by Martijn Coenen <[email protected]>
--- drivers/android/binder.c | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-)
drivers/android/binder.c +31 -9
@@ -1947,6 +1947,26 @@ static void binder_send_failed_reply(struct binder_transaction *t, } } +/** +* binder_cleanup_transaction() - cleans up undelivered transaction +* @t: transaction that needs to be cleaned up +* @reason: reason the transaction wasn't delivered +* @error_code: error to return to caller (if synchronous call) +*/ +static void binder_cleanup_transaction (struct binder_transaction *t, +const char *reason, +uint32_t error_code) +{ +if (t->buffer->target_node && !(t->flags &TF_ONE_WAY )) { +binder_send_failed_reply (t, error_code); + }else { +binder_debug (BINDER_DEBUG_DEAD_TRANSACTION , +"undelivered transaction %d, %s\n" , + t->debug_id, reason); +binder_free_transaction (t); + } +} +/** * binder_validate_object() - checks for a valid metadata object in a buffer. * @buffer: binder_buffer that we're parsing.
@@ -4015,12 +4035,20 @@ static int binder_thread_read(struct binder_proc *proc,if (put_user (cmd, (uint32_t __user *)ptr)) {if (t_from)binder_thread_dec_tmpref (t_from); + +binder_cleanup_transaction (t,"put_user failed" , +BR_FAILED_REPLY ); +return -EFAULT ; } ptr +=sizeof (uint32_t );if (copy_to_user (ptr, &tr,sizeof (tr))) {if (t_from)binder_thread_dec_tmpref (t_from); + +binder_cleanup_transaction (t,"copy_to_user failed" , +BR_FAILED_REPLY ); +return -EFAULT ; } ptr +=sizeof (tr);
@@ -4090,15 +4118,9 @@ static void binder_release_work(struct binder_proc *proc,struct binder_transaction *t; t = container_of(w, struct binder_transaction, work); -if (t->buffer->target_node && - !(t->flags &TF_ONE_WAY )) { -binder_send_failed_reply (t,BR_DEAD_REPLY ); - }else { -binder_debug (BINDER_DEBUG_DEAD_TRANSACTION , -"undelivered transaction %d\n" , - t->debug_id); -binder_free_transaction (t); - } + +binder_cleanup_transaction (t,"process died." , +BR_DEAD_REPLY ); }break ;case BINDER_WORK_RETURN_ERROR : { struct binder_error *e = container_of(
Signature · 1 line
2.15.0.448.gf294e3d99a-goog
Greg KH
maintainerQuoted 8 lines · Martijn Coenen
If a call to
put_user()fails, we failed to
properly free a transaction and send a failed
reply (if necessary).Signed-off-by Martijn Coenen <[email protected]>--- drivers/android/binder.c | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-)
Is this relevant for 4.14 and any older kernels as well?
thanks,
greg k-h
Quoted 1 line · Greg KH
Is this relevant for 4.14 and any older kernels as well?
The problem was introduced with fine-grained locking, which is 4.14 and up only.
Thanks,
Martijn