Roel Kluin writes: > Due to optimization A call to memset() may be removed as a dead store when > the buffer is not used after its value is overwritten. > > Signed-off-by: Roel Kluin <roel.kluin@xxxxxxxxx> > --- > see http://cwe.mitre.org/data/slices/2000.html#14 > > checkpatch.pl, compile and sparse tested. Comments? > > diff --git a/crypto/sha1_generic.c b/crypto/sha1_generic.c > index 0416091..86de0da 100644 > --- a/crypto/sha1_generic.c > +++ b/crypto/sha1_generic.c > @@ -49,8 +49,8 @@ static int sha1_update(struct shash_desc *desc, const u8 *data, > src = data; > > if ((partial + len) > 63) { > - u32 temp[SHA_WORKSPACE_WORDS]; > - > + u32 *temp = kzalloc(SHA_WORKSPACE_WORDS * sizeof(u32), > + GFP_KERNEL); > if (partial) { > done = -partial; > memcpy(sctx->buffer + partial, data, done + 64); > @@ -64,6 +64,7 @@ static int sha1_update(struct shash_desc *desc, const u8 *data, > } while (done + 63 < len); > > memset(temp, 0, sizeof(temp)); > + kfree(temp); > partial = 0; > } > memcpy(sctx->buffer + partial, src, len - done); At best this might solve the issue right now, but it's not future-proof by any margin. One problem is that just like the lifetimes of auto variables are known to the compiler, allowing dead store elimination (DSE) on them, there is development going on to make malloc() and free() known to the compiler. I don't think it's complete yet, but once free() is known, the sequence "memset(p, 0, n); free(p);" will obviously be DSE:d just like in the current case with the auto variable. And as soon as gcc can optimize malloc() and free(), you can be sure that some eager kernel hacker will mark the kernel's allocators accordingly, and then we're back to square one. I fear that the only portable (across compiler versions) and safe solution is to invoke an assembly-coded dummy function with prototype void use(void *p); and rewrite the code above as { u32 temp[...]; ... memset(temp, 0, sizeof temp); use(temp); } This forces the compiler to consider the buffer live after the memset, so the memset cannot be eliminated. The reason the use() function needs to be in assembly code is that with link-time optimizations soon commonplace (LTO in gcc-4.5), a compiler can possibly discover that even an out-of-line function void use(void *p) { } doesn't in fact use *p, which then enables (in theory) the preceeding memset() to be DSE:d. -- 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
