lkml.org 
[lkml]   [2020]   [Aug]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: WARNING in binder_transaction_buffer_release (2)
On Thu, Aug 6, 2020 at 1:19 PM syzbot
<syzbot+e113a0b970b7b3f394ba@syzkaller.appspotmail.com> wrote:
> syzbot suspects this issue was fixed by commit:
>
> commit 4b836a1426cb0f1ef2a6e211d7e553221594f8fc
> Author: Jann Horn <jannh@google.com>
> Date: Mon Jul 27 12:04:24 2020 +0000
>
> binder: Prevent context manager from incrementing ref 0
[...]
> dashboard link: https://syzkaller.appspot.com/bug?extid=e113a0b970b7b3f394ba
[...]
> If the result looks correct, please mark the issue as fixed by replying with:
>
> #syz fix: binder: Prevent context manager from incrementing ref 0

I think this issue still exists, syzbot probably just hit it in a
weird way that doesn't work anymore.

This warning:

case BINDER_TYPE_FD: {
/*
* No need to close the file here since user-space
* closes it for for successfully delivered
* transactions. For transactions that weren't
* delivered, the new fd was never allocated so
* there is no need to close and the fput on the
* file is done when the transaction is torn
* down.
*/
WARN_ON(failed_at &&
proc->tsk == current->group_leader);
} break;

can be false-positive if the sender and recipient of the transaction
are associated with the same task_struct. But there isn't really any
reason why you wouldn't be able to have sender and recipient in the
same process, as long as the binder_proc is different.
(binder_transaction() has a weird check that refuses transactions to
handle 0 based on task_struct equality - which IMO doesn't really make
sense -, but transactions to other handles can happen just fine even
if both ends are in the same task_struct.)

Maybe the best fix is just to rip out that WARN_ON()?

\
 
 \ /
  Last update: 2020-08-06 18:37    [W:0.067 / U:0.788 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site