Michael Mann
changed
bug 9248
| What |
Removed |
Added |
| Status |
CONFIRMED
|
RESOLVED
|
| Resolution |
---
|
FIXED
|
Comment # 9
on bug 9248
from Michael Mann
(In reply to comment #8)
> Ideally I think we
> wouldn't have to check at all, and just trust that we eventually run off the
> end of the TVB and throw an exception if the loop runs too long. I don't
> know if your fixes to offset incrementation make that possible or not.
I tried adding just the offset incrementation and removing your
length_remaining check and it was still stuck in the loop (per "map loop"
problem mentioned in comment #5)
> I'm
> coming to the conclusion that 99% of the length_remaining checks are
> unnecessary, probably the original author thought it was unsafe to run past
> the end of the TVB.
I believe the length_remaining checks prevent one "bogus" length value from not
screwing up the entire packet. I've been focusing on packet 36 (seems to be
first instance of near-infinite loop), and with the length_remaining checks
removed, "good" dissection would cease (too early IMO) when the that loop is
hit because a bounds check would be thrown.
With the recursiveness of dissect_openwire_type(), the key may be finding the
1% of length_remaining checks that really are necessary.
> Whether this stays open depends on how much time you
> want to spend on it, and whether the original author can provide any more
> guidance. Your current patch is good enough for the amount of time I have
> left this week :)
Committed a fix to r52463 and scheduled for backporting. Closing because
"cleaning up dissector" shouldn't really be part of this bug.
You are receiving this mail because:
- You are watching all bug changes.
