Re: [RFC PATCH 2/2] integrity: double check iint_cache was initialized

[Casey Schaufler]

On 3/24/2021 4:58 AM, Dmitry Vyukov wrote:
> On Wed, Mar 24, 2021 at 12:49 PM Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote:
>> On Wed, 2021-03-24 at 12:37 +0100, Dmitry Vyukov wrote:
>>> On Wed, Mar 24, 2021 at 12:21 PM Tetsuo Handa
>>> <penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote:
>>>> On 2021/03/24 20:10, Mimi Zohar wrote:
>>>>> On Wed, 2021-03-24 at 19:10 +0900, Tetsuo Handa wrote:
>>>>>> On 2021/03/24 1:13, Mimi Zohar wrote:
>>>>>>> On Wed, 2021-03-24 at 00:14 +0900, Tetsuo Handa wrote:
>>>>>>>> On 2021/03/23 23:47, Mimi Zohar wrote:
>>>>>>>>> Initially I also questioned making "integrity" an LSM.  Perhaps it's
>>>>>>>>> time to reconsider.   For now, it makes sense to just fix the NULL
>>>>>>>>> pointer dereferencing.
>>>>>>>> Do we think calling panic() as "fix the NULL pointer dereferencing" ?
>>>>>>> Not supplying "integrity" as an "lsm=" option is a user error.  There
>>>>>>> are only two options - allow or deny the caller to proceed.   If the
>>>>>>> user is expecting the integrity subsystem to be properly working,
>>>>>>> returning a NULL and allowing the system to boot (RFC patch version)
>>>>>>> does not make sense.   Better to fail early.
>>>>>> What does the "user" mean? Those who load the vmlinux?
>>>>>> Only the "root" user (so called administrators)?
>>>>>> Any users including other than "root" user?
>>>>>>
>>>>>> If the user means those who load the vmlinux, that user is explicitly asking
>>>>>> for disabling "integrity" for some reason. In that case, it is a bug if
>>>>>> booting with "integrity" disabled is impossible.
>>>>>>
>>>>>> If the user means something other than those who load the vmlinux,
>>>>>> is there a possibility that that user (especially non "root" users) is
>>>>>> allowed to try to use "integrity" ? If processes other than global init
>>>>>> process can try to use "integrity", wouldn't it be a DoS attack vector?
>>>>>> Please explain in the descripotion why calling panic() does not cause
>>>>>> DoS attack vector.
>>>>> User in this case, is anyone rebooting the system and is intentionally
>>>>> changing the default values, dropping the "integrity" option on the
>>>>> boot command line.
>>>> OK. Then, I expect that the system boots instead of calling panic().
>>>> That user is explicitly asking for disabling "integrity" for some reason.
>>> That was actually my intention. The prebuilt kernel that I use for
>>> things has all LSMs enabled, but then I needed to try some workload
>>> with only 1 specific LSM, so I gave a different lsm= argument.
>> IMA/EVM is dependent on "integrity".  Was your intention to also
>> disable IMA and EVM?
> I think, yes... or not sure. I was trying to test a bug that requires
> a different major LSM and all minor LSMs are presumably irrelevant. I
> dropped existing lsm= arg and added something like lsm=apparmor.

This is the legacy case that security= supports. If you specify
security=apparmor you will get all the "minor" LSMs you have compiled
in and the "major" LSM you've specified, AppArmor in this case. This
is exactly the behavior you used to get before lsm= was introduced.

>
>> If so, when disabling "integrity", don't load an
>> IMA policy.
> I don't really know what this means. I guess it simply comes from the
> image? If so, there was no easy way to avoid loading.