[Toybox] [PATCH] fix use of uninitialized value in ls

Rob Landley rob at landley.net
Fri Dec 11 23:35:57 PST 2015


On Fri, Dec 11, 2015 at 5:26 PM, enh <enh at google.com> wrote:
> actually found by cferris@ *running* valgrind, but ykwim.

--- a/toys/posix/ls.c
+++ b/toys/posix/ls.c
@@ -299,6 +299,7 @@ static void listfiles(int dirfd, struct dirtree *indir)
   }

   memset(totals, 0, sizeof(totals));
+  memset(len, 0, sizeof(len));

Um, doesn't entrylen() set the len fields? (It uses len as output only?)

The entrylen() code is a bunch of conditional assignments, but the
same conditionals should be on the users of those fields? Let's see,
use of len on 352 is after entrylen() on 350, 379-384 are after
entrylen() on 378, 413 and 421 are after 408, and final user on 498 is
also after 408. So yes, entrylen() is always called to initialize
len() before len is used.

All those users except line 352 are just *len, I.E. len[0], which is
unconditionally set by entrylen(). I suspect what valgrind is
complaining about is that the conditionally set fields (ala len[1] on
FLAG_i) are unconditionally added to the relevant totals[] column
(makes the loop simple), but the resulting totals column is only used
under the same flag tests that entrylen() sets. So when those flags
aren't set, we do add uninitialized data to a column which is then
never used. The uninitialized data is used in a write-only manner.

*shrug* I can add the memset if it makes people feel better about
valgrind, but it shouldn't change the behavior of the code in any way
that I can see?

Rob

 1449905757.0


More information about the Toybox mailing list