• Todd Kjos's avatar
    ANDROID: binder: remove WARN() for redundant txn error · e46a3b3b
    Todd Kjos authored
    binder_send_failed_reply() is called when a synchronous
    transaction fails. It reports an error to the thread that
    is waiting for the completion. Given that the transaction
    is synchronous, there should never be more than 1 error
    response to that thread -- this was being asserted with
    a WARN().
    
    However, when exercising the driver with syzbot tests, cases
    were observed where multiple "synchronous" requests were
    sent without waiting for responses, so it is possible that
    multiple errors would be reported to the thread. This testing
    was conducted with panic_on_warn set which forced the crash.
    
    This is easily reproduced by sending back-to-back
    "synchronous" transactions without checking for any
    response (eg, set read_size to 0):
    
        bwr.write_buffer = (uintptr_t)&bc1;
        bwr.write_size = sizeof(bc1);
        bwr.read_buffer = (uintptr_t)&br;
        bwr.read_size = 0;
        ioctl(fd, BINDER_WRITE_READ, &bwr);
        sleep(1);
        bwr2.write_buffer = (uintptr_t)&bc2;
        bwr2.write_size = sizeof(bc2);
        bwr2.read_buffer = (uintptr_t)&br;
        bwr2.read_size = 0;
        ioctl(fd, BINDER_WRITE_READ, &bwr2);
        sleep(1);
    
    The first transaction is sent to the servicemanager and the reply
    fails because no VMA is set up by this client. After
    binder_send_failed_reply() is called, the BINDER_WORK_RETURN_ERROR
    is sitting on the thread's todo list since the read_size was 0 and
    the client is not waiting for a response.
    
    The 2nd transaction is sent and the BINDER_WORK_RETURN_ERROR has not
    been consumed, so the thread's reply_error.cmd is still set (normally
    cleared when the BINDER_WORK_RETURN_ERROR is handled). Therefore
    when the servicemanager attempts to reply to the 2nd failed
    transaction, the error is already set and it triggers this warning.
    
    This is a user error since it is not waiting for the synchronous
    transaction to complete. If it ever does check, it will see an
    error.
    
    Changed the WARN() to a pr_warn().
    Signed-off-by: default avatarTodd Kjos <tkjos@android.com>
    Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
    Cc: stable <stable@vger.kernel.org>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    e46a3b3b
binder.c 162 KB