sctp_userspace_get_mtu_from_ifn is broken and useless on non-Windows platforms

NEW
Unassigned

Status

()

Core
WebRTC: Networking
P3
normal
Rank:
29
3 months ago
a month ago

People

(Reporter: jld, Unassigned)

Tracking

(Depends on: 1 bug, Blocks: 2 bugs)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: sb+)

(Reporter)

Description

3 months ago
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

3 months ago
Lennart: any thoughts on this?
Rank: 29
Flags: needinfo?(lennart.grahl)
Priority: -- → P2

Comment 2

3 months ago
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
Flags: needinfo?(lennart.grahl)

Updated

3 months ago
Whiteboard: sb? → sb+

Comment 3

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

So this depends now on us updating our copy of usrsctp: bug 1297418.
Depends on: 1297418
Mass change P2->P3 to align with new Mozilla triage process.
Priority: P2 → P3
You need to log in before you can comment on or make changes to this bug.