Commit bc315dea authored by Jeffry Molanus's avatar Jeffry Molanus Committed by Darek Stojaczyk
Browse files

bdev_iscsi: fix crash when connecting to a non existing iqn 



When connecting to an iscsi url that effectively does not exist we free
the data structures of the very context we are processing. This results in a
use after free and subsequently in a crash.

Signed-off-by: default avatarJeffry Molanus <jeffry.molanus@gmail.com>
Change-Id: I67cab1efb161bfa23fa1022e150661080d90b556
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/462614


Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarJeffry Molanus <Jeffry.molanus@gmail.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarDarek Stojaczyk <dariusz.stojaczyk@intel.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent ed56a3d4

lib/bdev/iscsi/bdev_iscsi.c

+37 −17
Original line number Diff line number Diff line
@@ -98,9 +98,10 @@ struct bdev_iscsi_conn_req { 
	char					*initiator_iqn;

	struct iscsi_context			*context;

	spdk_bdev_iscsi_create_cb		create_cb;

	spdk_bdev_iscsi_create_cb		create_cb_arg;

	void					*create_cb_arg;

	bool					unmap_supported;

	int					lun;

	int					status;

	TAILQ_ENTRY(bdev_iscsi_conn_req)	link;

};
@@ -110,17 +111,12 @@ complete_conn_req(struct bdev_iscsi_conn_req *req, struct spdk_bdev *bdev, 
{

	TAILQ_REMOVE(&g_iscsi_conn_req, req, link);

	req->create_cb(req->create_cb_arg, bdev, status);

	if (status) {

		/* if the request failed and no iscsi lun was

		 * created then we could not hand over this

		 * memory and have to free it manually now.



	/*

	 * we are still running in the context of iscsi_service()

	 * so do not tear down its data structures here

	 */

		iscsi_destroy_context(req->context);

		free(req->initiator_iqn);

		free(req->bdev_name);

		free(req->url);

	}

	free(req);

	req->status = status;

}



static int
@@ -145,14 +141,29 @@ _iscsi_free_lun(void *arg) 
	free(lun);

}



static void

_bdev_iscsi_conn_req_free(struct bdev_iscsi_conn_req *req)

{



	free(req->initiator_iqn);

	free(req->bdev_name);

	free(req->url);

	/* destroy will call iscsi_disconnect() implicitly if connected */

	iscsi_destroy_context(req->context);

	free(req);

}



static void

bdev_iscsi_finish(void)

{

	struct bdev_iscsi_conn_req *req;

	struct bdev_iscsi_conn_req *req, *tmp;



	while (!TAILQ_EMPTY(&g_iscsi_conn_req)) {

		req = TAILQ_FIRST(&g_iscsi_conn_req);

		complete_conn_req(req, NULL, -EINTR);

	/* clear out pending connection requests here. We cannot

	 * simply set the state to a non SCSI_STATUS_GOOD state as

	 * the connection poller wont run anymore

	 */

	TAILQ_FOREACH_SAFE(req, &g_iscsi_conn_req, link, tmp) {

		_bdev_iscsi_conn_req_free(req);

	}



	if (g_conn_poller) {
@@ -725,8 +736,16 @@ iscsi_bdev_conn_poll(void *arg) 
				SPDK_ERRLOG("iscsi_service failed: %s\n", iscsi_get_error(context));

			}

		}

	}



		if (req->status != SCSI_STATUS_GOOD) {

			/*

			 * An error has occurred during connecting.  This req has already

			 * been removed from the g_iscsi_conn_req list, but we needed to

			 * wait until iscsi_service unwound before we could free the req.

			 */

			_bdev_iscsi_conn_req_free(req);

		}

	}

	return -1;

}
@@ -748,6 +767,7 @@ create_iscsi_disk(const char *bdev_name, const char *url, const char *initiator_ 
		return -ENOMEM;

	}



	req->status = SCSI_STATUS_GOOD;

	req->bdev_name = strdup(bdev_name);

	req->url = strdup(url);

	req->initiator_iqn = strdup(initiator_iqn);