audio: fix device name use-after-free in AudioDevice::open

[mautier]

Fixes a use-after-free in AudioDevice::open, which occurs when
selecting a particular device by name (as opposed to using a default
device).

Specifically, when extracting a C-style string pointer from a
Option<CString>, the option was consumed, its contents turned into a
pointer, and the original CString dropped. After that the C-style
string pointer is dangling, as its backing rust CString has been freed.

To fix this, the CString must be kept alive, which is achieved by simply
creating an intermediary Option<&CString>, and consuming that.

Example

When trying to open an audio recording device called HD Webcam C615 Analog Mono on my machine:

• device_ptr points to @\xf4~UUU\x00\x00m C615 Analog Mono (which when interpreted as a null-terminated C string is actually just @\xf4~UUU), which shows that the user-provided name was clobbered somehow.
• valgrind reports:

==9713== Invalid read of size 1
==9713==    at 0x4C33DA3: strcmp (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9713==    by 0x50D47E4: ??? (in /usr/lib/x86_64-linux-gnu/libSDL2-2.0.so.0.8.0)
==9713==    by 0x127B51: sdl2::audio::AudioDevice<CB>::open (audio.rs:658)
==9713==    by 0x1277AF: sdl2::audio::AudioDevice<CB>::open_capture (audio.rs:706)
==9713==    by 0x114F87: sdl2::audio::<impl sdl2::sdl::AudioSubsystem>::open_capture (audio.rs:93)
==9713==   [...]

==9713==  Address 0xb869fa0 is 0 bytes inside a block of size 27 free'd
==9713==    at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9713==    by 0x1360BD: alloc::alloc::dealloc (alloc.rs:103)
==9713==    by 0x136668: <alloc::alloc::Global as core::alloc::AllocRef>::dealloc (alloc.rs:179)
==9713==    by 0x1362CF: alloc::alloc::box_free (alloc.rs:245)
==9713==    by 0x135B06: core::ptr::drop_in_place (mod.rs:177)
==9713==    by 0x135A03: core::ptr::drop_in_place (mod.rs:177)
==9713==    by 0x1280DD: sdl2::audio::AudioDevice<CB>::open::{{closure}} (audio.rs:655)
==9713==    by 0x132A28: core::option::Option<T>::map_or (option.rs:483)
==9713==    by 0x127AC7: sdl2::audio::AudioDevice<CB>::open (audio.rs:655)
==9713==    by 0x1277AF: sdl2::audio::AudioDevice<CB>::open_capture (audio.rs:706)
==9713==    by 0x114F87: sdl2::audio::<impl sdl2::sdl::AudioSubsystem>::open_capture (audio.rs:93)
==9713==   [...]

==9713==  Block was alloc'd at
==9713==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9713==    by 0x14429E: alloc (alloc.rs:81)
==9713==    by 0x14429E: alloc (alloc.rs:172)
==9713==    by 0x14429E: allocate_in<u8,alloc::alloc::Global> (raw_vec.rs:87)
==9713==    by 0x14429E: with_capacity<u8> (raw_vec.rs:141)
==9713==    by 0x14429E: with_capacity<u8> (vec.rs:357)
==9713==    by 0x14429E: <&str as std::ffi::c_str::CString::new::SpecIntoVec>::into_vec (c_str.rs:345)
==9713==    by 0x13A397: std::ffi::c_str::CString::new (c_str.rs:351)
==9713==    by 0x127A3E: sdl2::audio::AudioDevice<CB>::open (audio.rs:652)
==9713==    by 0x1277AF: sdl2::audio::AudioDevice<CB>::open_capture (audio.rs:706)
==9713==    by 0x114F87: sdl2::audio::<impl sdl2::sdl::AudioSubsystem>::open_capture (audio.rs:93)
==9713==   [...]

After applying the proposed change, valgrind is appeased! 🌺

[mautier]

After some light grepping I found an additional instance of the issue in AudioQueue::open_queue, but that's it.

I saw a number of other locations where the option.as_ref().map_or() pattern (and other similar variants) is used, so maybe the comment block warning I added is too much? If so I'm happy to remove it.

[Cobrand]

Very good catch, thanks a lot!

[Cobrand]

As a side note, I think these ptr related issues are detected by clippy usually, aren't they? If they are, we should run a check with clippy on the whole project to be sure, and if they aren't, we should submit an issue report to clippy to add this as an edge-case.

[mautier]

Sadly it doesn't seem like clippy raises any red flags here…

There is a related lint: temporary_cstring_as_ptr, but it's too limited to catch this particular instance.
There's also an existing clippy tracking issue for extending that lint, but looks like it's been dormant for a while: rust-lang/rust-clippy#2045.