On Thu, Feb 25, 2010 at 10:56 AM, Mikael Pettersson <mikpe@xxxxxxxx> wrote: > 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. Would barrier() (which is a simple memory clobber) after the memset work? -- Brian Gerst -- 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
