Messages in this thread | | | From | Jann Horn <> | Date | Thu, 6 Aug 2020 18:08:36 +0200 | Subject | Re: 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()?
| |