diff --git a/scst/src/dev_handlers/kunit/user/suites/scst_user_cdb_test.c b/scst/src/dev_handlers/kunit/user/suites/scst_user_cdb_test.c index 8c1e05e49..ddced41d8 100644 --- a/scst/src/dev_handlers/kunit/user/suites/scst_user_cdb_test.c +++ b/scst/src/dev_handlers/kunit/user/suites/scst_user_cdb_test.c @@ -34,8 +34,96 @@ static void user_cdb_tail(struct kunit *test) } } +struct user_cdb_ref_case { + const char *name; + unsigned int state; + bool sent; + bool background; + bool no_cmd; + bool missing; + bool dead; + int error; +}; + +static const struct user_cdb_ref_case user_cdb_ref_cases[] = { + { .name = "sent", .state = UCMD_STATE_EXECING, .sent = true }, + { .name = "background", .state = UCMD_STATE_EXECING, .background = true }, + { .name = "parsing", .state = UCMD_STATE_PARSING, .sent = true }, + { .name = "no_core_command", .state = UCMD_STATE_EXECING, .sent = true, + .no_cmd = true, .error = -EINVAL }, + { .name = "not_published", .state = UCMD_STATE_EXECING, .error = -EINVAL }, + { .name = "finished", .state = UCMD_STATE_EXECING + 1, .sent = true, + .error = -EINVAL }, + { .name = "missing_handle", .missing = true, .error = -ESRCH }, + { .name = "dying_command", .dead = true, .error = -ESRCH }, +}; + +static void user_cdb_ref_desc(const struct user_cdb_ref_case *p, char *desc) +{ + snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s", p->name); +} + +KUNIT_ARRAY_PARAM(user_cdb_ref, user_cdb_ref_cases, user_cdb_ref_desc); + +static void user_cdb_unexpected_release(struct kref *ref) +{ + KUNIT_FAIL(kunit_get_current_test(), "dropped the fixture's retained reference"); +} + +static void user_cdb_references(struct kunit *test) +{ + const struct user_cdb_ref_case *p = test->param_value; + struct scst_user_dev *dev; + struct scst_user_cmd *ucmd, *result; + struct scst_cmd *cmd, *selected; + unsigned int i; + + dev = kunit_kzalloc(test, sizeof(*dev), GFP_KERNEL); + ucmd = kunit_kzalloc(test, sizeof(*ucmd), GFP_KERNEL); + cmd = kunit_kzalloc(test, sizeof(*cmd), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, dev); + KUNIT_ASSERT_NOT_NULL(test, ucmd); + KUNIT_ASSERT_NOT_NULL(test, cmd); + spin_lock_init(&dev->udev_cmd_threads.cmd_list_lock); + for (i = 0; i < ARRAY_SIZE(dev->ucmd_hash); i++) + INIT_LIST_HEAD(&dev->ucmd_hash[i]); + ucmd->dev = dev; + ucmd->cmd = p->no_cmd ? NULL : cmd; + ucmd->h = 1; + ucmd->state = p->state; + ucmd->sent_to_user = p->sent; + ucmd->background_exec = p->background; + /* The fixture retains one reference; a dying hash entry has zero. */ + if (!p->dead) + kref_init(&ucmd->ucmd_ref); + kref_init(&cmd->cmd_ref); + list_add_tail(&ucmd->hash_list_entry, &dev->ucmd_hash[ucmd->h]); + + selected = cmd; + result = dev_user_ext_cdb_get(dev, p->missing ? 2 : ucmd->h, &selected); + KUNIT_EXPECT_EQ(test, kref_read(&ucmd->ucmd_ref), p->dead ? 0 : (p->error ? 1 : 2)); + KUNIT_EXPECT_EQ(test, kref_read(&cmd->cmd_ref), p->error ? 1 : 2); + if (p->error) { + if (IS_ERR(result)) + KUNIT_EXPECT_EQ(test, PTR_ERR(result), (long)p->error); + else + KUNIT_FAIL(test, "invalid CDB lookup unexpectedly succeeded"); + KUNIT_EXPECT_PTR_EQ(test, selected, NULL); + } else { + KUNIT_ASSERT_FALSE(test, IS_ERR(result)); + KUNIT_EXPECT_PTR_EQ(test, result, ucmd); + KUNIT_EXPECT_PTR_EQ(test, selected, cmd); + kref_put(&cmd->cmd_ref, user_cdb_unexpected_release); + kref_put(&ucmd->ucmd_ref, user_cdb_unexpected_release); + KUNIT_EXPECT_EQ(test, kref_read(&cmd->cmd_ref), 1); + KUNIT_EXPECT_EQ(test, kref_read(&ucmd->ucmd_ref), 1); + } + list_del_init(&ucmd->hash_list_entry); +} + static struct kunit_case user_cdb_cases[] = { KUNIT_CASE(user_cdb_tail), + KUNIT_CASE_PARAM(user_cdb_references, user_cdb_ref_gen_params), {} }; diff --git a/scst/src/dev_handlers/scst_user_main.c b/scst/src/dev_handlers/scst_user_main.c index c329bb3fc..f7db3048e 100644 --- a/scst/src/dev_handlers/scst_user_main.c +++ b/scst/src/dev_handlers/scst_user_main.c @@ -1937,12 +1937,55 @@ const u8 *dev_user_ext_cdb(const struct scst_cmd *cmd) return cmd->cdb + SCST_MAX_CDB_SIZE; } +/* Return paired user/core command references; rejected requests retain neither. */ +SCST_KUNIT_VISIBLE +struct scst_user_cmd *dev_user_ext_cdb_get(struct scst_user_dev *dev, unsigned int cmd_h, + struct scst_cmd **cmd) +{ + struct scst_user_cmd *ucmd; + int res; + + *cmd = NULL; + spin_lock_irq(&dev->udev_cmd_threads.cmd_list_lock); + + ucmd = __ucmd_find_hash(dev, cmd_h); + if (unlikely(!ucmd)) { + TRACE_MGMT_DBG("cmd_h %d not found", cmd_h); + res = -ESRCH; + goto out_unlock; + } + + if (unlikely(ucmd_get_check(ucmd))) { + TRACE_MGMT_DBG("Found being destroyed cmd_h %d", cmd_h); + res = -ESRCH; + goto out_unlock; + } + + if (!ucmd->cmd || ucmd->state > UCMD_STATE_EXECING || + !(ucmd->sent_to_user || ucmd->background_exec)) { + TRACE_MGMT_DBG("Invalid ucmd state %d for cmd_h %d", ucmd->state, cmd_h); + spin_unlock_irq(&dev->udev_cmd_threads.cmd_list_lock); + /* The final put can remove the command from the hash under this lock. */ + ucmd_put(ucmd); + return ERR_PTR(-EINVAL); + } + + *cmd = ucmd->cmd; + scst_cmd_get(*cmd); + spin_unlock_irq(&dev->udev_cmd_threads.cmd_list_lock); + return ucmd; + +out_unlock: + spin_unlock_irq(&dev->udev_cmd_threads.cmd_list_lock); + return ERR_PTR(res); +} + static int dev_user_get_ext_cdb(struct file *file, void __user *arg) { int res = 0, rc; struct scst_user_dev *dev; struct scst_user_cmd *ucmd; - struct scst_cmd *cmd = NULL; + struct scst_cmd *cmd; struct scst_user_get_ext_cdb get; const u8 *ext_cdb; @@ -1964,37 +2007,12 @@ static int dev_user_get_ext_cdb(struct file *file, void __user *arg) TRACE_BUFFER("Get ext cdb", &get, sizeof(get)); - spin_lock_irq(&dev->udev_cmd_threads.cmd_list_lock); - - ucmd = __ucmd_find_hash(dev, get.cmd_h); - if (unlikely(!ucmd)) { - TRACE_MGMT_DBG("cmd_h %d not found", get.cmd_h); - res = -ESRCH; - goto out_unlock; + ucmd = dev_user_ext_cdb_get(dev, get.cmd_h, &cmd); + if (IS_ERR(ucmd)) { + res = PTR_ERR(ucmd); + goto out; } - if (unlikely(ucmd_get_check(ucmd))) { - TRACE_MGMT_DBG("Found being destroyed cmd_h %d", get.cmd_h); - res = -ESRCH; - goto out_unlock; - } - - if (ucmd->cmd && ucmd->state <= UCMD_STATE_EXECING && - (ucmd->sent_to_user || ucmd->background_exec)) { - cmd = ucmd->cmd; - scst_cmd_get(cmd); - } else { - TRACE_MGMT_DBG("Invalid ucmd state %d for cmd_h %d", - ucmd->state, get.cmd_h); - res = -EINVAL; - goto out_unlock; - } - - spin_unlock_irq(&dev->udev_cmd_threads.cmd_list_lock); - - if (!cmd) - goto out_put; - ext_cdb = dev_user_ext_cdb(cmd); if (!ext_cdb) goto out_cmd_put; @@ -2012,17 +2030,11 @@ static int dev_user_get_ext_cdb(struct file *file, void __user *arg) out_cmd_put: scst_cmd_put(cmd); - -out_put: ucmd_put(ucmd); out: TRACE_EXIT_RES(res); return res; - -out_unlock: - spin_unlock_irq(&dev->udev_cmd_threads.cmd_list_lock); - goto out; } static int dev_user_process_scst_commands(struct scst_user_dev *dev) diff --git a/scst/src/dev_handlers/scst_user_test.h b/scst/src/dev_handlers/scst_user_test.h index c909562df..eff55ae34 100644 --- a/scst/src/dev_handlers/scst_user_test.h +++ b/scst/src/dev_handlers/scst_user_test.h @@ -8,6 +8,8 @@ bool ucmd_get_check(struct scst_user_cmd *ucmd); const u8 *dev_user_ext_cdb(const struct scst_cmd *cmd); +struct scst_user_cmd *dev_user_ext_cdb_get(struct scst_user_dev *dev, unsigned int cmd_h, + struct scst_cmd **cmd); void dev_user_unjam_cmd(struct scst_user_cmd *ucmd, int busy, unsigned long *flags); void dev_user_flush_pool(struct sgv_pool *pool); int dev_user_process_cleanup(struct scst_user_dev *dev);