Commit cc740794 authored by Daniel Verkamp's avatar Daniel Verkamp Committed by Jim Harris
Browse files

jsonrpc: avoid calling poll() with a closed fd 



If a connection is closed by the remote end, the JSON-RPC server could
potentially call poll() on the pollfd containing the invalid file
descriptor.  Rework the logic so that the pollfd's fd field is set to a
negative value so that poll() will ignore it until the connection is
fully cleaned up by spdk_jsonrpc_server_conn_remove().

Also add handling for the potential error conditions returned by poll()
so that if something does go wrong, the server doesn't get stuck polling
a broken pollfd forever.

Change-Id: Id02e3a230740c8ffd513888cb0564891b3aca069
Signed-off-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/369285


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 122e2846

lib/jsonrpc/jsonrpc_internal.h

+1 −0
Original line number Diff line number Diff line
@@ -69,6 +69,7 @@ struct spdk_jsonrpc_server_conn { 
	uint32_t outstanding_requests;

	struct spdk_ring *send_queue;

	struct spdk_jsonrpc_request *send_request;

	struct pollfd *pfd;

};



struct spdk_jsonrpc_server {

lib/jsonrpc/jsonrpc_server_tcp.c

+26 −3
Original line number Diff line number Diff line
@@ -107,13 +107,27 @@ spdk_jsonrpc_server_shutdown(struct spdk_jsonrpc_server *server) 
	free(server);

}



static void

spdk_jsonrpc_server_conn_close(struct spdk_jsonrpc_server_conn *conn)

{

	conn->closed = true;



	/* Set the pollfd fd to a negative value so it is ignored by poll() */

	conn->pfd->fd = -1;



	if (conn->sockfd >= 0) {

		close(conn->sockfd);

		conn->sockfd = -1;

	}

}



static void

spdk_jsonrpc_server_conn_remove(struct spdk_jsonrpc_server_conn *conn)

{

	struct spdk_jsonrpc_server *server = conn->server;

	int conn_idx = conn - server->conns;



	close(conn->sockfd);

	spdk_jsonrpc_server_conn_close(conn);



	spdk_ring_free(conn->send_queue);
@@ -159,6 +173,8 @@ spdk_jsonrpc_server_accept(struct spdk_jsonrpc_server *server) 
		pfd = &server->pollfds[conn_idx + 1];

		pfd->fd = conn->sockfd;

		pfd->events = POLLIN | POLLOUT;

		pfd->revents = 0;

		conn->pfd = pfd;



		server->num_conns++;
@@ -389,7 +405,7 @@ spdk_jsonrpc_server_poll(struct spdk_jsonrpc_server *server) 
			rc = spdk_jsonrpc_server_conn_send(conn);

			if (rc != 0) {

				SPDK_TRACELOG(SPDK_TRACE_RPC, "closing conn due to send failure\n");

				conn->closed = true;

				spdk_jsonrpc_server_conn_close(conn);

				continue;

			}

		}
@@ -398,11 +414,18 @@ spdk_jsonrpc_server_poll(struct spdk_jsonrpc_server *server) 
			rc = spdk_jsonrpc_server_conn_recv(conn);

			if (rc != 0) {

				SPDK_TRACELOG(SPDK_TRACE_RPC, "closing conn due to recv failure\n");

				conn->closed = true;

				spdk_jsonrpc_server_conn_close(conn);

				continue;

			}

		}



		if (pfd->revents & (POLLERR | POLLNVAL)) {

			SPDK_TRACELOG(SPDK_TRACE_RPC, "closing conn due to poll() flag %d\n",

				      (int)pfd->revents);

			spdk_jsonrpc_server_conn_close(conn);

			continue;

		}



		pfd->revents = 0;

	}