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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Tue, 7 Apr 2020 16:50:11 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     David Howells <dhowells@...hat.com>
Cc:     Joe Perches <joe@...ches.com>, Waiman Long <longman@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Linux-MM <linux-mm@...ck.org>, keyrings@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Matthew Wilcox <willy@...radead.org>,
        David Rientjes <rientjes@...gle.com>, law@...hat.com
Subject: Re: [PATCH v2] mm: Add kvfree_sensitive() for freeing sensitive data objects

On Tue, Apr 7, 2020 at 3:54 PM David Howells <dhowells@...hat.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

Powered by blists - more mailing lists