findutils-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Findutils-patches] [PATCH] find -prune now makes sure it has valid


From: Colin Watson
Subject: Re: [Findutils-patches] [PATCH] find -prune now makes sure it has valid stat() information
Date: Fri, 8 May 2009 10:22:07 +0100
User-agent: Mutt/1.5.18 (2008-05-17)

On Fri, May 08, 2009 at 09:35:12AM +0100, James Youngman wrote:
> On Thu, May 7, 2009 at 10:14 PM, Colin Watson <address@hidden> 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                                       address@hidden




reply via email to

[Prev in Thread] Current Thread [Next in Thread]