1225292 - FFMPEG: use-of-uninitialized-value in [@ff_get_cpu_flags_x86]

Status: RESOLVED INVALID

(Core :: Audio/Video: Playback, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: call_stack.txt

Found fuzzing ffmpeg commit: 3692d859f45fa8765fa5a330e79108b03c17c6bd

Reproduced with the following command with MSan build:
./ffmpeg -nostats -loglevel 0 -i dummy.mp3 -f null -

Comment 1

[Tyson Smith [:tsmith]]

Attachment: dummy.mp3

Comment 2

[Tyson Smith [:tsmith]]

Patched this locally to make progress fuzzing and hit many other uninitialized reads in this function.

I fixed these: (and likely others b/c I just initialized everything in that function to make progress)

MemorySanitizer: use-of-uninitialized-value /home/user/code/ffmpeg/libavutil/x86/cpu.c:106:9 in ff_get_cpu_flags_x86
MemorySanitizer: use-of-uninitialized-value /home/user/code/ffmpeg/libavutil/x86/cpu.c:137:13 in ff_get_cpu_flags_x86
MemorySanitizer: use-of-uninitialized-value /home/user/code/ffmpeg/libavutil/x86/cpu.c:165:9 in ff_get_cpu_flags_x86

Comment 3

[Jean-Yves Avenard [:jya]]

note that those generates a lot of compilation warning as found with bug 1214462 when using the visual studio compiler

Comment 4

[Michael Niedermayer [:mn]]

max_std_level and the other variables in ff_get_cpu_flags_x86() are set from ff_cpu_cpuid() which is build from external asm with yasm (unless thats disabled) clangs memory sanitizer seems not able to track this.
For testing with clang msan its probably needed to use --disable-yasm, this is a bad idea when not testing with msan though due to the likely significant speedloss.
valgrind should be able to search for uninitialized variables with enabled yasm, its alot slower though

this issue likely affects the other fuzzing bugs too

Comment 5

[Tyson Smith [:tsmith]]

Thanks. I have verified that using --disable-yasm resolves this issue with MSan. I will keep this in mind when reviewing issues that are initialized by assembly.

Comment 6

[Jean-Yves Avenard [:jya]]

Michael, any particular reasons ffmpeg requires yasm to compile assembly as opposed to using intrinsic assembly?

One of the issue for us is that our build are still stuck on an old version of yasm (1.1) and we had to disable avx2 as a consequence.

Using inline assembly would have allowed to easily bypass that problem and remove the dependency on yasm.

just a passing-by thought.

Comment 7

[Michael Niedermayer [:mn]]

Not specifically to this (cpuid) code but more generally 
intrinsics have many problems. 
with yasm (or inline asm) the code one writes is the code one gets, with intrinsics a different compiler version or different moon phase and the code might run alot slower because the compiler failed to assign registers and starts spilling everything on the stack in a inner loop. Also i think there are portability issues. 
OTOH the big problem with inline asm is that it only works with gcc compatible compilers, yasm works with any compiler. Also our yasm code can be build with nasm in case that helps

Comment 8

[Jean-Yves Avenard [:jya]]

(In reply to Tyson Smith [:tsmith] from comment #5)
> Thanks. I have verified that using --disable-yasm resolves this issue with
> MSan. I will keep this in mind when reviewing issues that are initialized by
> assembly.

I don't think it makes much sense in fuzzing ffmpeg without yasm being enabled. As all you will be stressing is the C-code decoders and those are almost never used on a standard system: always the assembly optimised code. This is what makes ffmpeg awesome: the speed and how well crafted the assembly is.

Comment 9

[Tyson Smith [:tsmith]]

(In reply to Jean-Yves Avenard [:jya] from comment #8)
> (In reply to Tyson Smith [:tsmith] from comment #5)
> > Thanks. I have verified that using --disable-yasm resolves this issue with
> > MSan. I will keep this in mind when reviewing issues that are initialized by
> > assembly.
> 
> I don't think it makes much sense in fuzzing ffmpeg without yasm being
> enabled. As all you will be stressing is the C-code decoders and those are
> almost never used on a standard system: always the assembly optimised code.
> This is what makes ffmpeg awesome: the speed and how well crafted the
> assembly is.

Yep I know and I am not. This was only to check for uninitiated memory usage in the the code base (no including the replaced asm) that I would be fuzzing.