On Fri, May 08, 2009 at 09:35:12AM +0100, James Youngman wrote: > On Thu, May 7, 2009 at 10:14 PM, Colin Watson <[email protected]> wrote: > > including a test > > I can't tell you how grateful I am that you actually included a test > :) Does the test reliably detect a problem if the bugfix is not > present?
Sadly not. While in the original case that caused me to notice this the stat buffer was left containing information for the previous inode that was statted, when I attempted to construct a minimal test case for it it seemed to just contain random junk off the stack. I didn't manage to identify anything really reliable. > Is the "oldfind" binary affected too (all tests are routinely run > against both binaries at all valid values of find's -O option) ? I don't think so. oldfind zeroes stat_buf.st_mode unconditionally in process_path, which should avoid this particular bug. In case it's interesting, I posted a further analysis to our bug report this morning: In case anyone is curious, the original bug was introduced in http://git.savannah.gnu.org/cgit/findutils.git/commit/?id=756b47b1609ecb0890f04302afba4c74779e263f due to https://savannah.gnu.org/bugs/?15531; I believe it was exposed recently due to http://git.savannah.gnu.org/cgit/findutils.git/commit/?id=cecfe3ef8e4db51d97f0cbed864f47893adc354e, which fixed a similar uninitialised stat data bug, but unfortunately the previous buggy code had the effect of catching the exact same property of the uninitialised stat data in question that ended up causing problems here ... > > that you may be able to get to demonstrate the problem. Whether the > > test actually fails for you is unfortunately not certain, since it's > > an undefined-data problem; you should at least be able to see the > > bug manually by breaking on pred_prune. > > It's possible there's something we can do to make a failure more > likely or more reproducible if similar problems occur in the future, > for example setting variables to 'invalid' values before calling > fts_read. Whatever we do we will need to make sure it does not break > fts. If you wanted to use invalid values, you could do so with reasonable assurance of not breaking fts in the 'if (ent->fts_info == FTS_NSOK || ent->fts_info == FTS_NS)' case of consider_visiting (find/ftsfind.c:474 or so in the rel-4-4-fixes branch), after calling fts_read. I considered zeroing stat_buf.st_mode there to match oldfind, but I was a bit concerned that that might hide other bugs (and this was before I figured out how the need_stat system works, anyway). Perhaps all stat_buf.st_mode accesses in predicates could be routed through a macro that checks for 0xDEADBEEF or similar? Thanks, -- Colin Watson [[email protected]]
