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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAG48ez2-stu91bgZawx_AaypEuMnc5=nJQUiDzUCb+CbsJS6kw@mail.gmail.com>
Date: Sat, 22 Mar 2025 02:50:15 +0100
From: Jann Horn <jannh@...gle.com>
To: Kees Cook <kees@...nel.org>
Cc: Vlastimil Babka <vbabka@...e.cz>, Christoph Lameter <cl@...ux.com>, Pekka Enberg <penberg@...nel.org>, 
	David Rientjes <rientjes@...gle.com>, Joonsoo Kim <iamjoonsoo.kim@....com>, 
	Andrew Morton <akpm@...ux-foundation.org>, Roman Gushchin <roman.gushchin@...ux.dev>, 
	Hyeonggon Yoo <42.hyeyoo@...il.com>, linux-mm@...ck.org, Miguel Ojeda <ojeda@...nel.org>, 
	Nathan Chancellor <nathan@...nel.org>, Marco Elver <elver@...gle.com>, 
	Nick Desaulniers <ndesaulniers@...gle.com>, Przemek Kitszel <przemyslaw.kitszel@...el.com>, 
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH 4/5] slab: Set freed variables to NULL by default

On Fri, Mar 21, 2025 at 9:41 PM Kees Cook <kees@...nel.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.)