Re: v5.1-rc1 binder_alloc_do_buffer_copy() BUG_ON triggered by selinux-testsuite

[Todd Kjos <tkjos@google.com>]

On Wed, Mar 20, 2019 at 12:50 PM Todd Kjos <tkjos@google.com> wrote:
>
> Paul,
>
> Looking at main() in test_binder.c...
>
> int main(int argc, char **argv)
> {
>
> [...]
>
>   // Line 493
>   struct binder_write_read bwr;
>   struct flat_binder_object obj;
>   struct {
>     uint32_t cmd;
>     struct binder_transaction_data txn;
>   } __attribute__((packed)) writebuf;
>   unsigned int readbuf[32];
>
> [...]
>   // Line 630
>   writebuf.txn.data.ptr.buffer = (uintptr_t)&obj;
>   writebuf.txn.data.ptr.offsets = (uintptr_t)&obj +   // [A]
>                                                  sizeof(struct
> flat_binder_object);
>
>   bwr.write_buffer = (uintptr_t)&writebuf;
>   bwr.write_size = sizeof(writebuf);
>
> It looks like bwr.txn.data.ptr.offsets points off the end of obj (see
> [A] above), which means the binder driver will read compiler-dependent
> stack data as the offset for the object. If it happens to be 0, then
> the test will work (read the object from offset 0). If it's not 0,
> then most likely offset > data_size (which is what found that BUG_ON
> case). With my patch applied, this will just cause an error to be
> returned (what you are seeing now).
>
> Same thing when you test with v5.0 -- if the offset happens to be 0,
> then the test will succeed. If not, then the test will fail because
> the transaction fails in an unexpected way.

Same issue at line 296 of test_binder.c when setting up the
transaction in request_manager_fd().

>
> -Todd
>
>
[...]