From d9a11fd5b150af12228cb6b5082904b5a35cea2d Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Thu, 16 Apr 2020 10:10:47 -0400 Subject: [PATCH] cuse: fix nvme_cuse unregister segfault Unregistering nvme_cuse when the device did not exist resulted in SEGFAULT within nvme_io_msg_ctrlr_unregister(). To prevent that, when no nvme_cuse is registered for the ctrlr do not unregister nvme_io_msg_producer. RPC and spdk_nvme_cuse_unregister() now return an error. Signed-off-by: Tomasz Zawadzki Change-Id: Id77cebe23ff91023a24cfe091f5f62a76a9175fd Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1921 Community-CI: Mellanox Build Bot Community-CI: Broadcom CI Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- include/spdk/nvme.h | 3 ++- lib/nvme/nvme_cuse.c | 14 ++++++++++++-- module/bdev/nvme/bdev_nvme_cuse_rpc.c | 7 ++++++- test/nvme/cuse/nvme_cuse.sh | 3 +++ 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/include/spdk/nvme.h b/include/spdk/nvme.h index 55118e9b0..939b8c67d 100644 --- a/include/spdk/nvme.h +++ b/include/spdk/nvme.h @@ -3044,8 +3044,9 @@ int spdk_nvme_cuse_register(struct spdk_nvme_ctrlr *ctrlr); * * \param ctrlr Opaque handle to the NVMe controller. * + * \return 0 on success. Negated errno on failure. */ -void spdk_nvme_cuse_unregister(struct spdk_nvme_ctrlr *ctrlr); +int spdk_nvme_cuse_unregister(struct spdk_nvme_ctrlr *ctrlr); int spdk_nvme_map_prps(void *prv, struct spdk_nvme_cmd *cmd, struct iovec *iovs, uint32_t len, size_t mps, diff --git a/lib/nvme/nvme_cuse.c b/lib/nvme/nvme_cuse.c index 08fbfc56c..b1084f22b 100644 --- a/lib/nvme/nvme_cuse.c +++ b/lib/nvme/nvme_cuse.c @@ -942,12 +942,22 @@ spdk_nvme_cuse_register(struct spdk_nvme_ctrlr *ctrlr) return rc; } -void +int spdk_nvme_cuse_unregister(struct spdk_nvme_ctrlr *ctrlr) { - nvme_cuse_stop(ctrlr); + struct cuse_device *ctrlr_device; + + ctrlr_device = nvme_cuse_get_cuse_ctrlr_device(ctrlr); + if (!ctrlr_device) { + SPDK_ERRLOG("Cannot find associated CUSE device\n"); + return -ENODEV; + } + + cuse_nvme_ctrlr_stop(ctrlr_device); nvme_io_msg_ctrlr_unregister(ctrlr, &cuse_nvme_io_msg_producer); + + return 0; } char * diff --git a/module/bdev/nvme/bdev_nvme_cuse_rpc.c b/module/bdev/nvme/bdev_nvme_cuse_rpc.c index a8765ec39..26cfd558f 100644 --- a/module/bdev/nvme/bdev_nvme_cuse_rpc.c +++ b/module/bdev/nvme/bdev_nvme_cuse_rpc.c @@ -118,6 +118,7 @@ spdk_rpc_nvme_cuse_unregister(struct spdk_jsonrpc_request *request, struct rpc_nvme_cuse_unregister req = {}; struct spdk_json_write_ctx *w; struct nvme_bdev_ctrlr *bdev_ctrlr = NULL; + int rc; if (spdk_json_decode_object(params, rpc_nvme_cuse_unregister_decoders, SPDK_COUNTOF(rpc_nvme_cuse_unregister_decoders), @@ -135,7 +136,11 @@ spdk_rpc_nvme_cuse_unregister(struct spdk_jsonrpc_request *request, goto cleanup; } - spdk_nvme_cuse_unregister(bdev_ctrlr->ctrlr); + rc = spdk_nvme_cuse_unregister(bdev_ctrlr->ctrlr); + if (rc) { + spdk_jsonrpc_send_error_response(request, rc, spdk_strerror(-rc)); + goto cleanup; + } w = spdk_jsonrpc_begin_result(request); spdk_json_write_bool(w, true); diff --git a/test/nvme/cuse/nvme_cuse.sh b/test/nvme/cuse/nvme_cuse.sh index 053932740..0865081d2 100755 --- a/test/nvme/cuse/nvme_cuse.sh +++ b/test/nvme/cuse/nvme_cuse.sh @@ -36,6 +36,9 @@ if [ -c "$ctrlr" ]; then exit 1 fi +# Verify removing non-existent cuse device +$rpc_py bdev_nvme_cuse_unregister -n Nvme0 && false + $rpc_py bdev_nvme_cuse_register -n Nvme0 sleep 1