Re: [PATCH v38 04/39] LSM: Maintain a table of LSM attribute data

[Tetsuo Handa]

On 2022/10/21 8:42, Casey Schaufler wrote:
> On 10/13/2022 3:04 AM, Tetsuo Handa wrote:
>> On 2022/09/28 4:53, Casey Schaufler wrote:
>>> @@ -483,6 +491,16 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>>>  {
>>>  	int i;
>>>  
>>> +	/*
>>> +	 * A security module may call security_add_hooks() more
>>> +	 * than once. Landlock is one such case.
>>> +	 */
>>> +	if (lsm_id == 0 || lsm_idlist[lsm_id - 1] != lsmid)
>>> +		lsm_idlist[lsm_id++] = lsmid;
>>> +
>>> +	if (lsm_id > LSMID_ENTRIES)
>>> +		panic("%s Too many LSMs registered.\n", __func__);
>> I'm not happy with LSMID_ENTRIES. This is a way towards forever forbidding LKM-based LSMs.
> 
> I don't see any way given the locking issues that we're ever going to
> mix built in security modules and loaded security modules on the same
> hook lists. The SELinux module deletion code is sufficiently scary that
> it is being removed. That does not mean that I think loadable modules
> are impossible, I think it means that their management is going to have
> to be separate, the same way the BPF programs are handled. The only way
> that I see a unified hook list is for all the LSMs to be implemented as
> loadable modules, and I can't see that happening in my lifetime.

I'm not expecting for unloadable LSM modules.
I'm expecting for loadable LSM modules.

I'm not expecting to make all LSM modules to be implemented as loadable
LSM modules, for some want to associate "security label" to everything
(including processes which might start before the global init process starts)
but others do not need to associate "security label" to everything.

> 
> I can see an LSM like BPF, as I mentioned before, that manages loaded
> modules. Over the years I've seen several designs that might work. I'm
> encouraged (and not a little bit frightened) by the success of the BPF
> work.

There can be LSM modules whose lifetime of hooks match the lifetime of
a process which registered hooks for that process. In that case, being
automatically unregistered upon process termination would be preferable.

But there are LSM modules whose lifetime of hooks is irrelevant to a process
which registered a hook for that process. In that case, we need a method for
allowing registered hooks to remain even after that process terminated.

Please don't think loadable LSM modules as something that require special
handling. TOMOYO is an LSM module whose lifetime of hooks is irrelevant to
a process which registered a hook for that process, but does not need to
associate "security label" to everything. It has to be trivial to convert
TOMOYO as a loadable LSM module.

> 
> Converting the array[LSMID_ENTRIES] implementation to a hlist like the
> hooks have used would not be that big a project and I don't see that
> making such a change would be a show-stopper for implementing loadable
> modules. I think that a lot of other issues would be more significant.

Defining constants for each LSM module (i.e. "LSM: Add an LSM identifier
for external use") is the show-stopper for implementing loadable modules.
We won't be able to accept whatever LSM modules to upstream, and we won't
be able to enable whatever LSM modules in distributor kernels.

LSM modules which cannot define a constant due to either "not accepted
to upstream" or "not enabled by distributor kernels" will be forbidden.
I expect that we assign a constant upon module registration (instead of
API visible constants) if we require all LSM modules to have a constant.

> 
> I will, on the other hand, listen to compelling arguments. It is not the
> intention of this code to lock out loadable modules. If I thought it would
> I would not have proposed it.

This code is exactly for locking out loadable modules.