smack: Possible NULL pointer deref in cred_free hook.

[Paul Moore]

On Wed, Feb 14, 2024 at 12:11 PM Casey Schaufler <casey at schaufler-ca.com> wrote:
> On 2/7/2024 10:53 AM, Casey Schaufler wrote:
> > On 2/6/2024 6:54 PM, Tetsuo Handa wrote:
> >> On 2024/02/07 10:39, Casey Schaufler wrote:
> >>> On 2/6/2024 6:31 AM, Tetsuo Handa wrote:
> >>>> Hello, Casey.
> >>>>
> >>>> I confirmed using fault injection shown below that smack_cred_free() is not
> >>>> prepared for being called without successful smack_cred_prepare().
> >>> The failure cases for smack_cred_prepare() result from memory allocation
> >>> failures. Since init_task_smack() is called before either of the potential
> >>> memory allocations the state of the cred will be safe for smack_cred_free().
> >>> The fault you've described here removes the init_task_smack(), which will
> >>> always succeed, and which is sufficient to prevent the smack_cred_free()
> >>> failure below. Are you suggesting that there is a case where a cred will
> >>> be freed without ever having been "prepared"?
> >> Yes. If smack_cred_prepare() is not the first entry of the cred_prepare list
> >> and the first entry of the cred_prepare list failed, smack_cred_prepare()
> >> will not be called (and therefore init_task_smack() will not be called).
>
> 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.

-- 
paul-moore.com