720150 - SPDY Division by Zero [@mozilla::net::SpdySession::HandleSynReply]

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Christoph Diehl [:posidron]]

Attachment: callstack

The complete SYN_REPLY packet:

0000   80 02 00 02 01 00 00 0C 00 00 00 01 00 00 78 BB
0010   DF A2 51 B2 62 60 64 00 02 00 00 00 00 FF FF   


Crash occurs right after:

[...]
186150912[10037b5c0]: 00000000: 00 00 00 01 00 00 78 BB DF A2 51 B2 
186150912[10037b5c0]: SpdySession::HandleSynReply 11a9e2400 SYN_REPLY for 0x1 fin=1


Program received signal EXC_ARITHMETIC, Arithmetic exception.
[Switching to process 15940 thread 0x3303]
0x0000000101544fc2 in mozilla::net::SpdySession::HandleSynReply (self=0x116d3d400) at /Users/cdiehl/Code/Mozilla/mz_spdy/netwerk/protocol/http/SpdySession.cpp:881
881     (self->mFrameDataSize - 6) * 100 / self->mDecompressBufferUsed;


More information can be found in the provided callstack and NSPR log.

Comment 1

[Christoph Diehl [:posidron]]

Attachment: NSPR Log

Comment 2

[Patrick McManus [:mcmanus]]

Christoph is running fuzzing tests - that's not a valid SYN_REPLY. (its too long for the length included)

Christoph, is your code out of date? That gdb output has a line number that does not match the trunk.

In any event, that code does not exit as expected on decompress failed because of the length issue. the compression is fine but truncated up to the packet length (20) which is how decompress buffer used is 0.

I will attach the fix in a minute. I don't see any further implications.

Comment 3

[Patrick McManus [:mcmanus]]

Attachment: patch 0

Comment 4

[Honza Bambas (:mayhemer)]

Comment on attachment 590705 [details] [diff] [review]
patch 0

Review of attachment 590705 [details] [diff] [review]:
-----------------------------------------------------------------

r=honzab

Isn't it better to just not accumulate telemetry in that case?  IMO it doesn't make much sense.

Comment 5

[Patrick McManus [:mcmanus]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4035cbbd550b

> 
> Isn't it better to just not accumulate telemetry in that case?  IMO it
> doesn't make much sense.

I was thinking more along the lines of "compression failed, so report an identity sized ratio" - but your suggestion is better. we'll do that.

Comment 6

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/4035cbbd550b