From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id A3DAC22846 for ; Tue, 10 Jul 2018 02:49:20 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 2vTiGEpZ7pCK for ; Tue, 10 Jul 2018 02:49:20 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id EDEE32282A for ; Tue, 10 Jul 2018 02:49:19 -0400 (EDT) From: Kirill Shcherbatov Subject: [tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module Date: Tue, 10 Jul 2018 09:49:14 +0300 Message-Id: In-Reply-To: <8846b566-24c5-9f11-ca29-18baed551921@tarantool.org> References: <8846b566-24c5-9f11-ca29-18baed551921@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Kirill Shcherbatov Tnx for comment; I've changed commit message and fixed few codestyle things which I missed yesterday. Closes #2946. @TarantoolBot document Title: fixed module reload There was a bug in tarantool documentation: https://tarantool.io/en/doc/1.7/book/box/ box_schema/#lua-function.box.schema.func.reload Now it is allowed to reload all functions in loadable module via one method. box.schema.func.reload("utils") -- ok since now Global reload is still unsupported because it seems to be useless. box.schema.func.reload() -- invalid! --- https://github.com/tarantool/tarantool/compare/kshch/gh-2946-module-reload https://github.com/tarantool/tarantool/issues/2946 src/box/call.c | 37 ++++++++++++++++--------------------- src/box/call.h | 18 ++++++++++++++++++ src/box/func.c | 20 +++++++++++++++++--- src/box/func.h | 15 +++++++++++++++ test/box/func_reload.result | 7 +++++++ test/box/func_reload.test.lua | 2 ++ 6 files changed, 75 insertions(+), 24 deletions(-) diff --git a/src/box/call.c b/src/box/call.c index 438be19..ef30abc 100644 --- a/src/box/call.c +++ b/src/box/call.c @@ -42,20 +42,8 @@ #include "rmean.h" #include "small/obuf.h" -/** - * Find a function by name and check "EXECUTE" permissions. - * - * @param name function name - * @param name_len length of @a name - * @param[out] funcp function object - * Sic: *pfunc == NULL means that perhaps the user has a global - * "EXECUTE" privilege, so no specific grant to a function. - * - * @retval -1 on access denied - * @retval 0 on success - */ -static inline int -access_check_func(const char *name, uint32_t name_len, struct func **funcp) +int +box_func_check_access(const char *name, uint32_t name_len, struct func **funcp) { struct func *func = func_by_name(name, name_len); struct credentials *credentials = effective_user(); @@ -136,17 +124,24 @@ box_func_reload(const char *name) { size_t name_len = strlen(name); struct func *func = NULL; - if ((access_check_func(name, name_len, &func)) != 0) + if ((box_func_check_access(name, name_len, &func)) != 0) return -1; if (func == NULL) { + /* Try to interpret name as a module name. */ + struct module *module; + if (module_reload(name, name + name_len, &module, true) == 0 && + module != NULL) + return 0; diag_set(ClientError, ER_NO_SUCH_FUNCTION, name); return -1; + } else { + /* Such function exists. Try to reload it. */ + if (func->def->language != FUNC_LANGUAGE_C || func->func == NULL) + return 0; + if (func_reload(func) == 0) + return 0; + return -1; } - if (func->def->language != FUNC_LANGUAGE_C || func->func == NULL) - return 0; /* Nothing to do */ - if (func_reload(func) == 0) - return 0; - return -1; } int @@ -165,7 +160,7 @@ box_process_call(struct call_request *request, struct port *port) * Sic: func == NULL means that perhaps the user has a global * "EXECUTE" privilege, so no specific grant to a function. */ - if (access_check_func(name, name_len, &func) != 0) + if (box_func_check_access(name, name_len, &func) != 0) return -1; /* permission denied */ /** diff --git a/src/box/call.h b/src/box/call.h index eabba69..55cf925 100644 --- a/src/box/call.h +++ b/src/box/call.h @@ -35,8 +35,11 @@ extern "C" { #endif /* defined(__cplusplus) */ +#include + struct port; struct call_request; +struct func; struct box_function_ctx { struct port *port; @@ -51,6 +54,21 @@ box_process_call(struct call_request *request, struct port *port); int box_process_eval(struct call_request *request, struct port *port); +/** + * Find a function by name and check "EXECUTE" permissions. + * + * @param name function name + * @param name_len length of @a name + * @param[out] funcp function object + * Sic: *pfunc == NULL means that perhaps the user has a global + * "EXECUTE" privilege, so no specific grant to a function. + * + * @retval -1 on access denied. + * @retval 0 on success. + */ +int +box_func_check_access(const char *name, uint32_t name_len, struct func **funcp); + #if defined(__cplusplus) } /* extern "C" */ #endif /* defined(__cplusplus) */ diff --git a/src/box/func.c b/src/box/func.c index dfbc5f3..245dbe7 100644 --- a/src/box/func.c +++ b/src/box/func.c @@ -34,6 +34,7 @@ #include "lua/utils.h" #include "error.h" #include "diag.h" +#include "call.h" #include /** @@ -302,7 +303,8 @@ module_sym(struct module *module, const char *name) * Reload a dso. */ int -module_reload(const char *package, const char *package_end, struct module **module) +module_reload(const char *package, const char *package_end, + struct module **module, bool check_access) { struct module *old_module = module_cache_find(package, package_end); if (old_module == NULL) { @@ -318,7 +320,19 @@ module_reload(const char *package, const char *package_end, struct module **modu struct func *func, *tmp_func; rlist_foreach_entry_safe(func, &old_module->funcs, item, tmp_func) { struct func_name name; - func_split_name(func->def->name, &name); + const char *func_name = func->def->name; + func_split_name(func_name, &name); + + /* + * Allow to reload only functions that belongs to + * current user. Skip other. + */ + struct func *dummy; + if (check_access && + box_func_check_access(func_name, strlen(func_name), + &dummy) != 0) + continue; + func->func = module_sym(new_module, name.sym); if (func->func == NULL) goto restore; @@ -441,7 +455,7 @@ func_reload(struct func *func) struct func_name name; func_split_name(func->def->name, &name); struct module *module = NULL; - if (module_reload(name.package, name.package_end, &module) != 0) { + if (module_reload(name.package, name.package_end, &module, false) != 0) { diag_log(); return -1; } diff --git a/src/box/func.h b/src/box/func.h index 0957546..168c2fc 100644 --- a/src/box/func.h +++ b/src/box/func.h @@ -116,6 +116,21 @@ func_call(struct func *func, box_function_ctx_t *ctx, const char *args, int func_reload(struct func *func); +/** + * Reload dynamically loadable module. + * + * @param package name begin pointer. + * @param package_end name end pointer. + * @param[out] module a pointer to store module object on success. + * @param check_access do ACL checks if specified. + * + * @retval -1 on error + * @retval 0 on success + */ +int +module_reload(const char *package, const char *package_end, + struct module **module, bool check_access); + #if defined(__cplusplus) } /* extern "C" */ #endif /* defined(__cplusplus) */ diff --git a/test/box/func_reload.result b/test/box/func_reload.result index 5aeb85c..9c05555 100644 --- a/test/box/func_reload.result +++ b/test/box/func_reload.result @@ -54,6 +54,10 @@ fio.symlink(reload1_path, reload_path) box.schema.func.reload("reload.foo") --- ... +box.schema.func.reload("reload") +--- +- error: Function 'reload' does not exist +... -- test of usual case reload. No hanging calls box.space.test:insert{0} --- @@ -77,6 +81,9 @@ fio.symlink(reload2_path, reload_path) box.schema.func.reload("reload.foo") --- ... +box.schema.func.reload("reload") +--- +... c:call("reload.foo") --- - [] diff --git a/test/box/func_reload.test.lua b/test/box/func_reload.test.lua index dc56d84..5569dcd 100644 --- a/test/box/func_reload.test.lua +++ b/test/box/func_reload.test.lua @@ -21,6 +21,7 @@ fio.symlink(reload1_path, reload_path) --check not fail on non-load func box.schema.func.reload("reload.foo") +box.schema.func.reload("reload") -- test of usual case reload. No hanging calls box.space.test:insert{0} @@ -30,6 +31,7 @@ _ = fio.unlink(reload_path) fio.symlink(reload2_path, reload_path) box.schema.func.reload("reload.foo") +box.schema.func.reload("reload") c:call("reload.foo") box.space.test:select{} box.space.test:truncate() -- 2.7.4