On 6/14/19 8:08 AM, Eric Blake wrote:
If the server passes us a malformed error reply type with a message
length longer than the overall structured reply, we would blindly obey
that size and get out of sync with the server (perhaps even hanging on
a read for data that will never come). Broken since its introduction
in commit 28952eda.
Fix it by parsing the tail of an error separate from the message,
which also lets us add other sanity checking, and makes it easier if a
future patch wants to capture instead of ignore the server's message.
---
REPLY.STRUCTURED_REPLY.RECV_ERROR_MESSAGE:
+ uint32_t length, msglen;
+ uint16_t type;
+
+ switch (recv_into_rbuf (h)) {
+ case -1: SET_NEXT_STATE (%.DEAD); return -1;
+ case 0:
+ length = be32toh (h->sbuf.sr.structured_reply.length);
+ msglen = be16toh (h->sbuf.sr.payload.error.len);
+ type = be16toh (h->sbuf.sr.structured_reply.type);
h->sbuf is a union...
+
+ length -= sizeof h->sbuf.sr.payload.error - msglen;
+
+ /* Special case two specific errors; ignore the tail for all others */
+ h->rbuf = NULL;
+ h->rlen = length;
+ switch (type) {
+ case NBD_REPLY_TYPE_ERROR:
+ if (length != 0) {
+ SET_NEXT_STATE (%.DEAD);
+ set_error (0, "error payload length too large");
+ return -1;
+ }
+ break;
+ case NBD_REPLY_TYPE_ERROR_OFFSET:
+ if (length != sizeof h->sbuf.offset) {
+ SET_NEXT_STATE (%.DEAD);
+ set_error (0, "error payload length too large");
+ return -1;
+ }
+ h->rbuf = &h->sbuf.offset;
...placing offset directly in sbuf means that sbuf.sr is partially
invalidated...
+ break;
+ }
+ SET_NEXT_STATE (%RECV_ERROR_TAIL);
+ }
+ return 0;
+
+ REPLY.STRUCTURED_REPLY.RECV_ERROR_TAIL:
struct command_in_flight *cmd;
uint16_t flags;
uint64_t handle;
uint32_t error;
+ uint64_t offset;
switch (recv_into_rbuf (h)) {
case -1: SET_NEXT_STATE (%.DEAD); return -1;
case 0:
flags = be16toh (h->sbuf.sr.structured_reply.flags);
handle = be64toh (h->sbuf.sr.structured_reply.handle);
error = be32toh (h->sbuf.sr.payload.error.error);
which is not good for this.
+++ b/lib/internal.h
@@ -143,6 +143,7 @@ struct nbd_handle {
uint32_t len;
uint16_t nrinfos;
uint32_t nrqueries;
+ uint64_t offset;
} sbuf;
So I'll need to move offset to a separate location other than sbuf.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization:
qemu.org |
libvirt.org