• Jann Horn's avatar
    binder: Prevent context manager from incrementing ref 0 · 74e42c22
    Jann Horn authored
    commit 4b836a14 upstream.
    
    Binder is designed such that a binder_proc never has references to
    itself. If this rule is violated, memory corruption can occur when a
    process sends a transaction to itself; see e.g.
    <https://syzkaller.appspot.com/bug?extid=09e05aba06723a94d43d>.
    
    There is a remaining edgecase through which such a transaction-to-self
    can still occur from the context of a task with BINDER_SET_CONTEXT_MGR
    access:
    
     - task A opens /dev/binder twice, creating binder_proc instances P1
       and P2
     - P1 becomes context manager
     - P2 calls ACQUIRE on the magic handle 0, allocating index 0 in its
       handle table
     - P1 dies (by closing the /dev/binder fd and waiting a bit)
     - P2 becomes context manager
     - P2 calls ACQUIRE on the magic handle 0, allocating index 1 in its
       handle table
       [this triggers a warning: "binder: 1974:1974 tried to acquire
       reference to desc 0, got 1 instead"]
     - task B opens /dev/binder once, creating binder_proc instance P3
     - P3 calls P2 (via magic handle 0) with (void*)1 as argument (two-way
       transaction)
     - P2 receives the handle and uses it to call P3 (two-way transaction)
     - P3 calls P2 (via magic handle 0) (two-way transaction)
     - P2 calls P2 (via handle 1) (two-way transaction)
    
    And then, if P2 does *NOT* accept the incoming transaction work, but
    instead closes the binder fd, we get a crash.
    
    Solve it by preventing the context manager from using ACQUIRE on ref 0.
    There shouldn't be any legitimate reason for the context manager to do
    that.
    
    Additionally, print a warning if someone manages to find another way to
    trigger a transaction-to-self bug in the future.
    
    Cc: stable@vger.kernel.org
    Fixes: 457b9a6f ("Staging: android: add binder driver")
    Acked-by: default avatarTodd Kjos <tkjos@google.com>
    Signed-off-by: default avatarJann Horn <jannh@google.com>
    Reviewed-by: default avatarMartijn Coenen <maco@android.com>
    Link: https://lore.kernel.org/r/20200727120424.1627555-1-jannh@google.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    74e42c22
binder.c 164 KB