On Thu, Apr 11, 2019 at 7:48 AM Richard Haines <richard_c_haines@xxxxxxxxxxxxxx> wrote: > On Wed, 2019-04-10 at 19:43 -0400, Paul Moore wrote: > > On Wed, Apr 10, 2019 at 1:04 PM Richard Haines > > <richard_c_haines@xxxxxxxxxxxxxx> wrote: > > > On Wed, 2019-04-10 at 11:35 -0400, Paul Moore wrote: > > > > On Wed, Apr 3, 2019 at 8:43 AM Richard Haines > > > > <richard_c_haines@xxxxxxxxxxxxxx> wrote: > > > > > Replace binder_test.c with separate manager, client and service > > > > > provider. > > > > > This works in the same way as a service provider/client > > > > > interacts > > > > > with a service manager in the Android world. It passes the > > > > > service > > > > > providers binder file descriptor to the client for the > > > > > impersonate > > > > > permission check. > > > > > > > > > > Also added tests for Dynamically Allocated Binder Devices and > > > > > passing > > > > > the sender SELinux security context on binder transactions. > > > > > > > > > > Note that the tests require a minimum kernel of 4.16, else some > > > > > tests may > > > > > fail. To run successfully the "binder: Add thread->process_todo > > > > > flag" > > > > > patch may be required that is available from: > > > > > https://lore.kernel.org/patchwork/patch/851324/ > > > > > This patch has been backported to some earlier kernels. > > > > > > > > > > Signed-off-by: Richard Haines <richard_c_haines@xxxxxxxxxxxxxx> > > > > > --- > > > > > defconfig | 3 + > > > > > policy/test_binder.te | 176 ++++---- > > > > > tests/binder/.gitignore | 6 +- > > > > > tests/binder/Makefile | 13 +- > > > > > tests/binder/binder_common.c | 155 +++++++ > > > > > tests/binder/binder_common.h | 37 ++ > > > > > tests/binder/check_binder.c | 27 +- > > > > > tests/binder/check_binderfs.c | 53 +++ > > > > > tests/binder/client.c | 450 ++++++++++++++++++++ > > > > > tests/binder/manager.c | 362 ++++++++++++++++ > > > > > tests/binder/service_provider.c | 404 ++++++++++++++++++ > > > > > tests/binder/test | 257 ++++++++++-- > > > > > tests/binder/test_binder.c | 705 ------------------------ > > > > > ---- > > > > > ---- > > > > > 13 files changed, 1785 insertions(+), 863 deletions(-) > > > > > create mode 100644 tests/binder/binder_common.c > > > > > create mode 100644 tests/binder/binder_common.h > > > > > create mode 100644 tests/binder/check_binderfs.c > > > > > create mode 100644 tests/binder/client.c > > > > > create mode 100644 tests/binder/manager.c > > > > > create mode 100644 tests/binder/service_provider.c > > > > > delete mode 100644 tests/binder/test_binder.c > > > > > > > > Hi Richard, > > > > > > > > Welcome back :) > > > > > > > > I had hoped to spend some time reading up on Binder so I could > > > > give > > > > this a proper review, but that hasn't happened so I'm inclined to > > > > merge it, assuming it works on my test system. However, > > > > considering > > > > your comment about this not working on kernel's older than 4.16, > > > > I > > > > think we should probably add some checks to only run this test on > > > > systems with the appropriate kernel support. > > > > > > > > If you look at tests/Makefile you will see a number of distro > > > > specific > > > > test list modifications, and there is even an example of checking > > > > the > > > > kernel version (search for "kvercmp" in the Makefile). I would > > > > suggest a simple check to make sure the kernel is at least v4.16, > > > > and > > > > if we find distro specific support (e.g. a particular distro > > > > backported the listed patch) we can always add an exception for > > > > that > > > > distro. > > > > > > > > How does that sound? > > > > > > There are tests for the OS in the 'test' script already. I guess > > > you > > > need to check if these are okay, as I check if < 4.16 and if so > > > print > > > message saying if fail check for the patch. > > > > Ah ha, yes you did, and I missed it. Sorry about that. I checked > > the > > Makefiles, didn't see any checks, and wrongly assumed they were not > > there. > > > > Looking quickly at the check it seems reasonable, if I notice any > > problems when testing I'll let you know. > > > > > I'm not sure this will get to the list as I appear to be black- > > > balled. > > > I did send a cover letter with this patch + another one regarding > > > running SCTP on < 4.20.17. I sent email to > > > owner-selinux@xxxxxxxxxxxxxxx but not heard anything yet. > > > > Hmm, that's not good. FWIW, the original patch obviously made it, > > but > > yes I'm not seeing your response in the list archives. Did you get > > any sort of majordomo hate mail back on your posts, or is is just > > silently dropping your messages? > > Just silently dropping messages. I had thought to remove from list then > add again, however it might smell a rat so left until owner-selinux > returns. I can only repeat what Julius Caesar muttered "Infamy! Infamy! > They've all got it in for me!" (well in Carry On Cleo any way). I got the brexit jump label gag, but you lost me on the "Carry on Cleo"; thankfully good ol' Google came to the rescue on that one ;) On the plus side, your posts seem to be coming through to the list now. On the negative side I realized when playing with your test changes that I wasn't building BINDERFS in my test kernels - oops. I'm fixing that now, but I might not get a chance to do another test until tomorrow; at least I can verify that your BINDERFS testing logic works :) -- paul moore www.paul-moore.com