smack: Possible NULL pointer deref in cred_free hook.

[Paul Moore]

On Wed, Feb 14, 2024 at 7:13 PM Casey Schaufler <casey at schaufler-ca.com> wrote:
> On 2/14/2024 2:15 PM, Tetsuo Handa wrote:
> > On 2024/02/15 3:55, Paul Moore wrote:
> >>> Ah, but it turns out that the only LSM that can fail in _cred_prepare()
> >>> is Smack. Even if smack_cred_prepare() fails it will have called
> >>> init_task_smack(), so there isn't *currently* a problem. Should another
> >>> LSM have the possibility of failing in whatever_cred_prepare() this
> >>> could be an issue.
> >> Let's make sure we fix this, even if it isn't a problem with the
> >> current code, it is very possible it could become a problem at some
> >> point in the future and I don't want to see us get surprised by this
> >> then.
> >>
> > Anyone can built-in an out-of-tree LSM where whatever_cred_prepare() fails.
> > An in-tree code that fails if an out-of-tree code (possibly BPF based LSM)
> > is added should be considered as a problem with the current code.
>
> Agreed. By the way, this isn't just a Smack problem.

I've tried to make this clear on previous issues, but let me say it
again: I don't care what individual LSMs are affected, a bug is a bug
and we need to fix it.

> You get what looks
> like the same failure on an SELinux system if security_prepare_creds() fails
> using the suggested "fault injection". It appears that any failure in
> security_prepare_creds() has the potential to be fatal.

Perhaps I didn't look at the original problem closely enough, but I
believe this should only be an issue with LSMs that register a
cred_free hook that assumes a valid LSM specific credential
initialization.  While SELinux registers a cred_prepare hook, it does
not register a cred_free hook.  Or am I missing something?

Looking quickly I suspect this affects Smack and AppArmor.  While
Landlock registers a cred_free hook, it looks like it should properly
handle being called without a cred_prepare hook first being executed.
Of course Mickaël is the one who should confirm this.

-- 
paul-moore.com