#9 fix: aide --init misses whole subtrees

closed
None
5
2005-02-20
2004-07-05
Bryan Henderson
No

Here's a fix to a problem wherein aide --init omits files from the
database it creates. I don't know the exact mechanism of the failure,
but it's based on the fact that aide --init refers back to the tree of
files to determine whether the descendants of a directory need to be
included in the database (NODE_ADD_CHILDREN attribute), but
does not in fact build the tree. The code to build the tree explicitly
skips the step in the --init case, because it does not think the tree
will be needed.

This patch approaches the issuing of messages under verbosity
control differently than a recent fix in the same area: It uses error()
instead of printf(). That seems to me the better way to go.

Discussion

  • Logged In: YES
    user_id=330646

    No patch is attached, and the description is missing a
    reproducable example. Please supply both if this is still a
    problem with the current CVS version.

     
    • assigned_to: nobody --> rvdb
    • status: open --> pending
     
    • status: pending --> open
     
  • Logged In: YES
    user_id=29357

    I've had trouble with Sourceforge in regards to uploading files. I don't
    remember what happened, but I'm trying again.

    I don't have a reproducible example. As I was debugging, I tried to narrow
    the problem down to a minimum test case (it failed consistently when run
    against my actual filesystem), but I couldn't. Bugs that cause programs to
    access uninitialized memory are like that.

    When I discovered that the program tries to access a data structure it never
    built, I just fixed that apparent problem, and didn't bother to figure out
    exactly how that bug led to the symptoms I saw.

    About it still being a problem: It's not a problem for me because I'm using
    the patch. I don't need anything more. But if this code is the same in Aide
    today as it was in July, then I'm sure it's still a problem for others.

     
  • patch file

     
    Attachments
  • Logged In: YES
    user_id=330646

    I just did a test on my home directory (110000 files upto 15
    directories deep). Aide does not miss a single file.

    Since there is no clear test case, it requires a throrough
    check of the code to determine if this patch is indeed needed.

     
  • Logged In: YES
    user_id=62241

    The comment from 'Date: 2004-11-13 19:56' is really weird.
    It doesn't become clear what the reporter debugged. Usually
    you discover misbehaviour at run-time and find yourself
    unable to reproduce it while being in the debugger. In
    either case, you used a specific config file as a test case,
    and that one has not been submitted together with this
    report. The last paragraph of the comment is not clear either:

    > About it still being a problem: It's not a problem for me
    > because I'm using the patch.

    So, if you don't use the patch, _when_ is it a problem? ;)

     
  • Logged In: YES
    user_id=29357

    Sorry I wasn't clear. Let me add some more words.

    The first phase of debugging I did was trying to narrow the
    failing test case down to the particular element that caused
    the failure. I tried cutting stuff out of my config file and
    retrying until I could figure out exactly which lines or
    sequences caused Aide to choke. I was unsuccessful. I
    could not find any pattern in what versions of the config file
    caused a failure and what versions didn't. The smallest config
    file I could make that demonstrated the problem was 100
    lines, and various 100 line variations demonstrated the
    problem. The problem moved around, too. Different parts of
    the filesystem got skipped with different variations of the
    config file.

    There is virtually no chance that someone else can reproduce
    the problem simply by using one of the config files I did. The
    problem is apparently an uninitialized memory problem, which
    means its manifestation depends on the exact contents of
    the filesystem, the C library, and other details we can't even
    think of.

    Having failed at isolating the problem from the outside, I went
    to Phase II and traced through the code as it failed (using
    printfs). This led me to accesses to memory that contained
    bogus information. Studying the code, it appeared to me that
    one part of the code chose not to set up a data structure
    because it didn't think it would be needed, then another part
    of the code accessed it. A simple change caused that first
    part not to make the bad assumption, and after I did that my
    problem went away. I took that as confirmation that what I
    saw really was a bug and had caused the failures.

    I then totally lost interest in producing a portable test case
    that would demonstrate the problem (which looks like maybe
    a 4 hour job). Or why it usually doesn't manifest. I figured
    someone who knew the code could look at the area I changed
    and say, "yep, that doesn't work" and debugging would be
    moot.

    So in summary: I could reproduce the problem easily before
    the patch, but I doubt there is anything I could provide that
    would help someone else do so. But someone else could
    definitely inspect the code and see if the program is really
    making a bad assumption where I claim it is.

     
  • Logged In: YES
    user_id=62241

    I have not enough insight into the [admittely not easy to
    follow] code to read it fluently and debug potential
    problems in source code only. Although I see a
    NODE_ADD_CHILDREN comparison in check_rxtree, it seems
    db_readline_disk does add children already based on
    check_rxtree return values before the db lines are written out.

     
  • Logged In: YES
    user_id=330646

    For the second time now, I have looked through the code to
    see what this patch does. In effect, it causes conf->tree to
    be populated when action is DO_INIT. conf->tree is always
    initialized by gen_tree() in aide.c, so the claim of unused
    memory does not hold for it.

    populate_tree(conf->tree) is called from aide.c. conf->tree
    is then used in report_tree() which reports on changes
    between the database and the filesystem. This makes
    absolutely no sense with DO_INIT since there cannot be any
    changes to report.

    I believe this patch to be bogus.

     
  • Logged In: YES
    user_id=62241

    The call of

    conf->tree=gen_tree(conf->selrxlst,conf->negrxlst,conf->equrxlst);

    in aide.c in line 500 is duplicate, btw. First one in line
    425 is enough.

     
    • status: open --> closed
     
  • Logged In: YES
    user_id=330646

    I removed the extra gen_tree() in CVS. Closing this bug now,
    because the rest of this patch is not needed.

     
  • Logged In: NO

    >conf->tree is always
    >initialized by gen_tree() in aide.c, so the claim of unused
    >memory does not hold for it.

    gen_tree() initializes the tree, but doesn't put anything in it. It's the
    contents of the nodes that I found not to have been set in my tracing.

    >populate_tree(conf->tree) is called from aide.c. conf->tree
    >is then used in report_tree() ...

    It's also used in db_readline_disk(), which I think is where the incorrect
    value of NODE_ADD_CHILDREN makes a difference for me.