From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 11 Jul 2018 18:57:05 +0300 From: Vladimir Davydov Subject: Re: [tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module Message-ID: <20180711155705.zkwqen2q5aeozzkq@esperanza> References: <8846b566-24c5-9f11-ca29-18baed551921@tarantool.org> <20180711124656.pzuh254bjku3k2xc@esperanza> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180711124656.pzuh254bjku3k2xc@esperanza> To: Kirill Shcherbatov Cc: tarantool-patches@freelists.org List-ID: On Wed, Jul 11, 2018 at 03:46:56PM +0300, Vladimir Davydov wrote: > On Tue, Jul 10, 2018 at 09:49:14AM +0300, Kirill Shcherbatov wrote: > > @@ -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; > > + > > AFAIU a user may reload a whole module only if he has the global EXECUTE > privilege (because we don't have such an entity as module in our data > dictionary to grant access rights for). access_check_func(), which is > called by func_reload(), already checks the global EXECUTE privilege and > returns 0 (success) if it is set, no matter if the function was found or > not. So all you have to do is call module_reload() from func_reload() if > access_check_func() returned func = NULL, no? During our verbal discussion with Kirill, he noticed that the fact that box.schema.func.reload() reloads the whole module when it is passed a function name is rather confusing: the user may not know that and call box.schema.func.reload() once per each used function, in which case he will effectively reload the whole module that contains those functions multiple times, which is pointless. Turns out Kirill isn't the only one who finds such an API weird, see https://github.com/tarantool/tarantool/issues/910#issuecomment-331435227 I talked to Kostja and he doesn't mind banning this API, i.e. making box.schema.func.reload() interpret its argument only as a module name. So let's rework this patch so that box_func_reload() directly calls module_reload(), and remove func_reload() altogether. As for the access checks, box_func_reload() should work only if the caller has the global execute privilege.