Bug 1555108

Created attachment 1407755 [details]
Patch to zero the context (which key information) when finished with them.

 void
 AES_DestroyContext(AESContext *cx, PRBool freeit)
 {
+    void *mem = cx->mem;
     if (cx->worker_cx && cx->destroy) {
         (*cx->destroy)(cx->worker_cx, PR_TRUE);
         cx->worker_cx = NULL;
         cx->destroy = NULL;
     }
+    PORT_Memset(cx, 0, sizeof(AESContext));
     if (freeit) {
-        PORT_Free(cx->mem);
+        PORT_Free(mem);
     }
 }


The code seems correct, and I agree it clears the context structures.

But I have two suggestions/questions:

Q1: The code doesn't clear the contents of cx->mem. Should it? But that would require to know the size of that block. Is it known?

Q2: The helper variable "mem" seems unnecessary. Why not simply place the memset call as the very last line in this function?

I explained this to kai verbally, but it's worth documenting.

cx is contained in mem:

    mem->  +---------+
           |         |
           .         .
           .         .
           .         .
           |         |
    cx->   +---------+
           |         |
           |         |
           .         .
           .         .
           | cx->mem |
           .         .
           .         .
           |         |
end of cx  +---------+
           |         |
           .         .
           .         .
           |         |
end of mem +---------+

This is done for memory alignment purposes. So space is allocated sizeof(cx) + sizeof(alignment) and assigned to mem. cx is set to the aligned pointer within mem, and cx->mem is set to mem. cx is used as the active context (the extra space in mem is not touched. This was all added by mozilla in the last year to speed up AES performance.

So the order in the patch is important.
1. We need to save mem before we clear the cx (because cx->mem would be cleared and we can't reference it later).
2. We have to clear the cx before we free mem, (because cx is embedded in the mem block and freeing mem frees cx)
3. Finally with mem saved and cx cleared we can safely free mem.