[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

[Nbd] Problems in negotiating modern handshake



There seems to be some non-trivial problems negotiating the handshake with
modern negotiation.

1. If negotiate() returns an error (i.e. NULL), the server SEGVs

2. If an export name is specified, then flags etc. are not sent, it
  simply returns from negotiate.

Aside from that, the error checking could be improved a little.

I've tweaked it here to do what I think it ought to do, but it now
isn't negotiating. What I can't work out is how it was negotiating
before.

Any ideas?

--
Alex Bligh

Signed-Off-By: Alex Bligh <alex@...872...>

diff --git a/nbd-server.c b/nbd-server.c
index cac8e3f..3ebc9ed 100644
--- a/nbd-server.c
+++ b/nbd-server.c
@@ -1341,24 +1341,38 @@ CLIENT* negotiate(int net, CLIENT *client, GArray* servers) {
		if (read(net, &namelen, sizeof(namelen)) < 0)
			err("Negotiation failed: %m");
		namelen = ntohl(namelen);
+		if (!namelen) {
+		  	err("Negotiation failed: bad export name");
+			return NULL;
+		}
		name = malloc(namelen+1);
		name[namelen]=0;
-		if (read(net, name, namelen) < 0)
+		if (read(net, name, namelen) < 0) {
			err("Negotiation failed: %m");
+			free(name);
+			return NULL;
+		}
+		if (!servers) {
+			err("Negotiation failed: no servers");
+			free(name);
+			return NULL;
+		}
		for(i=0; i<servers->len; i++) {
			SERVER* serve = &(g_array_index(servers, SERVER, i));
			if(!strcmp(serve->servename, name)) {
-				CLIENT* client = g_new0(CLIENT, 1);
+				client = g_new0(CLIENT, 1);
				client->server = serve;
				client->exportsize = OFFT_MAX;
				client->net = net;
				client->modern = TRUE;
-				free(name);
-				return client;
+				break;
			}
		}
		free(name);
-		return NULL;
+		if (!client) {
+			err("Negotiation failed: bad export name");
+			return NULL;
+		}
	}
	/* common */
	size_host = htonll((u64)(client->exportsize));
@@ -1824,7 +1843,8 @@ int serveloop(GArray* servers) {
					close(net);
					net=0;
				}
-				serve = client->server;
+				else
+				  serve = client->server;
			}
			for(i=0;i<servers->len && !net;i++) {
				serve=&(g_array_index(servers, SERVER, i));




Reply to: