On 07/02/2022 22:08, Brijesh Singh wrote:

On 2/7/22 1:09 PM, Dov Murik wrote:

quoted

On 07/02/2022 18:23, Brijesh Singh wrote:

quoted

On 2/7/22 2:52 AM, Borislav Petkov wrote:

quoted

Those are allocated on stack, why are you clearing them?

Yep, no need to explicitly clear it. I'll take it out in next rev.

Wait, this is key material generated by PSP and passed to userspace.
Why leave copies of it floating around kernel memory?  I thought that's
the whole reason for these memzero_explicit() calls (maybe add a
comment?).

Ah, now I remember I added the memzero_explicit() to address your review
feedback :) In that patch version, we were using the kmalloc() to store
the response data; since then, we switched to stack. We will leak the
key outside when the stack is converted private-> shared; I don't know
if any of these are going to happen. I can add a comment and keep the
memzero_explicit() call.

Just to be clear, I didn't mean necessarily "leak the key to the
untrusted host" (even if a page is converted back from private to
shared, it is encrypted, so host can't read its contents).  But even
*inside* the guest, when dealing with sensitive data like keys, we
should minimize the amount of copies that float around (I assume this is
the reason for most of the uses of memzero_explicit() in the kernel).

-Dov

Boris, let me know if you are okay with it?

quoted

As an example, in arch/x86/crypto/aesni-intel_glue.c there are two calls
to memzero_explicit(), both on stack variables; the only reason for
these calls (as I understand it) is to avoid some future possible leak
of this sensitive data (keys, cipher context, etc.).  I'm sure there are
other examples in the kernel code.