1119843 - Uninitialised value use in sctp_med_chunk_output

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect)

Description

[Julian Seward [:jseward]]

Attachment: Valgrind complainery, plus a bit of context

test path = dom/media/tests/mochitest/ipc/test_ipc.html

I feel like I've seen something similar to this recently, but searching
bugzilla turns up nothing.

I tried several times to get an origin for the uninitialised value,
but failed.  It seems that the use of --track-origins=yes slows things
down so much that the code takes different paths.  With origin tracking
it produces a bunch of complaining along the lines of

1335473920[6bef1d70]: [|WebrtcVideoSessionConduit] CodecStatistics.cpp:119: Video error duration: 63 ms

and no complaint from Valgrind.  Without origin tracking, CodecStatistics.cpp
still complains, but later on.  So I wonder if there is some timeouts that
can be adjusted?  A bit of a bummer, since knowing the origin is pretty much
essential to make sense of this.  Any ideas?

Comment 1

[Julian Seward [:jseward]]

netwerk/sctp/src/netinet/sctp_output.c:8611 (the location of the
complaint) is the condition in

    /* Do clear IP_DF ? */
    if (chk->flags & CHUNK_FLAGS_FRAGMENT_OK) {  <--------------
      no_fragmentflg = 0;
    }

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

CodecStatistics.cpp:119: Video error duration: 63 ms

That just says "it took a really long time to recover from a video error".  Informative debug.  The timeouts are hard-coded, see CodecStatistics IIRC.

The uninitialized variable would be an upstream bug.  Looks to be minor.  Michael?

Comment 3

[Michael Tüxen]

I'm not sure which version of the SCTP sources were used, but I think this issue was fixed upstream in
https://code.google.com/p/sctp-refimpl/source/detail?r=9043

Can you retest with SCTP sources including that fix?

Best regards
Michael

Comment 4

[Julian Seward [:jseward]]

(In reply to Michael Tüxen from comment #3)
> I'm not sure which version of the SCTP sources were used, but I think this
> issue was fixed upstream in
> https://code.google.com/p/sctp-refimpl/source/detail?r=9043

As far as I can see, our tree (netwerk/sctp/src/netinet, that is)
does not contain that fix.

That said, I cannot reproduce this Valgrind error.  I am inclined to close
this as invalid until such time as I can reproduce it.  Maybe it already
got fixed some other way.

Comment 5

[Julian Seward [:jseward]]

Comment 4 is wrong.  I can reproduce this error, but need to pass
--e10s to ./mach mochitests, and the correct test path is 
dom/media/tests/mochitest/test_dataChannel_basicAudio.html

I also verified that 
> https://code.google.com/p/sctp-refimpl/source/detail?r=9043

fixes the problem.  Can we pull this fix into our tree?  If so,
how should this be done?

Comment 6

[Michael Tüxen]

Sure. You can pull in the changes to
/trunk/KERN/usrsctp/usrsctplib/netinet/sctp_input.c
/trunk/KERN/usrsctp/usrsctplib/netinet/sctp_output.c
from
https://code.google.com/p/sctp-refimpl/source/detail?r=9045
into your source tree. Or better, update to the latest version since
there have been fixed more issues.
Randell has a script to do this...

Best regards
Michael

Comment 7

[Julian Seward [:jseward]]

Randall, shall I just pull this patch in, or do you want to do
something more heavyweight -- resync with upstream?

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

I think we should consider updating; I had an update in the wings anyways.  Let me check it

Comment 9

[Julian Seward [:jseward]]

Randell, ping?

Comment 10

[Julian Seward [:jseward]]

Closing as fixed, because the fix for blocker bug 1139020 also fixes this.