* Larry H. <research@xxxxxxxxxxxxxx> wrote: > This patch deploys the use of the PG_sensitive page allocator flag > within the CryptoAPI subsystem, in all relevant locations where > algorithm and key contexts are allocated. > > Some calls to memset for zeroing the buffers have been removed to > avoid duplication of the sanitizing process, since this is already > taken care of by the allocator during page freeing. > > The only noticeable impact on performance might be in the > blkcipher modifications, although this is likely negligible and > balanced with the security benefits of this patch in the long > term. I think there's a rather significant omission here: there's no discussion about on-kernel-stack information leaking out. If a thread that does a crypto call happens to leave sensitive on-stack data (this can happen easily as stack variables are not cleared on return), or if a future variant or modification of a crypto algorithm leaves such information around - then there's nothing that keeps that data from potentially leaking out. This is not academic and it happens all the time: only look at various crash dumps on lkml. For example this crash shows such leaked information: [ 96.138788] [<ffffffff810ab62e>] perf_counter_exit_task+0x10e/0x3f3 [ 96.145464] [<ffffffff8104cf46>] do_exit+0x2e7/0x722 [ 96.150837] [<ffffffff810630cf>] ? up_read+0x9/0xb [ 96.156036] [<ffffffff8151cc0b>] ? do_page_fault+0x27d/0x2a5 [ 96.162141] [<ffffffff8104d3f4>] do_group_exit+0x73/0xa0 [ 96.167860] [<ffffffff8104d433>] sys_exit_group+0x12/0x16 [ 96.173665] [<ffffffff8100bb2b>] system_call_fastpath+0x16/0x1b The 'ffffffff8151cc0b' 64-bit word is actually a leftover from a previous system context. ( And this is at the bottom of the stack that gets cleared all the time - the top of the kernel stack is a lot more more persistent in practice and crypto calls tend to have a healthy stack footprint. ) So IMO the GFP_SENSITIVE facility (beyond being misnomer - it should be something like GFP_NON_PERSISTENT instead) actually results in _worse_ security in the end: because people (and organizations) 'think' that their keys are safe against information leaks via this space, while they are not. The kernel stack can be freed, be reused by something else partially and then written out to disk (say as part of hibernation) where it's recoverable from the disk image. So this whole facility probably only makes sense if all kernel stacks that handle sensitive data are zeroed on free. But i havent seen any kernel thread stack clearing functionality in this patch-set - is it an intentional omission? (or have i missed some aspect of the patch-set) Also, there's no discussion about long-lived threads keeping sensitive information in there kernel stack indefinitely. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
