1384292 - sctp_userspace_get_mtu_from_ifn is broken and useless on non-Windows platforms

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect, P3)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Source: http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/netwerk/sctp/src/netinet/sctp_userspace.c#40

The if_index parameter is always 0xffffffff, so the if_indextoname fails, but its return value isn't checked; this means that stack garbage is used as the interface name for SIOCGIFMTU, which will almost certainly fail and cause this routine to return 0.

But that doesn't matter, because apparently nothing is actually using the sctp_ifn::ifn_mtu field that's initialized by this function.

I'd suggest removing this function, at least in the !defined(_WIN32) case.  (The Windows implementation may or may not be broken in a similar way; I haven't investigated.)

In particular, I want this code to go away on Linux, because I'm trying to lock down the set of allowed ioctls, and eventually I'll be going after socket().

Comment 1

[Nils Ohlmeier [:drno]]

Lennart: any thoughts on this?

Comment 2

[Lennart Grahl [:lgrahl]]

The code from the upstream repo has been adjusted slightly but the return value is still not being checked (see: https://github.com/sctplab/usrsctp/blob/5b776478da5a4ed3ab935929c30c25d6e3fbf140/usrsctplib/netinet/sctp_userspace.c#L88:L105).

I've opened an issue in their repo: https://github.com/sctplab/usrsctp/issues/157

Comment 3

[Nils Ohlmeier [:drno]]

Fixed upstream https://github.com/sctplab/usrsctp/commit/1313bd57676c1f22db222000059fc2e914ad94fa

So this depends now on us updating our copy of usrsctp: bug 1297418.

Comment 4

[Bulk Bug Changes for mreavy's org]

Mass change P2->P3 to align with new Mozilla triage process.

Comment 5

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

fixed by bug 1297418