The Valgrind Project releases version 3.4.0 [LWN.net]

The Valgrind Project releases version 3.4.0

Posted Jan 15, 2009 9:41 UTC (Thu) by Trou.fr (subscriber, #26289)
Parent article: The Valgrind Project releases version 3.4.0

"Those who maintain large projects could likely improve their code by running Valgrind and correcting any issues that it finds."

Never blinding trust the output of an automated tool ! You should always analyse the code before "fixing" the problem. The Debian OpenSSL fiasco originated from a commented line, to fix a "uninitialized memory" problem.

to post comments

The Valgrind Project releases version 3.4.0

Posted Jan 15, 2009 12:38 UTC (Thu) by tialaramex (subscriber, #21167) [Link] (3 responses)

Fixing the bug that was /reported/ would not have caused the Debian OpenSSL fiasco.

There were two identical lines of code in that OpenSSL source file, one of them triggered a report from memory access checkers, and was commented, "fixing" it was unnecessary but harmless.

The other was essential to the correct operation of the code, it didn't trigger reports from checkers and it didn't have a comment. But a Debian developer "fixed" that line anyway, eliminating the bulk of the PRNG.

If your code accesses uninitialised memory there will be a correct fix for that. If it accesses recently free'd memory, there will again be a correct fix for that. Obviously an improper fix may make things worse, but it's only fair to assume that "correcting any issues" doesn't mean "just modify the code more or less randomly until the error goes away".

The Valgrind Project releases version 3.4.0

Posted Jan 15, 2009 14:02 UTC (Thu) by BenHutchings (subscriber, #37955) [Link] (2 responses)

According to Luciano Bello's talk at 25C3, valgrind warned about both lines. One function was always using uninitialised memory. The other was using a caller-provided buffer supposed to provide entropy and one of its callers was passing in uninitialised memory.

The Valgrind Project releases version 3.4.0

Posted Jan 16, 2009 8:06 UTC (Fri) by liljencrantz (guest, #28458) [Link]

Which is why the OP said NOT to "just modify the code more or less randomly until the error goes away".

The Valgrind Project releases version 3.4.0

Posted Jan 28, 2009 1:41 UTC (Wed) by bluefoxicy (guest, #25366) [Link]

One of its callers was passing uninitialized memory... uninitialized memory for write is harmless, and otherwise you need to fix its caller!

The Valgrind Project releases version 3.4.0

Posted Jan 15, 2009 15:56 UTC (Thu) by iabervon (subscriber, #722) [Link]

You should always analyze the code before making any change. Valgrind was entirely correct that uninitialized memory was going into the entropy pool, and that fixing this might be good. But taking some code that uses uninitialized memory passed in an argument and just not using that argument is not a sensible solution.

The Valgrind Project releases version 3.4.0

Posted Jan 23, 2009 8:54 UTC (Fri) by renox (guest, #23785) [Link]

More a process issue than a tool issue: changing things in a critical software like ssh without pushing the change upstream immediately is a bad idea whatever the change is.