Re: pinfo->fd->flags.visited and NFS

[Ian Schorr <ian.schorr () gmail com>]

Thanks gentlemen.  That makes sense (Guy), and that's more or less
what the problem was (Didier).

Unfortunately it isn't quite as simple as an errant (tree) check
encapsulating the call to me code, but they are problems that are
dependent on the tree being present.  I've found four bugs so far, and
there's at least one more.  Mostly they're problems where tree not
being set causes some variable not to be initialized, and eventually
that causes some problems where offset isn't incremented (sort of like
your second example, but not exactly - the code's explicitly not doing
certain things to increment the offset, not a situation where offset
isn't returned properly).  That leads to problems that eventually
cause packet dissection to abort before it gets to my code - but only
on the first pass dissecting the packet.

Oddly, one bug was triggering an explit ASSERT but Wireshark is
printing nothing to the console.

Anyway, I've fixed all the bugs that are affecting processing of the
NFSv4 replies I've been working with - now I need to work through the
bugs affecting the calls.

Fortunately the bugs aren't my code =)

Thanks,
Ian

On Thu, Jan 14, 2010 at 6:46 PM, didier <dgautheron () magic fr> wrote:
> Hi,
> Le jeudi 14 janvier 2010 à 17:37 +1100, Ian Schorr a écrit :
> > Hi Didier,
> >
> > On Thu, Jan 14, 2010 at 4:54 PM, didier <dgautheron () magic fr> wrote:
> > > Hi,
> > > The whole file is first dissected sequentially with
> > > pinfo->fd->flags.visited set to FALSE.
> > >
> > > The most common error for what you're seeing is that the code is inside
> > > a if (tree) block, with the new packet list tree is null when loading a
> > > file, before it was null only with colors disable.
> > >
> > > You can test if it's the case by setting a filter like 'frame' and
> > > reloading the file with CTRL R.
> >
> > You're right - if I first open the capture, filter (with "frame") and
> > then reload, then looks like the flag is FALSE and the code block is
> > executed.
> >
> > This function itself never tests "if <tree or subtree>", and so far I
> > haven't found a situation where any of its parent functions in the
> > stack do this.  There's so little difference between what NFSv3 and
> > NFSv4 dissecting is doing at this point, it seems unusual.  But I'll
> > keep looking.  Thanks for the tip.
> >
> > I'm still not clear on why your example is a problem though - what's
> > the logic error in doing this test during an "if (tree)" block?
> pinfo->fd->flags.visited is only FALSE the first time and the first time
> tree is null... thus in
> if (tree)
>        if (!pinfo->fd->flags.visited)
>                foo()
>
> foo is never executed.
>
> same with:
>
> fitem = proto_add_uint( ...)
> if (!fitem) return offset
> offset = foo()
> return offset
>
> The bug could be a lot earlier though.
>
> The code you're looking at may not being called at all if a previous
> function return a wrong offset when tree is null, or it could be in the
> rpc code.
>
> Didier
>
>
>
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe