Re: [Libguestfs] [nbdkit PATCH 7/7] nbd: Implement structured replies

Time to enhance the nbd plugin to request structured replies from the server. For now, deal only with structured reads. The server can now return sparse reads, even though we need nbdkit version 3 before we can in turn return sparse reads back to the client. In general, we have to assume the server is malicious, and so we must sanity check that it sends replies we expect. Thus, we have a choice between either implementing bookkeeping to ensure that the server sends exactly as many non-overlapping chunks as needed to cover the entire read request, or else ensuring that even when the server cheats, we do not leak uninitialized memory back to our client. I chose for simplicity, with the result that I end up calling memset() more frequently than necessary for a compliant server, rather than worrying about bookkeeping to detect a non-compliant server that is attempting to cause an information leak. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- plugins/nbd/nbd.c | 232 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 212 insertions(+), 20 deletions(-)

+ case NBD_STRUCTURED_REPLY_MAGIC: + if (!h->structured) { + nbdkit_error ("structured response without negotiation"); + return nbd_mark_dead (h); + } + if (read_full (h->fd, sizeof rep.simple + (char *) &rep, + sizeof rep - sizeof rep.simple)) + return nbd_mark_dead (h); + rep.structured.flags = be16toh (rep.structured.flags); + rep.structured.type = be16toh (rep.structured.type); + rep.structured.length = be32toh (rep.structured.length); + nbdkit_debug ("received structured reply %s for cookie %#" PRIx64 + ", payload length %" PRId32, + name_of_nbd_reply_type(rep.structured.type), + rep.structured.handle, rep.structured.length); + if (rep.structured.length > 64 * 1024 * 1024) { + nbdkit_error ("structured reply length is suspiciously large: %" PRId32, + rep.structured.length); + return nbd_mark_dead (h); + } + if (rep.structured.length) { + /* Special case for OFFSET_DATA in order to read tail of chunk + directly into final buffer later on */ + len = (rep.structured.type == NBD_REPLY_TYPE_OFFSET_DATA && + rep.structured.length > sizeof offset) ? sizeof offset : + rep.structured.length; + buf = malloc (len);

+ case NBD_REPLY_TYPE_OFFSET_DATA: + if (rep.structured.length <= sizeof offset) { + nbdkit_error ("structured reply OFFSET_DATA too small"); + free (buf); + return nbd_mark_dead (h); + } + memcpy (&offset, buf, sizeof offset); + offset = be64toh (offset); + len = rep.structured.length - sizeof offset; + break;

+ case NBD_REPLY_TYPE_OFFSET_HOLE: + if (rep.structured.length != sizeof offset + sizeof len) { + nbdkit_error ("structured reply OFFSET_HOLE size incorrect"); + free (buf); + return nbd_mark_dead (h); + } + memcpy (&offset, buf, sizeof offset); + offset = be64toh (offset); + memcpy (&len, buf, sizeof len); + len = be32toh (len); + if (!len) { + nbdkit_error ("structured reply OFFSET_HOLE length incorrect"); + free (buf); + return nbd_mark_dead (h); + } + zero = true; + break;