Bindings are not memory safe (probably)

[asonix]

Questions, enhancements, tips, etc.

Please use libvips discussions for other topics.

https://github.com/libvips/libvips/discussions

Bug report

According to the libvips documentation, the methods new_from_buffer and new_from_memory use the provided buffer to back the VipsImage, rather than copying it internally. That means that dropping the buffer will invalidate the VipsImage.

Here's some example code that would trigger UB (probably)

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let app = libvips::VipsApp::new("vips-problem", true)?;

    let buf = std::fs::read("./example.png")?;
    let image = libvips::VipsImage::new_from_buffer(&buf, "")?;

    // drop the buffer before trying to transcode to jpeg
    drop(buf);
    image.image_write_to_file("example.jpg")?;

    println!("Hello, world!");

    drop(app);

    Ok(())
}

While I haven't experienced a segfault while running this simple example, it does fail with an IO error "cannot write to file" even though the output file here is not the problem, it's that there is no input buffer anymore.

if vips actually can detect and handle this internally then this isn't a real problem, but my hunch is that does not happen.

[jcupitt]

Hello @asonix,

new_from_buffer() etc. are part of the C language binding and need to be reimplemented for other languages. For example, the python binding does this:

https://github.com/libvips/pyvips/blob/master/pyvips/vimage.py#L332-L366

It's a performance issue. When you pass large objects, you need to control copy behaviour, and you can't do that from the C side without extra (possibly unnecessary) copies.

The libvips C API is supposed to be convenient for humans to write, but it's a pretty terrible target for language bindings. The heavy use of varargs is very tricky, for example. libvips has a lower-level GObject API that's designed for binding (the C API is a thin skin over this layer), and I think I would target that from Rust. There's a chapter in the docs:

https://www.libvips.org/API/current/binding.html

This lower level API is relatively small (you only need maybe 20 API calls for a basic binding of the whole of libvips), supports full introspection (you can discover the rest of the API at runtime), gives you control over copy behaviour (fixing your issue here), and has no varargs (it's fully typesafe and memory safe).

pyvips is several 1000 lines now, but it has a lot of extra convenience functions. The core is only a few 100 lines and binds the whole of libvips dynamically, so as well as being memory safe, it might also be less work than binding the C API.

[asonix]

Thank you for this great explanation! I realize now that I meant to open this issue on the rust bindings repo and not the upstream libvips, sorry!

I might look into writing my own bindings sometime, anyway :)