Opened 9 years ago
Last modified 15 months ago
#6763 new defect
swscale: Out-of-bounds memory accesses
| Reported by: | Gramner | Owned by: | |
|---|---|---|---|
| Priority: | important | Component: | swscale |
| Version: | git-master | Keywords: | crash |
| Cc: | Blocked By: | ||
| Blocking: | Reproduced by developer: | no | |
| Analyzed by developer: | no |
Description
Many assembly functions in swscale will read past the end of their input buffers which causes segfaults and/or bus errors if the buffer happens to be located near the end of a memory page and the next page is invalid.
Aligning input buffers isn't even enough for formats like RGB24 (and requiring alignment would be a bad idea anyway since it wouldn't work with memory-mapped input files for example).
Using swscale with x264 CLI seems to be a fairly consistent way to trigger such out-of-bounds crashes. This command line for example will cause segfaults in ff_rgb24ToY_avx():
./x264 -o /dev/null --input-csp rgb --input-res 512x512 <any_input_file>
If asm is disabled in swscale the problem goes away.
Change History (6)
comment:1 by , 9 years ago
| Keywords: | crash added |
|---|---|
| Priority: | normal → important |
comment:2 by , 9 years ago
| Version: | unspecified → git-master |
|---|
Not sure which disassembly you're interested in, but the source shows the entire row loop done using 128-bit loads with no special handling for the tail: http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libswscale/x86/input.asm;h=af9afcaa53a74f51eaa1257bafd1052524046074;hb=HEAD#l243
With width=512 as an example, on the last loop iteration "movu m4, [srcq+12]" reads 16 bytes from offset 1524 which results in an overflow of 4 bytes.
The input buffer is the exact size of the input data with zero padding as no such requirement is documented.
comment:3 by , 3 years ago
comment:4 by , 19 months ago
What do we think the ideal resolution is here?
- Simply document this requirement.
- Modify all assembly routines to handle the tail separately.
- Always memcpy the last row before calling into asm.
- Always call the C routine for the last row, instead of asm
I am leaning towards 3, with an option for 1.
Edit: added fourth option
comment:5 by , 19 months ago
It's worth pointing out that these alignment requirements are actually explicitly mentioned, albeit in the documentation for AVFrame:
https://ffmpeg.org/doxygen/2.7/frame_8h_source.html#l00191
Admittedly, this is a very non-obvious place, and also doesn't extend to sws_scale().
After investigating the situation more closely, I have come to notice that practically none of the asm routines are designed to handle images whose stride is not a multiple of at least 8, and I am leaning towards disabling the offending ASM routines entirely as an immediate fix.
A full fix (with a split C wrapper to handle the edges) would be the preferred solution long-term, but this will require a more substantial rewrite of the scaling core - something that is on my to-do list for 2025.
comment:6 by , 15 months ago
I just ran into this same problem.
sws_scale(swsctx, FrameIn->data,FrameIn->linesize, 0, FrameIn->height, FrameOut->data, FrameOut->linesize);
I included some debugging print lines:
Frame In: 540x360 linesize=640
Frame Out: 540x360 linesize=1620
The video is mp4, YUV444 and being converted to RGB24. It reports to stderr:
[swscaler @ 0x16c7300] Warning: data is not aligned! This can lead to a speed loss
The output buffer is allocated as 540x360x3 = 583200 bytes.
However, the misalignment causes sws_scale to write-overflow. (Valgrind detected it.) I see the same problem when using vlc and other video players on the same video since they use the same ffmpeg library.
My workaround: Allocate enough space to round up to a width that is divisible by 8. (Allocate 544x360x3 = 587520 bytes.)



(Version information, backtrace, disassembly and register dump missing.)
Could you confirm that your input buffers are sufficiently padded?