Here is my personal todo list based on this conversation. - change example policy in commit message to demonstrate intended use. No raw ioctl values. - Look into making logic more general, less ioctl specific - Look at making the code clearer. I.e. address Paul's comments on lack of clarity in struct/variable naming. In the spirit of keeping this commit concise and as basic as possible (it's already 800 LOC!) I will not address suggestions to propagate additional policy information such as ioctl names and groups into the kernel binary. I agree that would be useful, but I will leave as future work. Regarding comments on policy syntax, those will be addressed in a separate non-kernel commit to the selinux project. Thanks again for all the feedback! On Thu, May 21, 2015 at 8:27 AM, Joshua Brindle <brindle@xxxxxxxxxxxxxxxxx> wrote: > Stephen Smalley wrote: >> >> On 05/21/2015 10:57 AM, Joshua Brindle wrote: >>> >>> William Roberts wrote: >>>> >>>> On May 21, 2015 7:53 AM, "Joshua Brindle"<brindle@xxxxxxxxxxxxxxxxx> >>>> wrote: >>>>> >>>>> William Roberts wrote: >>>>>> >>>>>> On Wed, May 20, 2015 at 9:17 PM, Jeffrey Vander >>>>>> Stoep<jeffv@xxxxxxxxxx> >>>>>> wrote: >>>>>> >>>>>>> Thanks for all the feedback and suggestions. Agreed that raw >>>>>>> numerical >>>>>>> values are confusing. I will fix up the commit message to set a >>>>>>> better >>>>>>> precedent for intended use. I included them more to illustrate what >>>>>>> is >>>>>>> happening under the hood. I like the idea of a qualifier for clarity. >>>>>>> The qualifier seems necessary for the suggested non-ioctl-specific >>>>>>> approach. >>>>>>> >>>>>>> Individual ioctl labels are only marginally better than raw numbers. >>>>>>> E.g. { TCSETSF TIOCGWINSZ TCGETA TCSETA TCSETAW TCSETAF TCSBRK TCXONC >>>>>>> TIOCMBIS }. More helpful...but not much. >>>>>>> >>>>>>> My plan was to group commonly used ioctl sets into macros. >>>>>>> >>>>>>> e.g. common_socket_ioc, priv_socket_ioc, tty_ioc, gpu_ioc, etc >>>>>>> >>>>>> This is exactly what I had in mind, just use m4 >>>>>> >>>>> Even outside of my analysis use case I think this is not a good idea. >>>>> >>>>> Consider the Android base policy defined gpu_op as a set of ioctls, and >>>> >>>> there were related rules for gpu users defined there. Then a device >>>> variant >>>> policy has additional gpu_op ioctl's. How does it add them? Does it >>>> have to >>>> re-add all of the related rules with the new ioctls? >>>> >>>> I could define a new macro my_device_GPU_ioctls and include the base >>>> macro >>>> in its expansion. >>> >>> And repeat every related rule (and keep them in sync as the base policy >>> develops). How much easier is it just to add your ioctl to an ioctl >>> attribute and be done? >> >> >> Technically not required for that purpose, e.g. I can do this today: >> $ cat device/lge/hammerhead/sepolicy/global_macros >> define(`r_file_perms', `{ ' r_file_perms `write }') >> and have it automatically add write everywhere we allow r_file_perms >> even in the core policy, because the build process unions on a >> file-by-file basis. >> >> Effectively your attribute would only be useful as inline documentation; >> it would have no binding to the actual semantic meaning of the check. >> If the policy writer allows it as gpu_ioc in policy but the driver is >> actually something other than a gpu driver or chooses to interpret a >> particular command value included in that attribute in some other way, >> then it might have an entirely different effect. So it might be helpful >> but not sufficient for analysis. > > > Unless the attributes were specific to the type in question: > > ioctl gpu_device gpu_ioc { 0x8910-0x8926 0x892A-0x8935 }; > > allow surfaceflinger gpu_device : chr_file gpu_ioc; > > This would 1) enforce the policy authors intention, 2) force policy authors > to be specific with what ioctls mean what with which devices and 3) allow > more meaningful analysis. > > _______________________________________________ > Selinux mailing list > Selinux@xxxxxxxxxxxxx > To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx. > To get help, send an email containing "help" to > Selinux-request@xxxxxxxxxxxxx. _______________________________________________ Selinux mailing list Selinux@xxxxxxxxxxxxx To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx. To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.