Hi Stephen, thanks for your comments. I'm still in the process of trying to get a cleaned up version running, using rcu and incorporating your comments. I will send it out this week, i hope. Quoting Stephen Smalley (sds@private): > On Tue, 2004-11-23 at 15:31, Serge Hallyn wrote: > > Attached is the next set of patches to implement stacking through > > chaining. > > - I noticed that your patch changes SELinux to use unregister_security() > for runtime disable rather than explicitly resetting security_ops to > secondary_ops. While I agree abstractly that we shouldn't be manually ... > - If someone were to use the stacker module to stack capabilities and > SELinux (or any module and SELinux), they would actually end up with > capabilities (or the other module) + SELinux + dummy, given the > retention of the secondary_ops in SELinux. Further, as the stacker > module does not call the stacked modules upon a register_security hook, > there would no longer be any way to change secondary_ops for SELinux > from dummy to any other setting, so the only way to stack capabilities > and SELinux with the stacker module would be via the stacker module > itself, correct? Hence, secondary_ops becomes useless in the presence > of the stacker at present. Stacker now exports a sysfs file "next_secondary" which would be used as follows: stacker compiled in selinux compiled in echo selinux > next_secondary modprobe capability To get capability loaded as a secondary under selinux under stacker. In SELinux, then, SELinux still unregisters, but if a secondary is loaded other than dummy, then SELinux manually loads that secondary whichever way (ie 'register_security' or "mod_unreg_security") selinux itself had registered. Does all that seem like a reasonable approach? > - You don't want the #ifdef CONFIG_SECURITY_STACKER stuff in security.h, > because that means that enabling/disabling the stacker requires a major > rebuild even though it only truly affects the security subtree, right? Right. Taken out, thanks. > - Claim that the stacker_capable() hook shouldn't just use > RETURN_ERROR_IF_ANY_ERROR() seems dubious to me; capable hook is > authoritative with respect to the core kernel, but I would still > typically expect restrictive behavior for multiple modules. That claim was inaccurate - the function was doing mostly the same thing as RETURN_ERROR_IF_ANY_ERROR, but with its own shortcircuit variable. Perhaps another variable should be added which lets stacker_capable act the way the original did, that is, authoritatively? I don't want to make this too complicated... > In general, > stacker module needs major cleanup. That it does. Hopefully the next version will be in far better shape. Until now I was concentrating on the kernel object annotation sharing patch... > - stacker-selinux-procattr-hack.patch doesn't seem to work as expected; > booting a kernel built with stacker+selinux+capabilities, I get vast > numbers of "stacker_setprocattr: no data after module name" errors and The new stacker (at least for now) simply calls the selinux getprocattr and setprocattr. If selinux is not loaded, it returns -ENOENT. I know of no LSMs other than my own and SELinux which used them anyway. thanks, -serge
This archive was generated by hypermail 2.1.3 : Mon Nov 29 2004 - 14:26:21 PST