smack: Possible NULL pointer deref in cred_free hook.

[Paul Moore]

On Thu, Feb 15, 2024 at 7:22 PM Casey Schaufler <casey at schaufler-ca.com> wrote:
> On 2/15/2024 3:38 PM, Paul Moore wrote:
> > 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.
>
> Yes, I understand that.
>
> >> 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?
>
> Yes, you're missing something. If security_prepare_creds() fails prepare_creds()
> will fail, and the system will lurch to a halt because it can't create a new
> cred. The cred_free hook is a red herring.

Okay, my apologies, I thought the issue was due to one of the LSMs
failing their cred_prepare hook and causing security_cred_free() to be
called for LSMs that hadn't successfully cred_prepare()'d the new
creds.

However, if I'm understanding you correctly, the issue is that a
failed security_prepare_creds() call can cause both prepare_creds()
and prepare_kernel_cred() to fail, yes?  If that is the case, can
someone explain to me why this is a problem?  Both prepare_creds() and
prepare_kernel_cred() can fail in ways unrelated to the LSM and thus
callers must be prepared to handle a failure in both prepare_cred()
functions.

-- 
paul-moore.com