[Libguestfs] [PATCH nbdkit v2 1/2] ocaml: Change pread method to avoid leaking heap memory.

[Richard W.M. Jones]

On Tue, Apr 23, 2019 at 10:16:45AM -0500, Eric Blake wrote:
> On 4/23/19 10:09 AM, Richard W.M. Jones wrote:
> > In the C part of the OCaml plugin we created a ‘bytes’ [byte array]
> > and passed it to the OCaml pread method.  The plugin is supposed to
> > overwrite the array with the returned data.
> > 
> > However if (eg. because of a bug) the plugin does not fill the array
> > then whatever was in the OCaml or possibly even the C heap before the
> > allocation is returned to the client, possibly resulting in a leak of
> > sensitive data.
> > 
> > This changes the signature of the pread function in OCaml plugins.
> > Instead of passing in an uninitialized ‘bytes’ object of the right
> > length, the count is passed explicitly and the pread method should
> > return a string of the correct length.  This is both more similar to
> > how other language plugins work, and is safer because all allocation
> > is done on the OCaml side.
> > 
> >   - pread : 'a -> bytes -> int64 -> flags -> unit
> >   + pread : 'a -> int32 -> int64 -> flags -> string
> > 
> 
> Incompatible change (all older OCaml plugins will fail to build/run with
> the newer nbdkit), but we never promised compatibility for language
> bindings.

Yup.  I most recently broke the OCaml bindings in a264cbee6c (1st Jan 2019),
and that wasn't the first time :-/  At least OCaml gives you a nice error.

Rich.

> > ---
> >  plugins/ocaml/ocaml.c      | 16 ++++++++++++----
> >  plugins/ocaml/NBDKit.ml    |  4 ++--
> >  plugins/ocaml/NBDKit.mli   |  2 +-
> >  tests/test_ocaml_plugin.ml |  8 +++++---
> >  4 files changed, 20 insertions(+), 10 deletions(-)
> 
> I wondered if nbdkit-ocaml-plugin.pod also needed a change, but it calls
> out NBDKit.mli as the official interface and you patched that instead.
> 
> LGTM.
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 




-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/