On Thu, Mar 21, 2019 at 2:50 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > On Thu, Mar 21, 2019 at 12:26 AM Todd Kjos <tkjos@xxxxxxxxxx> wrote: > > I can send you a patch tomorrow (I won't be able to test it though). > > So, I was a bit quicker than you and I think I managed to fix the test myself :) > > See: > https://github.com/SELinuxProject/selinux-testsuite/pull/50/commits/b559c3f54eae6130cb9e79c295b0f94db26e09e4 Looks good. Thanks! > > It seems the problem was indeed in the offsets array handling as you > discovered. With the above commit included the PR#50 version of the > binder test no longer fails for me. > > Thanks, > > > > > On Wed, Mar 20, 2019 at 4:23 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > > > > > On Wed, Mar 20, 2019 at 3:50 PM Todd Kjos <tkjos@xxxxxxxxxx> 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 > > > > -- > Ondrej Mosnacek <omosnace at redhat dot com> > Software Engineer, Security Technologies > Red Hat, Inc.
