Re: [PATCH v2] mm: Add kvfree_sensitive() for freeing sensitive data objects

[Linus Torvalds <torvalds@linux-foundation.org>]

On Tue, Apr 7, 2020 at 3:54 PM David Howells <dhowells@redhat.com> wrote:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94527
>
> With regard to this, I've got back "not sure what Linus was talking about WRT
> DSE, if he's got examples he could pass along, they'd be appreciated"

I'll do that. We have real examples in the kernel, although they
probably aren't all that _important_.

I don't see that comment on the bugzilla, but I'll put the stupid
example in there too.

One such example would be kernel/async.c: async_run_entry_fn(), where we have

        /* 2) remove self from the pending queues */
        spin_lock_irqsave(&async_lock, flags);
        list_del_init(&entry->domain_list);
        list_del_init(&entry->global_list);

        /* 3) free the entry */
        kfree(entry);
        atomic_dec(&entry_count);

and while it's good form to do "list_del_init()" on those fields in
entry, the fact that we then do "kfree(entry)" right afterwards means
that the stores that re-initialize the entry list are dead.

So _if_ we had some way to tell the compiler that "hey, kfree(ptr)
kills the lifetime of that object", the compiler could eliminate the
dead stores.

I think that dead store elimination is perhaps less important than if
the compiler could warn about us stupidly using the dead storage
afterwards, but I mention it as a "it can actually matter for code
generation" example too.

Now, the above is a particularly stupid example, because if we cared,
we could just turn the "list_del_init()" into a plain "list_del()",
and simply not do the unnecessary re-initialization of the list entry
after removing it.

But I picked a stupid example because it's easy to understand.

Less stupidly, we sometimes have "cleanup" functions that get rid of
things, and are called before you free the underlying storage.

And there, the cleanup function might be used in general, and not only
just before freeing. So the re-initialization could make sense in that
context, but might again be just dead stores for the actual final
freeing case.

Is this a big deal? No it's not. But it's not really any different
from the dead store elimination that gcc already does for local
variables on stack.

          Linus