Closed
Bug 1342732
Opened 7 years ago
Closed 7 years ago
memory overruns reported for libyuv update
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox-esr45 | --- | unaffected |
firefox51 | --- | unaffected |
firefox52 | --- | unaffected |
firefox-esr52 | --- | unaffected |
firefox53 | --- | unaffected |
firefox54 | --- | fixed |
People
(Reporter: jesup, Assigned: jesup)
References
Details
(Keywords: csectype-bounds, sec-moderate)
Attachments
(1 file)
4.33 KB,
patch
|
sotaro
:
review+
|
Details | Diff | Splinter Review |
Triggered by the landing of the libyuv update in bug 1341543. This appears to be an upstream bug, apparently recently introduced in optimizations in September 2016 for "Add low level support for 12 bit 420, 422 and 444 YUV video frame conversion": https://codereview.chromium.org/2371293002 It's very unclear if we'd ever use these functions, though perhaps it's possible. It's unclear to me if the bug is the array being too small, or the offset in the ANY_SIMD call being too large. Safe solution it to expand the temp array. I'll report it upstream. ** CID 1401524: Memory - illegal accesses (OVERRUN) /media/libyuv/libyuv/source/row_any.cc: 726 in HalfFloatRow_Any_AVX2() ________________________________________________________________________________________________________ *** CID 1401524: Memory - illegal accesses (OVERRUN) /media/libyuv/libyuv/source/row_any.cc: 726 in HalfFloatRow_Any_AVX2() 720 } 721 722 #ifdef HAS_HALFFLOATROW_SSE2 723 ANY11P16(HalfFloatRow_Any_SSE2, HalfFloatRow_SSE2, float, 1, 1, 7) 724 #endif 725 #ifdef HAS_HALFFLOATROW_AVX2 >>> CID 1401524: Memory - illegal accesses (OVERRUN) >>> Overrunning array of 128 bytes at byte offset 128 by dereferencing pointer "&temp[64]". 726 ANY11P16(HalfFloatRow_Any_AVX2, HalfFloatRow_AVX2, float, 1, 1, 15) 727 #endif 728 #ifdef HAS_HALFFLOATROW_F16C 729 ANY11P16(HalfFloatRow_Any_F16C, HalfFloatRow_F16C, float, 1, 1, 15) 730 ANY11P16(HalfFloat1Row_Any_F16C, HalfFloat1Row_F16C, float, 1, 1, 15) 731 #endif ** CID 1401525: Memory - illegal accesses (OVERRUN) /media/libyuv/libyuv/source/row_any.cc: 723 in HalfFloatRow_Any_SSE2() ________________________________________________________________________________________________________ *** CID 1401525: Memory - illegal accesses (OVERRUN) /media/libyuv/libyuv/source/row_any.cc: 723 in HalfFloatRow_Any_SSE2() 717 memcpy(temp, src_ptr + n * SBPP, r * SBPP); \ 718 ANY_SIMD(temp, temp + 64, shuffler, MASK + 1); \ 719 memcpy(dst_ptr + n * BPP, temp + 64, r * BPP); \ 720 } 721 722 #ifdef HAS_HALFFLOATROW_SSE2 >>> CID 1401525: Memory - illegal accesses (OVERRUN) >>> Overrunning array of 128 bytes at byte offset 128 by dereferencing pointer "&temp[64]". 723 ANY11P16(HalfFloatRow_Any_SSE2, HalfFloatRow_SSE2, float, 1, 1, 7) 724 #endif 725 #ifdef HAS_HALFFLOATROW_AVX2 726 ANY11P16(HalfFloatRow_Any_AVX2, HalfFloatRow_AVX2, float, 1, 1, 15) 727 #endif 728 #ifdef HAS_HALFFLOATROW_F16C
Assignee | ||
Comment 1•7 years ago
|
||
MozReview-Commit-ID: ARnj1K5o2LM
Attachment #8841299 -
Flags: review?(sotaro.ikeda.g)
Assignee | ||
Comment 2•7 years ago
|
||
https://bugs.chromium.org/p/libyuv/issues/detail?id=686 There's no way for externals to submit hidden sec issues...
Comment 3•7 years ago
|
||
Comment on attachment 8841299 [details] [diff] [review] Adjust ANY11P16 Review of attachment 8841299 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/libyuv/libyuv/source/row_any.cc @@ +706,5 @@ > // Any 1 to 1 with parameter and shorts. BPP measures in shorts. > #define ANY11P16(NAMEANY, ANY_SIMD, T, SBPP, BPP, MASK) \ > void NAMEANY(const uint16* src_ptr, uint16* dst_ptr, T shuffler, \ > int width) { \ > + SIMD_ALIGNED(uint16 temp[64 * 2]); \ Don't we need to use sizeof(uint16) here?
Attachment #8841299 -
Flags: review?(sotaro.ikeda.g)
Assignee | ||
Comment 4•7 years ago
|
||
\
> > + SIMD_ALIGNED(uint16 temp[64 * 2]); \
>
> Don't we need to use sizeof(uint16) here?
It's already uint16_t typed
Assignee | ||
Comment 5•7 years ago
|
||
Comment on attachment 8841299 [details] [diff] [review] Adjust ANY11P16 re-requesting
Attachment #8841299 -
Flags: review?(sotaro.ikeda.g)
Updated•7 years ago
|
Attachment #8841299 -
Flags: review?(sotaro.ikeda.g) → review+
Assignee | ||
Comment 6•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/dabb9120d849fec9dd6946ee4c43d15d0b9679ae
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Blocks: 1341543
status-firefox51:
--- → unaffected
status-firefox52:
--- → unaffected
status-firefox53:
--- → unaffected
status-firefox54:
--- → fixed
status-firefox-esr45:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Comment 7•7 years ago
|
||
This did get merged to m-c, though the bug didn't get marked for some reason. https://hg.mozilla.org/mozilla-central/rev/dabb9120d849fec9dd6946ee4c43d15d0b9679ae
Target Milestone: --- → mozilla54
Updated•7 years ago
|
Group: core-security → core-security-release
Updated•7 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•