smack: Possible NULL pointer deref in cred_free hook.

[Serge E. Hallyn]

On Thu, Feb 15, 2024 at 10:32:59PM -0500, Paul Moore wrote:
> 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.

Sure does look that way...