[Top][All Lists]
[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