thesofproject / sof

Sound Open Firmware
Other
525 stars 306 forks source link

[FEATURE] Add fuzzing endpoints for math functions #5080

Open cujomalainey opened 2 years ago

cujomalainey commented 2 years ago

Is your feature request related to a problem? Please describe. We have a lot of math functions with precise lookups. If not implemented correctly they could in theory be overflowed resulting in either broken audio or bad memory accesses.

Describe the solution you'd like Fuzz endpoints for the math functions. Fuzzing time should be quite short since there is no state.

Describe alternatives you've considered N/A

Additional context Recommended by @lgirdwood

cujomalainey commented 2 years ago

@afq984 I set you as the placeholder owner, if you have time to do this great, otherwise I might bump it to Eddy once I add him to the org

lgirdwood commented 2 years ago

@singalsu @ShriramShastry fyi. @cujomalainey I guess we can do this via the cmocka or host based build ? If this works well we could extend to other APIs that take host data (at the layers down from the IPC like the components set / get config()).

cujomalainey commented 2 years ago

We really just need to copy the ipc fuzz endpoint, drop all the calls to initialize SOF since the math should not need it. Drop the initialize function. Register the new endpoint and setup for short fuzzing (like 5-10s should probably be good). And we are good.

Thinking about it, it might be better to simply sample the domain of the functions and run it with something like valgrind or the sanitizers as part of the cmocka test like Liam suggested for the math.

The set/get APIs will get hit once we add the components, but significantly less frequently. So it might is worth doing a fuzzing endpoint in which we call those APIs on the components directly to make sure we get good coverage.

lgirdwood commented 2 years ago

Fwiw, all the mocka and testbench can all run with valgrind now (and testbench pulls in more of pipeline logic too).

cujomalainey commented 2 years ago

that looks good, I think if we add sanitizers we should be good, specifically ubsan and maybe address, memory i think should not matter as we are not doing allocs in math

cujomalainey commented 1 year ago

The more I think about this, we could just run the range of values with ubsan and that should cover this as part of something like a unit test

lgirdwood commented 1 year ago

@singalsu @andrula-song @ShriramShastry I know we have UTs for testing maths APIs, can we expand these a little and use ubsan ? https://developers.redhat.com/blog/2014/10/16/gcc-undefined-behavior-sanitizer-ubsan

singalsu commented 1 year ago

@singalsu @andrula-song @ShriramShastry I know we have UTs for testing maths APIs, can we expand these a little and use ubsan ? https://developers.redhat.com/blog/2014/10/16/gcc-undefined-behavior-sanitizer-ubsan

I didn't know such exist, seems like it's good to add to UT x86 build at least, probably also to testbench since valgrind is not detecting all of that.