| Message ID | 20230327161251.1129511-1-vmalik@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [bpf-next] kallsyms: move module-related functions under correct configs | expand |
On Mon, Mar 27, 2023 at 06:12:51PM +0200, Viktor Malik wrote: > Functions for searching module kallsyms should have non-empty > definitions only if CONFIG_MODULES=y and CONFIG_KALLSYMS=y. Until now, > only CONFIG_MODULES check was used for many of these, which may have > caused complilation errors on some configs. > > This patch moves all relevant functions under the correct configs. > > Signed-off-by: Viktor Malik <vmalik@redhat.com> > Reported-by: kernel test robot <lkp@intel.com> > Link: https://lore.kernel.org/oe-kbuild-all/202303181535.RFDCnz3E-lkp@intel.com/ Thanks Viktor! Does this fix something from an existing commit? If so which one? The commit log should mention it. LUis
On 3/27/23 19:47, Luis Chamberlain wrote: > On Mon, Mar 27, 2023 at 06:12:51PM +0200, Viktor Malik wrote: >> Functions for searching module kallsyms should have non-empty >> definitions only if CONFIG_MODULES=y and CONFIG_KALLSYMS=y. Until now, >> only CONFIG_MODULES check was used for many of these, which may have >> caused complilation errors on some configs. >> >> This patch moves all relevant functions under the correct configs. >> >> Signed-off-by: Viktor Malik <vmalik@redhat.com> >> Reported-by: kernel test robot <lkp@intel.com> >> Link: https://lore.kernel.org/oe-kbuild-all/202303181535.RFDCnz3E-lkp@intel.com/ > > Thanks Viktor! Does this fix something from an existing commit? If so > which one? The commit log should mention it. Ah, right, I forgot about that. The commit log is missing: Fixes: bd5314f8dd2d ("kallsyms, bpf: Move find_kallsyms_symbol_value out of internal header") I can post v2 but I'm also fine with maintainers applying the tag. Thanks! Viktor > > LUis >
On Mon, Mar 27, 2023 at 08:20:56PM +0200, Viktor Malik wrote: > On 3/27/23 19:47, Luis Chamberlain wrote: > > On Mon, Mar 27, 2023 at 06:12:51PM +0200, Viktor Malik wrote: > > > Functions for searching module kallsyms should have non-empty > > > definitions only if CONFIG_MODULES=y and CONFIG_KALLSYMS=y. Until now, > > > only CONFIG_MODULES check was used for many of these, which may have > > > caused complilation errors on some configs. > > > > > > This patch moves all relevant functions under the correct configs. > > > > > > Signed-off-by: Viktor Malik <vmalik@redhat.com> > > > Reported-by: kernel test robot <lkp@intel.com> > > > Link: https://lore.kernel.org/oe-kbuild-all/202303181535.RFDCnz3E-lkp@intel.com/ > > > > Thanks Viktor! Does this fix something from an existing commit? If so > > which one? The commit log should mention it. > > Ah, right, I forgot about that. The commit log is missing: > > Fixes: bd5314f8dd2d ("kallsyms, bpf: Move find_kallsyms_symbol_value out of internal header") > > I can post v2 but I'm also fine with maintainers applying the tag. That patch went through the bpf tree so its fix can go throug that tree. So up to Daniel if he wants a new patch. Luis
On 3/27/23 9:28 PM, Luis Chamberlain wrote: > On Mon, Mar 27, 2023 at 08:20:56PM +0200, Viktor Malik wrote: >> On 3/27/23 19:47, Luis Chamberlain wrote: >>> On Mon, Mar 27, 2023 at 06:12:51PM +0200, Viktor Malik wrote: >>>> Functions for searching module kallsyms should have non-empty >>>> definitions only if CONFIG_MODULES=y and CONFIG_KALLSYMS=y. Until now, >>>> only CONFIG_MODULES check was used for many of these, which may have >>>> caused complilation errors on some configs. >>>> >>>> This patch moves all relevant functions under the correct configs. >>>> >>>> Signed-off-by: Viktor Malik <vmalik@redhat.com> >>>> Reported-by: kernel test robot <lkp@intel.com> >>>> Link: https://lore.kernel.org/oe-kbuild-all/202303181535.RFDCnz3E-lkp@intel.com/ >>> >>> Thanks Viktor! Does this fix something from an existing commit? If so >>> which one? The commit log should mention it. >> >> Ah, right, I forgot about that. The commit log is missing: >> >> Fixes: bd5314f8dd2d ("kallsyms, bpf: Move find_kallsyms_symbol_value out of internal header") >> >> I can post v2 but I'm also fine with maintainers applying the tag. > > That patch went through the bpf tree so its fix can go throug that tree. > So up to Daniel if he wants a new patch. Fixing up is fine with me. Viktor, which config combinations did you test for this patch and under which architectures? I suspect kbuild bot might still complain. For example, your patch moves dereference_module_function_descriptor() stub definition under !CONFIG_MODULES || !CONFIG_KALLSYMS. Looking at ppc's dereference_module_function_descriptor() implementation (!CONFIG_PPC64_ELF_ABI_V2 case), it's build under CONFIG_MODULES, so you'll have two different implementations of the functions, the generic one then w/o __weak attribute. Also small nit, please fix the patch up wrt indentation to align with kernel coding style. Thanks, Daniel
On 3/28/23 10:23, Daniel Borkmann wrote: > On 3/27/23 9:28 PM, Luis Chamberlain wrote: >> On Mon, Mar 27, 2023 at 08:20:56PM +0200, Viktor Malik wrote: >>> On 3/27/23 19:47, Luis Chamberlain wrote: >>>> On Mon, Mar 27, 2023 at 06:12:51PM +0200, Viktor Malik wrote: >>>>> Functions for searching module kallsyms should have non-empty >>>>> definitions only if CONFIG_MODULES=y and CONFIG_KALLSYMS=y. Until now, >>>>> only CONFIG_MODULES check was used for many of these, which may have >>>>> caused complilation errors on some configs. >>>>> >>>>> This patch moves all relevant functions under the correct configs. >>>>> >>>>> Signed-off-by: Viktor Malik <vmalik@redhat.com> >>>>> Reported-by: kernel test robot <lkp@intel.com> >>>>> Link: https://lore.kernel.org/oe-kbuild-all/202303181535.RFDCnz3E-lkp@intel.com/ >>>> >>>> Thanks Viktor! Does this fix something from an existing commit? If so >>>> which one? The commit log should mention it. >>> >>> Ah, right, I forgot about that. The commit log is missing: >>> >>> Fixes: bd5314f8dd2d ("kallsyms, bpf: Move find_kallsyms_symbol_value out of internal header") >>> >>> I can post v2 but I'm also fine with maintainers applying the tag. >> >> That patch went through the bpf tree so its fix can go throug that tree. >> So up to Daniel if he wants a new patch. > > Fixing up is fine with me. Viktor, which config combinations did you test for this > patch and under which architectures? > > I suspect kbuild bot might still complain. For example, your patch moves > dereference_module_function_descriptor() stub definition under !CONFIG_MODULES || > !CONFIG_KALLSYMS. Looking at ppc's dereference_module_function_descriptor() > implementation (!CONFIG_PPC64_ELF_ABI_V2 case), it's build under CONFIG_MODULES, > so you'll have two different implementations of the functions, the generic one > then w/o __weak attribute. Right, I didn't notice this one, thanks. Looks like dereference_module_function_descriptor will need to stay in the original place (under CONFIG_MODULES only). I'll cross-compile for arches that redefine this function (powerpc, ia64, parisc) with relevant config combinations and submit v2. The rest of the functions don't have arch-dependent implementations, so they should be fine. > > Also small nit, please fix the patch up wrt indentation to align with kernel coding > style. Ok, will do. Thanks, Viktor > > Thanks, > Daniel >
diff --git a/include/linux/module.h b/include/linux/module.h index 41cfd3be57e5..eaad4b7a3788 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -608,16 +608,6 @@ static inline bool within_module(unsigned long addr, const struct module *mod) /* Search for module by name: must be in a RCU-sched critical section. */ struct module *find_module(const char *name); -/* Returns 0 and fills in value, defined and namebuf, or -ERANGE if - symnum out of range. */ -int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type, - char *name, char *module_name, int *exported); - -/* Look for this name: can be of form module:name. */ -unsigned long module_kallsyms_lookup_name(const char *name); - -unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name); - extern void __noreturn __module_put_and_kthread_exit(struct module *mod, long code); #define module_put_and_kthread_exit(code) __module_put_and_kthread_exit(THIS_MODULE, code) @@ -661,20 +651,6 @@ static inline void __module_get(struct module *module) __mod ? __mod->name : "kernel"; \ }) -/* Dereference module function descriptor */ -void *dereference_module_function_descriptor(struct module *mod, void *ptr); - -/* For kallsyms to ask for address resolution. namebuf should be at - * least KSYM_NAME_LEN long: a pointer to namebuf is returned if - * found, otherwise NULL. */ -const char *module_address_lookup(unsigned long addr, - unsigned long *symbolsize, - unsigned long *offset, - char **modname, const unsigned char **modbuildid, - char *namebuf); -int lookup_module_symbol_name(unsigned long addr, char *symname); -int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name); - int register_module_notifier(struct notifier_block *nb); int unregister_module_notifier(struct notifier_block *nb); @@ -765,45 +741,6 @@ static inline void module_put(struct module *module) #define module_name(mod) "kernel" -/* For kallsyms to ask for address resolution. NULL means not found. */ -static inline const char *module_address_lookup(unsigned long addr, - unsigned long *symbolsize, - unsigned long *offset, - char **modname, - const unsigned char **modbuildid, - char *namebuf) -{ - return NULL; -} - -static inline int lookup_module_symbol_name(unsigned long addr, char *symname) -{ - return -ERANGE; -} - -static inline int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name) -{ - return -ERANGE; -} - -static inline int module_get_kallsym(unsigned int symnum, unsigned long *value, - char *type, char *name, - char *module_name, int *exported) -{ - return -ERANGE; -} - -static inline unsigned long module_kallsyms_lookup_name(const char *name) -{ - return 0; -} - -static inline unsigned long find_kallsyms_symbol_value(struct module *mod, - const char *name) -{ - return 0; -} - static inline int register_module_notifier(struct notifier_block *nb) { /* no events will happen anyway, so this can always succeed */ @@ -831,13 +768,6 @@ static inline void set_module_sig_enforced(void) { } -/* Dereference module function descriptor */ -static inline -void *dereference_module_function_descriptor(struct module *mod, void *ptr) -{ - return ptr; -} - #endif /* CONFIG_MODULES */ #ifdef CONFIG_SYSFS @@ -899,7 +829,39 @@ int module_kallsyms_on_each_symbol(const char *modname, int (*fn)(void *, const char *, struct module *, unsigned long), void *data); -#else + +/* Dereference module function descriptor */ +void *dereference_module_function_descriptor(struct module *mod, void *ptr); + +/* For kallsyms to ask for address resolution. namebuf should be at + * least KSYM_NAME_LEN long: a pointer to namebuf is returned if + * found, otherwise NULL. + */ +const char *module_address_lookup(unsigned long addr, + unsigned long *symbolsize, + unsigned long *offset, + char **modname, const unsigned char **modbuildid, + char *namebuf); +int lookup_module_symbol_name(unsigned long addr, char *symname); +int lookup_module_symbol_attrs(unsigned long addr, + unsigned long *size, + unsigned long *offset, + char *modname, + char *name); + +/* Returns 0 and fills in value, defined and namebuf, or -ERANGE if + * symnum out of range. + */ +int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type, + char *name, char *module_name, int *exported); + +/* Look for this name: can be of form module:name. */ +unsigned long module_kallsyms_lookup_name(const char *name); + +unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name); + +#else /* CONFIG_MODULES && CONFIG_KALLSYMS */ + static inline int module_kallsyms_on_each_symbol(const char *modname, int (*fn)(void *, const char *, struct module *, unsigned long), @@ -907,6 +869,57 @@ static inline int module_kallsyms_on_each_symbol(const char *modname, { return -EOPNOTSUPP; } + +/* Dereference module function descriptor */ +static inline +void *dereference_module_function_descriptor(struct module *mod, void *ptr) +{ + return ptr; +} + +/* For kallsyms to ask for address resolution. NULL means not found. */ +static inline const char *module_address_lookup(unsigned long addr, + unsigned long *symbolsize, + unsigned long *offset, + char **modname, + const unsigned char **modbuildid, + char *namebuf) +{ + return NULL; +} + +static inline int lookup_module_symbol_name(unsigned long addr, char *symname) +{ + return -ERANGE; +} + +static inline int lookup_module_symbol_attrs(unsigned long addr, + unsigned long *size, + unsigned long *offset, + char *modname, + char *name) +{ + return -ERANGE; +} + +static inline int module_get_kallsym(unsigned int symnum, unsigned long *value, + char *type, char *name, + char *module_name, int *exported) +{ + return -ERANGE; +} + +static inline unsigned long module_kallsyms_lookup_name(const char *name) +{ + return 0; +} + +static inline unsigned long find_kallsyms_symbol_value(struct module *mod, + const char *name) +{ + return 0; +} + #endif /* CONFIG_MODULES && CONFIG_KALLSYMS */ #endif /* _LINUX_MODULE_H */
Functions for searching module kallsyms should have non-empty definitions only if CONFIG_MODULES=y and CONFIG_KALLSYMS=y. Until now, only CONFIG_MODULES check was used for many of these, which may have caused complilation errors on some configs. This patch moves all relevant functions under the correct configs. Signed-off-by: Viktor Malik <vmalik@redhat.com> Reported-by: kernel test robot <lkp@intel.com> Link: https://lore.kernel.org/oe-kbuild-all/202303181535.RFDCnz3E-lkp@intel.com/ --- include/linux/module.h | 155 ++++++++++++++++++++++------------------- 1 file changed, 84 insertions(+), 71 deletions(-)