Closed
Bug 1280283
Opened 9 years ago
Closed 9 years ago
Fix uchex warnings about null-check after pointer deref in webrtc/signaling
Categories
(Core :: WebRTC: Signaling, defect, P2)
Core
WebRTC: Signaling
Tracking
()
RESOLVED
FIXED
mozilla50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: cpeterson, Assigned: cpeterson)
Details
Attachments
(1 file)
2.06 KB,
patch
|
drno
:
review+
|
Details | Diff | Splinter Review |
These warnings were reported by µchex, a static analysis tool described in this paper:
How to Build Static Checking Systems Using Orders of Magnitude Less Code
Fraser Brown, Andres Nötzli, Dawson Engler (ASPLOS '16)
https://web.stanford.edu/%7Emlfbrown/paper.pdf
These functions null check their pointer parameters, implying the parameters could legitimately be null, but they also dereference the pointers *before* the null checks.
These warnings are not actual bugs today because none of these functions' current callers pass pointers that could be null. (They're passing the addresses of local stack variables.) The SDP coding style includes many superfluous null checks of pointer parameters, which triggers these uchex warnings. These null checks could be removed or replaced with debug asserts.
Attachment #8762937 -
Flags: review?(adam)
Updated•9 years ago
|
Rank: 25
Priority: -- → P2
Comment 1•9 years ago
|
||
Comment on attachment 8762937 [details] [diff] [review]
webrtc-signaling-uchex.patch
Review of attachment 8762937 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks!
LGTM
Attachment #8762937 -
Flags: review?(adam) → review+
Comment 2•9 years ago
|
||
(In reply to Chris Peterson [:cpeterson] from comment #0)
> The SDP coding style includes many
> superfluous null checks of pointer parameters, which triggers these uchex
> warnings. These null checks could be removed or replaced with debug asserts.
If you could provide us with a list of these that would be great.
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Nils Ohlmeier [:drno] from comment #2)
> (In reply to Chris Peterson [:cpeterson] from comment #0)
> > The SDP coding style includes many
> > superfluous null checks of pointer parameters, which triggers these uchex
> > warnings. These null checks could be removed or replaced with debug asserts.
>
> If you could provide us with a list of these that would be great.
next_token() is a good example. It's a static function within sdp_utils.c so all its callers are known. Its parameter checks seem overly defensive and verbose for a function (though mostly harmless) that's not a public API.
http://searchfox.org/mozilla-central/source/media/webrtc/signaling/src/sdp/sipcc/sdp_utils.c#69
Some other examples:
http://searchfox.org/mozilla-central/source/media/webrtc/signaling/src/sdp/sipcc/sdp_utils.c#325
http://searchfox.org/mozilla-central/source/media/webrtc/signaling/src/sdp/sipcc/sdp_utils.c#355
http://searchfox.org/mozilla-central/source/media/webrtc/signaling/src/sdp/sipcc/sdp_utils.c#440
Pushed by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6cdb479ad755
Fix uchex warnings about null-check after pointer deref in webrtc/signaling. r=drno
Comment 5•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in
before you can comment on or make changes to this bug.
Description
•