Re: [PATCH v6] LSM: Multiple concurrent LSMs

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). 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.



> > 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.



> > 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.



> > 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?

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... )
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... ).

For accelerating the development of custom LSM module which users really want,
LSM interface should be opened to allow LKM-based LSM modules.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html