Re: [PATCH 4/5] slab: Set freed variables to NULL by default

[Jann Horn <jannh@google.com>]

On Fri, Mar 21, 2025 at 9:41 PM Kees Cook <kees@kernel.org> wrote:
> To defang a subset of "dangling pointer" use-after-free flaws[1], take the
> address of any lvalues passed to kfree() and set them to NULL after
> freeing.
>
> To do this manually, kfree_and_null() (and the "sensitive" variant)
> are introduced.

Unless callers of kfree() are allowed to rely on this behavior, we
might want to have an option to use a poison value instead of NULL for
this in debug builds.


Hmm... in theory, we could have an object that points to its own type, like so:

    struct foo {
     struct foo *ptr;
    };

And if that was initialized like so:

    struct foo *obj = kmalloc(sizeof(struct foo));
    obj->ptr = obj;

Then the following is currently fine, but would turn into UAF with this patch:

    kfree(obj->ptr);

That is a fairly contrived example; but maybe it would make sense to
reorder the NULL assignment and the actual freeing to avoid this
particular case?

Another pattern that is theoretically currently fine but would be
problematic after this would be if code continues to access a pointer
after the pointed-to object has been freed, but just to check for
NULL-ness to infer something about the state of the object containing
the pointer, or to check pointer identity, or something like that. But
I guess that's probably not very common? Something like:

    if (ptr) {
        mutex_lock(&some_lock);
        ...
        kfree(ptr);
    }
    ...
    if (ptr) {
        ...
        mutex_unlock(&some_lock);
    }

But from scrolling through the kernel sources and glancing at a few
hundred kfree() calls (not a lot compared to the ~40000 callsites we
have), I haven't actually found any obvious instances like that. There
is KMSAN test code that intentionally does a UAF, which might need to
be adjusted to not hit a NULL deref instead.
(And just an irrelevant sidenote: snd_gf1_dma_interrupt() has
commented-out debugging code that would UAF the "block" variable if it
was not commented out.)