On 11/7/2012 5:15 AM, Tetsuo Handa wrote: > Reordering the message for explanation... > > Casey Schaufler wrote: >> On 11/6/2012 4:44 AM, Tetsuo Handa wrote: >>> sizeof(*lp) should be 32 rather than 12 on x86_32 unless CONFIG_SLOB=y . >> So the reality is that I'd be happy to report the number of lsm_list >> blobs rather than the size, if that would be more palatable. Or, not >> report a quantity at all, just say "Leaked some bytes while delisting >> an LSM" and be done. Do you feel strongly one way or the other? Does >> anyone else care? > ksize(lp) returns exact size compared to sizeof(*lp). That was easy to adopt. > But another approach is > to embed "struct list_head" into "struct security_operations", something like > > struct security_operations { > char name[SECURITY_NAME_MAX + 1]; > > int (*ptrace_access_check) (struct task_struct *child, unsigned int mode); > struct list_head ptrace_access_check_list; > int (*ptrace_traceme) (struct task_struct *parent); > struct list_head ptrace_traceme_list; > int (*capget) (struct task_struct *target, > kernel_cap_t *effective, > kernel_cap_t *inheritable, kernel_cap_t *permitted); > struct list_head capget_list; > (...snipped...) > }; > > so that we can avoid kmalloc()/kfree() upon registration/unregistration. How does one go about adding the hooks to the appropriate lists? I came up with one macro based method that made Eric's hair fall out, and he never even saw it. I don't think embedding a list for each hook into the security_operations structure gets anything other than bloat. Because of typing you would need a separate enlist and delist function for each hook. Try it if you don't believe me. >>> Seems unsafe list deletion. I think we need to use _rcu version since somebody >>> (kernel threads) may be walking on the list when lsm_delist_ops() is called by >>> the init process. >> Here's the question, and I don't know the answer. lsm_delist() gets called >> under two conditions: an early memory allocation failure that will almost >> certainly precede a fatal memory allocation error, or a call to >> security_reset_ops(), which is strictly a one-time thing. Might it be >> better to have a lock of some sort to deal with the unlikely case of a >> call to lsm_delist than to use the _rcu versions? I honestly don't know >> what the cost of using _rcu is, so it may not matter. > Is list_for_each_entry() safe against concurrent list_del() without locks? > If I understand correctly, it isn't safe to call list_for_each_entry() without > locks if list elements can be removed by list_del(). > > I considered singly linked list (rather than using doubly linked list with _rcu > primitives) so that list traversal without locks becomes safe with regard to > list addition/deletion. Concurrent list addition/deletion is not safe, but we > can afford using locks only for protecting such case because list > addition/deletion happens only upon registration/unregistration. That would entail fighting with the list macro use purists. >>> Maybe we want noinline attribute, for inlining lsm_delist() into >>> lsm_delist_ops() is a bloat. And so does noinline attribute to lsm_enlist(). >> I'm hesitant to try to second guess the compilers on this. If a >> performance expert told me to do it, I would, but I'd hate to be too >> clever. > Thinking from how frequently lsm_enlist()/lsm_delist() is called, I think > it does not worth inlining lsm_enlist()/lsm_delist() calls. I agree, I also don't think is worth specifying noinline. >>> Overall, this version can eliminate overhead caused by unimplemented hooks. >>> Then, I think we can afford allowing legal registration of trivial LKM-based >>> LSM modules after the init process starts; LSM modules which wants to use >>> lsm_blobs are limited to COMPOSER_MAX but LSM modules which do not need >>> lsm_blobs are allowed to chain as many as the administrator wants. >> So if an LSM includes no hooks that use blobs none would get >> allocated. Yama, for example. There would have to be a separate >> load order from blob slot number. I'm tempted to say it's too >> rare a case to worry about, but we have the example of Yama >> sitting there challenging that assertion. >> >> I will consider this. > Thank you. > Does adding 'bool use_blobs' field to "struct security_operations" help? Only if LSM writers are trustworthy and careful. I expect that a check at registration on the hooks that use blobs would be sufficient. I will check all the inode, file, cred and such hooks and if any are called the LSM gets a slot. > > There are three reasons I want to allow legal registration of LKM-based LSM > modules after the init process starts. > > First is that many distributions nowadays enable multiple LSM modules. Since > LSM modules cannot be a LKM, all possible LSM modules have to be built-in, > bloating the vmlinuz file. If LSM modules are allowed to be a LKM, distributors > can reduce the size of vmlinuz (making the kernel load time shorter). TOMOYO is > sitting here waiting for legal registration of LKM-based LSM modules. > > Second is that several distributions (e.g. Fedora and RHEL) enable only one LSM > module (e.g. SELinux). On such distributions, users who want to use other LSM > modules at their own risk have to replace the kernel package; this limitation > is a strong psychological barrier and maintenance burden for such users. I'm > using AKARI (which is a LKM-based TOMOYO) for troubleshooting RHEL systems > because replacing the kernel package (in order to use TOMOYO) is unlikely > acceptable for RHEL users. > > Third is that LSM is not the tool for thought control. LSM should be the tool > for minimizing the burden of all subsystems' maintainers. > I hope LSM folks have learned that "not everybody can configure SELinux" and > "even AppArmor is not configurable for somebody" > ( http://events.linuxfoundation.org/images/stories/pdf/lcna_co2012_handa.pdf ) > and "Yama is sufficient for some users" and other (not yet in-tree) modules may > be sufficient for some other users > ( http://events.linuxfoundation.org/images/stories/pdf/lcna_co2012_henderson.pdf ). > > For accelerating the development of custom LSM module which users really want, > LSM interface should be opened to allow LKM-based LSM modules. > -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with the words "unsubscribe selinux" without quotes as the message.