Closed Bug 1016631 Opened 6 years ago Closed 6 years ago

(cosmetic) sysctl()'s 2nd argument is u_int

Categories

(Core :: General, defect)

All
FreeBSD
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: jbeich, Assigned: jbeich)

References

Details

Attachments

(1 file, 3 obsolete files)

Attached patch fix (obsolete) — Splinter Review
According to OS X and BSDs manpage the 2nd argument is |u_int namelen|. To confuse more, sysctlnametomib()'s last argument is |size_t *namelenp|. int is smaller while size_t is usually larger than u_int on 64bit archs but...
- namelen is not a pointer (to write out of bounds)
- name[] length is constant and fits into u_int (to overflow)

https://developer.apple.com/library/mac/documentation/Darwin/Reference/Manpages/man3/sysctl.3.html

A quick search reveals not many files are affected. Do we still care?

content/media/webrtc/LoadMonitor.cpp (Tier3, my fault)
toolkit/crashreporter/google-breakpad/src/client/ios/Breakpad.mm (NPOTB)
toolkit/crashreporter/google-breakpad/src/client/mac/Framework/Breakpad.mm (NPOTB)
toolkit/crashreporter/google-breakpad/src/client/mac/handler/dynamic_images.cc
xpcom/base/nsDebugImpl.cpp
Comment on attachment 8429607 [details] [diff] [review]
fix

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

::: toolkit/crashreporter/google-breakpad/src/client/mac/handler/dynamic_images.cc
@@ +562,1 @@
>    int err = sysctlnametomib("sysctl.proc_cputype", mib, &mibLen);

Oh, it was valid use. sysctlnametomib() wants size_t while sysctl() later wants u_int.
Attached patch fix (obsolete) — Splinter Review
dropped breakpad changes entirely due to other files being NPOTB
Attachment #8429607 - Attachment is obsolete: true
Attachment #8429633 - Flags: review?(vdjeric)
Attachment #8429633 - Flags: review?(gpascutto)
Attachment #8429633 - Flags: review?(gpascutto) → review+
Attachment #8429633 - Flags: review?(vdjeric) → review+
Attached patch fix (obsolete) — Splinter Review
Rebased after bug 1013675. Not the other way around because that'd be more prone to introduce typos.
Attachment #8429633 - Attachment is obsolete: true
Please, land after bug 1013675.
Keywords: checkin-needed
Attached patch fix + styleSplinter Review
s/NULL/nullptr while here per Mozilla code style. Retaining r+ as it's a fix for my code entirely within Tier3 ifdef.
Attachment #8430407 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/3e62fb5c4235
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.