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

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.

> --- > 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