Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@dev.tarantool.org, kyukhin@tarantool.org,
	avtikhon@tarantool.org
Subject: [Tarantool-patches] [PATCH 1/1] func: fix use after free on function unload
Date: Wed,  6 Nov 2019 16:12:09 +0300	[thread overview]
Message-ID: <8a0ed2a5af17c887c8ca13f406852f76f742adb4.1573045803.git.v.shpilevoy@tarantool.org> (raw)

Functions are stored in lists inside module objects. Module
objects are stored in a hash table, where key is a package name.
But the key was a pointer at one of module's function definition
object. Therefore, when that function was deleted, its freed
package name memory was still in the hash key, and could be
accessed, when another function was deleted.

Now module does not use memory of its functions, and keep a copy
of the package name.
---
Branch: https://github.com/tarantool/tarantool/tree/gerold103/func-unload-use-after-free
No issue.

It should be pushed to all branched down to 1.10.

 src/box/func.c | 25 ++++++++++++++-----------
 src/box/func.h |  2 ++
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/src/box/func.c b/src/box/func.c
index 7ac92bc84..d9481b714 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -209,12 +209,12 @@ module_cache_find(const char *name, const char *name_end)
  * Save module to the module cache.
  */
 static inline int
-module_cache_put(const char *name, const char *name_end, struct module *module)
+module_cache_put(struct module *module)
 {
-	size_t name_len = name_end - name;
-	uint32_t name_hash = mh_strn_hash(name, name_len);
+	size_t package_len = strlen(module->package);
+	uint32_t name_hash = mh_strn_hash(module->package, package_len);
 	const struct mh_strnptr_node_t strnode = {
-		name, name_len, name_hash, module};
+		module->package, package_len, name_hash, module};
 
 	if (mh_strnptr_put(modules, &strnode, NULL, NULL) == mh_end(modules)) {
 		diag_set(OutOfMemory, sizeof(strnode), "malloc", "modules");
@@ -248,12 +248,16 @@ module_load(const char *package, const char *package_end)
 	if (module_find(package, package_end, path, sizeof(path)) != 0)
 		return NULL;
 
-	struct module *module = (struct module *) malloc(sizeof(*module));
+	int package_len = package_end - package;
+	struct module *module = (struct module *)
+		malloc(sizeof(*module) + package_len + 1);
 	if (module == NULL) {
-		diag_set(OutOfMemory, sizeof(struct module), "malloc",
-			 "struct module");
+		diag_set(OutOfMemory, sizeof(struct module) + package_len + 1,
+			 "malloc", "struct module");
 		return NULL;
 	}
+	memcpy(module->package, package, package_len);
+	module->package[package_len] = 0;
 	rlist_create(&module->funcs);
 	module->calls = 0;
 	module->is_unloading = false;
@@ -264,7 +268,7 @@ module_load(const char *package, const char *package_end)
 	}
 	char load_name[PATH_MAX + 1];
 	snprintf(load_name, sizeof(load_name), "%s/%.*s." TARANTOOL_LIBEXT,
-		 dir_name, (int)(package_end - package), package);
+		 dir_name, package_len, package);
 	if (symlink(path, load_name) < 0) {
 		diag_set(SystemError, "failed to create dso link");
 		goto error;
@@ -275,7 +279,6 @@ module_load(const char *package, const char *package_end)
 	if (rmdir(dir_name) != 0)
 		say_warn("failed to delete temporary dir %s", dir_name);
 	if (module->handle == NULL) {
-		int package_len = (int) (package_end - package_end);
 		diag_set(ClientError, ER_LOAD_MODULE, package_len,
 			  package, dlerror());
 		goto error;
@@ -346,7 +349,7 @@ module_reload(const char *package, const char *package_end, struct module **modu
 		rlist_move(&new_module->funcs, &func->item);
 	}
 	module_cache_del(package, package_end);
-	if (module_cache_put(package, package_end, new_module) != 0)
+	if (module_cache_put(new_module) != 0)
 		goto restore;
 	old_module->is_unloading = true;
 	module_gc(old_module);
@@ -486,7 +489,7 @@ func_c_load(struct func_c *func)
 		module = module_load(name.package, name.package_end);
 		if (module == NULL)
 			return -1;
-		if (module_cache_put(name.package, name.package_end, module)) {
+		if (module_cache_put(module)) {
 			module_delete(module);
 			return -1;
 		}
diff --git a/src/box/func.h b/src/box/func.h
index 2236fd873..18b83faac 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -56,6 +56,8 @@ struct module {
 	size_t calls;
 	/** True if module is being unloaded. */
 	bool is_unloading;
+	/** Module's package name. */
+	char package[0];
 };
 
 /** Virtual method table for func object. */
-- 
2.21.0 (Apple Git-122.2)

             reply	other threads:[~2019-11-06 13:06 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-06 13:12 Vladislav Shpilevoy [this message]
2019-11-21 17:30 ` Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8a0ed2a5af17c887c8ca13f406852f76f742adb4.1573045803.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=avtikhon@tarantool.org \
    --cc=kyukhin@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 1/1] func: fix use after free on function unload' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox