From a929fa04171587f4b45b82cbf02547ab2d32ec08 Mon Sep 17 00:00:00 2001 From: yidong0635 Date: Thu, 25 Aug 2022 13:40:40 +0800 Subject: [PATCH] spdk_dd: Fix SEGV on the error path. We can see this app using dd_exit to cleanup. Some error cases: spdk_bdev_get_io_channel failed for -ENOMEM, or it results in dd_open_bdev failed and then dd_exit. For a bdev, dd_exit does cleanup using spdk_put_io_channel. And spdk_put_io_channel allows `ch == NULL`. This causes SEGV. So here add a separate function to check ch, and put cleanup together. This is found by xnvme pre-test. Signed-off-by: yidong0635 Change-Id: I9dd860da250a86f52139e69c690dd257a7ff7717 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/14195 Tested-by: SPDK CI Jenkins Reviewed-by: Tomasz Zawadzki Reviewed-by: Konrad Sztyber Reviewed-by: Ben Walker Reviewed-by: Krzysztof Karas --- app/spdk_dd/spdk_dd.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/app/spdk_dd/spdk_dd.c b/app/spdk_dd/spdk_dd.c index 0305f46c1..7fce247e2 100644 --- a/app/spdk_dd/spdk_dd.c +++ b/app/spdk_dd/spdk_dd.c @@ -144,6 +144,19 @@ static bool g_interrupt; static void dd_target_populate_buffer(struct dd_io *io); +static void +dd_cleanup_bdev(struct dd_target io) +{ + /* This can be only on the error path. + * To prevent the SEGV, need add checks here. + */ + if (io.u.bdev.ch) { + spdk_put_io_channel(io.u.bdev.ch); + } + + spdk_bdev_close(io.u.bdev.desc); +} + static void dd_exit(int rc) { @@ -157,8 +170,7 @@ dd_exit(int rc) close(g_job.input.u.aio.fd); } } else if (g_job.input.type == DD_TARGET_TYPE_BDEV && g_job.input.open) { - spdk_put_io_channel(g_job.input.u.bdev.ch); - spdk_bdev_close(g_job.input.u.bdev.desc); + dd_cleanup_bdev(g_job.input); } if (g_job.output.type == DD_TARGET_TYPE_FILE) { @@ -171,8 +183,7 @@ dd_exit(int rc) close(g_job.output.u.aio.fd); } } else if (g_job.output.type == DD_TARGET_TYPE_BDEV && g_job.output.open) { - spdk_put_io_channel(g_job.output.u.bdev.ch); - spdk_bdev_close(g_job.output.u.bdev.desc); + dd_cleanup_bdev(g_job.output); } if (g_job.input.type == DD_TARGET_TYPE_FILE || g_job.output.type == DD_TARGET_TYPE_FILE) {