Missing lifetime annotation on `RawSamples<S>`?

[vnrst]

This function duplicates the pointer to the underlying data self.data.mem.ptr, but RawSamples<S> doesn't have a lifetime annotation to mark this loan.

https://github.com/diwic/alsa-rs/blob/db4bdd7f4e467148c091720589d217fdafb5d168/src/direct/pcm.rs#L418-L437

The same underlying memory is invalidated when DriverMemory<S> is dropped.

https://github.com/diwic/alsa-rs/blob/db4bdd7f4e467148c091720589d217fdafb5d168/src/direct/pcm.rs#L231-L235

So the raw samples can still be accessed after the MmapIO object is dropped, and they will contain a raw pointer to invalid memory.

extern crate alsa;

use alsa::pcm::{PCM, HwParams, Format, Access, State};
use alsa::direct::pcm::MmapCapture;

fn main() {
    let pcm = PCM::new("default", alsa::Direction::Capture, false).unwrap();
    let hwp = HwParams::any(&pcm).unwrap();
    hwp.set_channels(2).unwrap();
    hwp.set_format(Format::s16()).unwrap();
    hwp.set_access(Access::MMapInterleaved).unwrap();
    pcm.hw_params(&hwp).unwrap();
    pcm.prepare().unwrap();
    let mut mmap = pcm.direct_mmap_capture<i32>().unwrap();
    let (raw_samples, _) = mmap.data_ptr();
    drop(mmap);
    println!("{}", raw_samples.samples()); // raw_samples is still accessible
}

I don't know if this can be considered a design flaw because the RawSamples object is designed to be used only with unsafe code and it's the user's responsibility to ensure that it's used in a safe manner. But adding a lifetime annotation like RawSamples<'a, S> along with a PhantomData marker would definitely remove any possibility of misuse.

[vnrst]

Edited my comment to say that this cannot be a use-after-free using only safe Rust. So it's not a vulnerability/UB.

[diwic]

So I think you have a point, but as you say it's only meant to be used with unsafe code anyway. And no lifetime annotation means simpler code. So maybe it would be better to just write documentation about this. And btw - this is quite transient data anyhow, wait a second and all values will be obsolete so having a lifetime annotation might give the impression that the data would be valid as long as the mmapio is valid, which might be true from a Rust memory safety perspective but not a practical one...

[diwic]

So I'm leaning towards keeping it as it is, but if you have a good use case for where this would be very helpful I'm open for such an argument :-)