Closed
Bug 1449951
Opened 7 years ago
Closed 6 years ago
Media libraries have questionable use of __declspec(safebuffers)
Categories
(Core :: Audio/Video: Playback, enhancement, P3)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: away, Unassigned)
References
Details
(Keywords: sec-audit)
We fail Binscope.exe's GSFunctionSafeBuffers check because of:
error BA2014: xul.dll is a C or C++ binary built with function(s) (InitCpuFlags,ArmCpuCaps,MipsCpuCaps) that disable the stack protector. The stack protector (/GS) is a security feature of the compiler which makes it more difficult to exploit stack buffer overflow memory corruption vulnerabilities. Disabling the stack protector, even on a function-by-function basis, is disallowed by SDL policy. To resolve this issue, remove occurrences of __declspec(safebuffers) from your code. If the additional code inserted by the stack protector has been shown in profiling to cause a significant performance problem for your application, attempt to move stack buffer modifications out of the hot path of execution to allow the compiler to avoid inserting stack protector checks in these locations rather than outright disabling the stack protector.
It is not at all clear to me why InitCpuFlags,ArmCpuCaps,MipsCpuCaps are marked SAFEBUFFERS: https://searchfox.org/mozilla-central/search?q=SAFEBUFFERS&case=true®exp=false&path=
Init in particular is a one-time function and should not care about the perf hit of stack checks. Mips and Arm aren't even needed in current builds.
Fortunately(?) these annotations are conditioned on !defined(__clang__), so if we let this bug sit around it will be fixed for free by changing to clang-cl.
Comment 1•7 years ago
|
||
Can we have a quick look here for libvpx and libyuv ?
Rank: 25
Flags: needinfo?(rjesup)
Flags: needinfo?(jyavenard)
Priority: -- → P3
Comment 2•7 years ago
|
||
ArmCpuCaps/MipsCpuCaps don't seem to be used anywhere except within InitCpuFlags... InitCpuFlags is used, all over the place (via TestCpuFlags()), but appears will actually be called only once from TestCpuFlags() -- (cpu_flags_ ? InitCpuFlags() : cpu_flags_); InitCpuFlags() stores the result in cpu_flags_). It's used directly all the time in MaskCpuFlags, but that's only used in tests.
Flags: needinfo?(rjesup)
Updated•7 years ago
|
Flags: needinfo?(jyavenard)
Updated•7 years ago
|
Component: Audio/Video → Audio/Video: Playback
Fortunately(?) these annotations are conditioned on !defined(clang), so
if we let this bug sit around it will be fixed for free by changing to
clang-cl.
Hi from 2019.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•