1

I am getting memory allocated by an external C function. I then convert the memory to a [u8] slice using std::slice::from_raw_parts(). To avoid repeated calls to from_raw_parts() I want to store the slice as a field. So far I have this.

struct ImageBuffer<'a> {
    //Slice view of buffer allocated by C
    pub bytes: &'a [u8],
}

But this is wrong right there. This says bytes has a lifetime larger than the ImageBuffer instance. This does not reflect the reality. The buffer is freed up from drop() and hence bytes should have a lifetime same as the struct instance. How do I model that?

With the current code it is easy to use after free.

impl <'a> ImageBuffer<'a> {
    pub fn new() -> ImageBuffer<'a> {
        let size: libc::size_t = 100;
        let ptr = unsafe {libc::malloc(size)};
        let bytes = unsafe {std::slice::from_raw_parts(ptr as *const u8, size)};

        ImageBuffer {
            bytes,
        }
    }
}

impl <'a> Drop for ImageBuffer<'a> {
    fn drop(&mut self) {
        unsafe {
            libc::free(self.bytes.as_ptr() as *mut libc::c_void)
        };
    }
}

fn main() {
    let bytes;

    {
        let img = ImageBuffer::new();
        bytes = img.bytes;    
    }

    //Use after free!
    println!("Size: {}. First: {}", bytes.len(), bytes[0]);
}

I can solve this problem by writing a getter function for bytes.

First make bytes private.

struct ImageBuffer<'a> {
    bytes: &'a [u8],
}

Then write this getter method. This establishes the fact that the returned bytes has a lifetime same as the struct instance.

impl <'a> ImageBuffer<'a> {
    pub fn get_bytes(&'a self) -> &'a [u8] {
        self.bytes
    }
}

Now, a use after free will not be allowed. The following will not compile.

fn main() {
    let bytes;

    {
        let img = ImageBuffer::new();
        bytes = img.get_bytes();    
    }

    println!("Size: {}. First: {}", bytes.len(), bytes[0]);
}

I find this solution deeply disturbing. The struct declaration is still conveying a wrong meaning (it still says bytes has a larger lifetime than the struct instance). The get_bytes() method counters that and conveys the correct meaning. I'm looking for an explanation of this situation and what the best way to handle it.

RajV
  • 6,860
  • 8
  • 44
  • 62
  • 2
    I think it's a contrived case of [Why can't I store a value and a reference to that value in the same struct?](https://stackoverflow.com/questions/32300132/why-cant-i-store-a-value-and-a-reference-to-that-value-in-the-same-struct) – cafce25 Dec 26 '22 at 01:45
  • @cafce25 thanks. I saw that question before posting. IMHO, it's not the same situation. For example, I don't really need to store ``ptr``. I can just store the slice ``bytes`` in the struct. To free the memory I can always get the ptr back from the ``&[u8]`` slice. I will still face the same problem. – RajV Dec 26 '22 at 01:52
  • 1
    You should do the opposite, only store `ptr` (and maybe length) and get `&[u8]` on-demand in `.get_bytes()`. – kmdreko Dec 26 '22 at 02:04
  • I have updated the question to store just the slice. – RajV Dec 26 '22 at 02:06
  • 3
    it's not deeply disturbing, your use of `unsafe` is guaranteeing to the compiler that you know what you are doing in terms of the lifetimes and then you are explicitly breaking that contract. – Ahmed Masud Dec 26 '22 at 02:08
  • @kmdreko ``ptr`` is of type ``*mut libc::c_void``. AFAIK it doesn't have any ``get_bytes()`` method. I need to use ``from_raw_parts()`` to get a byte array. But that function is not trivial and I want to avoid excessive calls to it. – RajV Dec 26 '22 at 02:09
  • You can store the reference, just not in the same struct as the ptr wich is the conceptual owner of the memory like explained in the dupe. – cafce25 Dec 26 '22 at 02:13
  • @cafce25 I think you are on the right track. I will try using an inner struct and hold the reference there. I will take a fresh look tomorrow. – RajV Dec 26 '22 at 02:17

1 Answers1

2

Lifetimes cannot be used to express what you are doing, and therefore you should not be using a reference, since references always use lifetimes.

Instead, store a raw pointer in your struct. It can be a slice pointer; just not a slice reference.

struct ImageBuffer {
    bytes: *const [u8],
}

To create the pointer, convert it from the C pointer without involving references. To create a safe reference, do it in your getter (or a Deref implementation):

impl ImageBuffer {
    pub fn new() -> ImageBuffer {
        let size: libc::size_t = 100;
        let ptr = unsafe {libc::malloc(size)};
        assert!(!ptr.is_null());
        let bytes = unsafe {
            std::ptr::slice_from_raw_parts(ptr as *const u8, size)
        };

        ImageBuffer {
            bytes,
        }
    }


    pub fn get_bytes(&self) -> &[u8] {
        // Safety: the pointer is valid until `*self` is dropped, and it
        //  cannot be dropped while it is borrowed by this reference 
        unsafe { &*self.bytes }
    }
}

This is essentially what Box, the basic owning pointer type, does, except that your pointer was allocated by the C allocator and this needs to be freed using it too (using your Drop implementation).

All of this is the normal and routine thing to do to make a safe wrapper for an owning C pointer.

Kevin Reid
  • 37,492
  • 13
  • 80
  • 108
  • How do I get ``*const [u8]`` from ``*mut libc::c_void`` which is what the C backend gives me? – RajV Dec 26 '22 at 11:30
  • @RajV With [`std::ptr::slice_from_raw_parts`](https://doc.rust-lang.org/std/ptr/fn.slice_from_raw_parts.html). I've updated my answer to demonstrate. – Kevin Reid Dec 26 '22 at 15:51