792068 - WebRTC crash [@sctp_getopt]

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect)

Description

[Christoph Diehl [:posidron]]

Attachment: testcase

!!! Real PeerConnection constructor called OMG !!!

!!! {8b584e18-86b7-4449-bc22-0e93235ba802} : calling initialize
!!! Queue for {8b584e18-86b7-4449-bc22-0e93235ba802} is currently: []
!!! Queue for {8b584e18-86b7-4449-bc22-0e93235ba802} is currently: []
!!! mozPeerConnection constructor called [object Window @ 0x144746b80 (native @ 0x10426ed00)]
!!! {8b584e18-86b7-4449-bc22-0e93235ba802} : setRemoteDescription called
!!! {8b584e18-86b7-4449-bc22-0e93235ba802} : queued setRemoteDescription
!!! Queue for {8b584e18-86b7-4449-bc22-0e93235ba802} is currently: [setRemoteDescription,]
!!! {8b584e18-86b7-4449-bc22-0e93235ba802} : setRemoteDescription returned
!!! {8b584e18-86b7-4449-bc22-0e93235ba802} : listen() called
PeerConnectionImpl::Listen()

Comment 1

[Christoph Diehl [:posidron]]

Attachment: callstack

Comment 2

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

32/64 bit type-punning problem (casting a socklen_t * into a size_t *)
user_socket.c 2282

Comment 3

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

Attachment: Change cast

Comment 4

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

Comment on attachment 662262 [details] [diff] [review]
Change cast

I'll fix the tabs...

Comment 5

[Eric Rescorla (:ekr)]

Comment on attachment 662262 [details] [diff] [review]
Change cast

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

::: netwerk/sctp/src/user_socket.c
@@ +2278,5 @@
>  			errno = EINVAL;
>  			return (-1);
>  		}
>  	case IPPROTO_SCTP:
> +    size_t len = *option_len; /* sizeof(size_t) !+ sizeof(socklen_t) (on 64 bit) */

Do you really want to do a mid-function variable declaration in C? I tend to move it to the top, even though it is now legal.

Comment 6

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

I actually wrapped it in {}'s after I uploaded the patch out of paranoia.  Also, I noted it's an in-out parameter.  This will get fixed by upstream, but until then I'll take this patch.  (i'll check the SCTP repo first though)

Comment 7

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

It is fixed upstream (I fed them the patch as well), checking if it's a good time to pull.

Comment 8

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

Upstream fix imported:
https://hg.mozilla.org/projects/alder/rev/c566634f51c6

Comment 9

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

This is not a useful candidate for a testcase IMHO