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

[Todd Kjos]

I can send you a patch tomorrow (I won't be able to test it though).

On Wed, Mar 20, 2019 at 4:23 PM Paul Moore <paul at paul-moore.com> wrote:
>
> On Wed, Mar 20, 2019 at 3:50 PM Todd Kjos <tkjos at 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.
>
> That might explain why the test used to work, but now fails - a
> different compiler (I rebuild the test before each test run).
>
> Keeping in mind I'm really quite ignorant when it comes to binder, how
> would you suggest fixing the test?
>
> --
> paul moore
> www.paul-moore.com