Commit dd8f4270 authored by Konrad Sztyber's avatar Konrad Sztyber
Browse files

keyring: pass module in spdk_keyring_remove_key() 



This ensures that the key will only be removed if the module matches the
module owning the key.  Also, while here, make this function return a
status indicating whether the key was removed successfully.

Signed-off-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
Change-Id: Iaabf863edd87863ddcb4b9b23831672b10eedb07
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/24807


Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
parent a41e6e9a

include/spdk/keyring_module.h

+4 −1
Original line number Diff line number Diff line
@@ -40,8 +40,11 @@ int spdk_keyring_add_key(const struct spdk_key_opts *opts); 
 * Remove a key from the keyring.

 *

 * \param name Name of the key to remove.

 * \param module Module owning the key to remove.

 *

 * \return 0 on success, negative errno otherwise.

 */

void spdk_keyring_remove_key(const char *name);

int spdk_keyring_remove_key(const char *name, struct spdk_keyring_module *module);



struct spdk_keyring_module {

	/** Name of the module */

lib/keyring/Makefile

+1 −1
Original line number Diff line number Diff line
@@ -4,7 +4,7 @@ 
SPDK_ROOT_DIR := $(abspath $(CURDIR)/../..)

include $(SPDK_ROOT_DIR)/mk/spdk.common.mk



SO_VER := 1

SO_VER := 2

SO_MINOR := 0



C_SRCS = keyring.c keyring_rpc.c

lib/keyring/keyring.c

+12 −3
Original line number Diff line number Diff line
@@ -150,21 +150,30 @@ keyring_remove_key(struct spdk_key *key) 
	keyring_put_key(key);

}



void

spdk_keyring_remove_key(const char *name)

int

spdk_keyring_remove_key(const char *name, struct spdk_keyring_module *module)

{

	struct spdk_key *key;

	int rc = 0;



	pthread_mutex_lock(&g_keyring.mutex);

	key = keyring_find_key(name);

	if (key == NULL) {

		SPDK_WARNLOG("Key '%s' does not exist\n", name);

		SPDK_ERRLOG("Key '%s' does not exist\n", name);

		rc = -ENOKEY;

		goto out;

	}



	if (key->module != module) {

		SPDK_ERRLOG("Key '%s' is not owned by module '%s'\n", name, module->name);

		rc = -EINVAL;

		goto out;

	}



	keyring_remove_key(key);

out:

	pthread_mutex_unlock(&g_keyring.mutex);

	return rc;

}



static struct spdk_key *

module/keyring/file/keyring_rpc.c

+1 −1
Original line number Diff line number Diff line
@@ -81,7 +81,7 @@ rpc_keyring_file_remove_key(struct spdk_jsonrpc_request *request, 
		return;

	}



	spdk_keyring_remove_key(req.name);

	spdk_keyring_remove_key(req.name, &g_keyring_file);

	spdk_jsonrpc_send_bool_response(request, true);

	free_rpc_keyring_file_remove_key(&req);

}

test/unit/lib/keyring/keyring.c/keyring_ut.c

+12 −6
Original line number Diff line number Diff line
@@ -85,6 +85,7 @@ test_keyring_add_remove(void) 
	struct spdk_key *key;

	char keybuf[UT_KEY_SIZE] = {}, rcvbuf[UT_KEY_SIZE] = {};

	struct ut_key_opts uopts = { .buf = keybuf, .len = UT_KEY_SIZE };

	struct spdk_keyring_module module2 = { .name = "ut2" };

	int rc;



	/* Add a key */
@@ -106,7 +107,7 @@ test_keyring_add_remove(void) 
	CU_ASSERT_EQUAL(memcmp(rcvbuf, keybuf, UT_KEY_SIZE), 0);



	/* Remove it and try to get another reference */

	spdk_keyring_remove_key("key0");

	spdk_keyring_remove_key("key0", &g_module);

	CU_ASSERT_PTR_NULL(spdk_keyring_get_key("key0"));



	/* Now that the key has been remove spdk_key_get_key() should result in an -ENOKEY error */
@@ -130,7 +131,7 @@ test_keyring_add_remove(void) 
	spdk_keyring_put_key(key);



	/* Remove the key without explicitly specifying global keyring */

	spdk_keyring_remove_key("key0");

	spdk_keyring_remove_key("key0", &g_module);

	CU_ASSERT_PTR_NULL(spdk_keyring_get_key("key0"));

	CU_ASSERT_PTR_NULL(spdk_keyring_get_key(":key0"));
@@ -149,15 +150,20 @@ test_keyring_add_remove(void) 
	rc = spdk_keyring_add_key(&opts);

	CU_ASSERT_EQUAL(rc, -EEXIST);



	spdk_keyring_remove_key(":key0");

	/* Try to remove a key owned by a different module */

	spdk_keyring_remove_key("key0", &module2);

	CU_ASSERT_PTR_NOT_NULL(spdk_keyring_get_key("key0"));

	CU_ASSERT_PTR_NOT_NULL(spdk_keyring_get_key(":key0"));



	spdk_keyring_remove_key(":key0", &g_module);

	CU_ASSERT_PTR_NULL(spdk_keyring_get_key("key0"));

	CU_ASSERT_PTR_NULL(spdk_keyring_get_key(":key0"));

	CU_ASSERT(g_remove_called);

	g_remove_called = false;



	/* Remove an already removed key */

	spdk_keyring_remove_key("key0");

	spdk_keyring_remove_key(":key0");

	spdk_keyring_remove_key("key0", &g_module);

	spdk_keyring_remove_key(":key0", &g_module);

	CU_ASSERT(!g_remove_called);



	/* Check that an error from module's add_key() results in failure */
@@ -200,7 +206,7 @@ test_keyring_get_put(void) 
	/* Remove the key and verify (relying on the address sanitizer to catch any use-after-free

	 * errors) that the reference is still valid

	 */

	spdk_keyring_remove_key("key0");

	spdk_keyring_remove_key("key0", &g_module);

	CU_ASSERT_EQUAL(strcmp(spdk_key_get_name(key), "key0"), 0);



	/* Release all but one reference and verify that it's still valid (again, relying on the