On 4/23/19 4:01 AM, Richard W.M. Jones wrote: > In the C part of the OCaml plugin we create a ‘bytes’ [byte array] and > pass it to the OCaml pread method. The plugin should 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. > > We can avoid this by initializing the array with zeroes. > > Credit: Eric Blake for finding the bug. > --- > plugins/ocaml/ocaml.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/plugins/ocaml/ocaml.c b/plugins/ocaml/ocaml.c > index d854f48..7193842 100644 > --- a/plugins/ocaml/ocaml.c > +++ b/plugins/ocaml/ocaml.c > @@ -444,6 +444,10 @@ pread_wrapper (void *h, void *buf, uint32_t count, > uint64_t offset, > caml_leave_blocking_section (); > > strv = caml_alloc_string (count); > + /* Initialize the buffer with zeroes in case the plugin does not > + * fill it completely. > + */ > + memset (String_val (strv), 0, count); > offsetv = caml_copy_int64 (offset); > flagsv = Val_flags (flags);
Looks reasonable for ensuring there is no sensitive leak. And compared to my comments on patch 2, at least here you are only slowing down .pread, and leaving .pwrite unchanged. Although my other comments on patch 2 question whether we can still come up with something more efficient by reusing a buffer rather than having to reinitialize it on every single NBD_CMD_READ (leaking previously-seen plugin contents is not the same risk as leaking uninitialized data). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
