diff mbox series

[bpf-next,v5,1/9] kernel: Add kallsyms_on_each_symbol variant for single module

Message ID 20211230023705.3860970-2-memxor@gmail.com
State Awaiting Upstream
Delegated to: Pablo Neira
Headers show
Series Introduce unstable CT lookup helpers | expand

Commit Message

Kumar Kartikeya Dwivedi Dec. 30, 2021, 2:36 a.m. UTC 
The module_kallsyms_on_each_symbol function iterates over symbols of all
modules. To implement BTF ID set processing in each module's BTF parsing
routine, we need a variant that can iterate over a single module's
symbols. To implement this, extract the single module functionality out
of module_kallsyms_on_each_symbol, and rename the old function to
module_kallsyms_on_each_symbol_all.

Then, the new module_kallsyms_on_each_symbol which iterates over a
single module's symbols uses this extracted helper with appropriate
locking.

Next commit will make use of it to implement BTF ID set concatentation
per hook and type.

Also, since we'll be using kallsyms_on_each_symbol for vmlinux BTF
parsing, remove its dependency on CONFIG_LIVEPATCH.

Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Miroslav Benes <mbenes@suse.cz>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Joe Lawrence <joe.lawrence@redhat.com>
Cc: live-patching@vger.kernel.org
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 include/linux/kallsyms.h | 11 ++++++-
 include/linux/module.h   | 37 +++++++++++++++++++++++-
 kernel/kallsyms.c        |  4 +--
 kernel/livepatch/core.c  |  2 +-
 kernel/module.c          | 62 ++++++++++++++++++++++++++++++----------
 5 files changed, 95 insertions(+), 21 deletions(-)

Comments

kernel test robot Jan. 1, 2022, 10:03 a.m. UTC | #1 
Hi Kumar,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Kumar-Kartikeya-Dwivedi/Introduce-unstable-CT-lookup-helpers/20211230-103958
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: ia64-randconfig-s031-20211230 (https://download.01.org/0day-ci/archive/20220101/202201011858.vOPCCeDV-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 11.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/25d6b438335605e4e002f7afde50a3eaf17a0b6c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Kumar-Kartikeya-Dwivedi/Introduce-unstable-CT-lookup-helpers/20211230-103958
        git checkout 25d6b438335605e4e002f7afde50a3eaf17a0b6c
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=ia64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   kernel/module.c:2762:23: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct mod_kallsyms [noderef] __rcu *kallsyms @@     got void * @@
   kernel/module.c:2762:23: sparse:     expected struct mod_kallsyms [noderef] __rcu *kallsyms
   kernel/module.c:2762:23: sparse:     got void *
>> kernel/module.c:4510:18: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct mod_kallsyms *kallsyms @@     got struct mod_kallsyms [noderef] __rcu *kallsyms @@
   kernel/module.c:4510:18: sparse:     expected struct mod_kallsyms *kallsyms
   kernel/module.c:4510:18: sparse:     got struct mod_kallsyms [noderef] __rcu *kallsyms
   kernel/module.c:2764:12: sparse: sparse: dereference of noderef expression
   kernel/module.c:2765:12: sparse: sparse: dereference of noderef expression
   kernel/module.c:2767:12: sparse: sparse: dereference of noderef expression
   kernel/module.c:2768:12: sparse: sparse: dereference of noderef expression
   kernel/module.c:2777:18: sparse: sparse: dereference of noderef expression
   kernel/module.c:2778:35: sparse: sparse: dereference of noderef expression
   kernel/module.c:2779:20: sparse: sparse: dereference of noderef expression
   kernel/module.c:2784:32: sparse: sparse: dereference of noderef expression
   kernel/module.c:2787:45: sparse: sparse: dereference of noderef expression

vim +4510 kernel/module.c

  4499	
  4500	int module_kallsyms_on_each_symbol(struct module *mod,
  4501					   int (*fn)(void *, const char *,
  4502						     struct module *, unsigned long),
  4503					   void *data)
  4504	{
  4505		struct mod_kallsyms *kallsyms;
  4506		int ret = 0;
  4507	
  4508		mutex_lock(&module_mutex);
  4509		/* We hold module_mutex: no need for rcu_dereference_sched */
> 4510		kallsyms = mod->kallsyms;
  4511		if (mod->state != MODULE_STATE_UNFORMED)
  4512			ret = __module_kallsyms_on_each_symbol(kallsyms, mod, fn, data);
  4513		mutex_unlock(&module_mutex);
  4514	
  4515		return ret;
  4516	}
  4517	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 4176c7eca7b5..89ed3eb2e185 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -65,11 +65,12 @@  static inline void *dereference_symbol_descriptor(void *ptr)
 	return ptr;
 }
 
+#ifdef CONFIG_KALLSYMS
+
 int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
 				      unsigned long),
 			    void *data);
 
-#ifdef CONFIG_KALLSYMS
 /* Lookup the address for a symbol. Returns 0 if not found. */
 unsigned long kallsyms_lookup_name(const char *name);
 
@@ -98,6 +99,14 @@  extern bool kallsyms_show_value(const struct cred *cred);
 
 #else /* !CONFIG_KALLSYMS */
 
+static inline int kallsyms_on_each_symbol(int (*fn)(void *, const char *,
+						    struct module *,
+						    unsigned long),
+					  void *data)
+{
+	return 0;
+}
+
 static inline unsigned long kallsyms_lookup_name(const char *name)
 {
 	return 0;
diff --git a/include/linux/module.h b/include/linux/module.h
index c9f1200b2312..e982aca57883 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -867,8 +867,43 @@  static inline bool module_sig_ok(struct module *module)
 }
 #endif	/* CONFIG_MODULE_SIG */
 
-int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
+#if defined(CONFIG_MODULES) && defined(CONFIG_KALLSYMS)
+
+#ifdef CONFIG_LIVEPATCH
+
+int module_kallsyms_on_each_symbol_all(int (*fn)(void *, const char *,
+						 struct module *,
+						 unsigned long),
+				       void *data);
+
+#else /* !CONFIG_LIVEPATCH */
+
+static inline int module_kallsyms_on_each_symbol_all(int (*fn)(void *, const char *,
+							   struct module *,
+							   unsigned long),
+						     void *data)
+{
+	return 0;
+}
+
+#endif /* CONFIG_LIVEPATCH */
+
+int module_kallsyms_on_each_symbol(struct module *mod,
+				   int (*fn)(void *, const char *,
 					     struct module *, unsigned long),
 				   void *data);
 
+#else /* !(CONFIG_MODULES && CONFIG_KALLSYMS) */
+
+static inline int module_kallsyms_on_each_symbol(struct module *mod,
+						 int (*fn)(void *, const char *,
+							   struct module *,
+							   unsigned long),
+						 void *data)
+{
+	return 0;
+}
+
+#endif /* CONFIG_MODULES && CONFIG_KALLSYMS */
+
 #endif /* _LINUX_MODULE_H */
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 3011bc33a5ba..da40f48f071e 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -224,10 +224,9 @@  unsigned long kallsyms_lookup_name(const char *name)
 	return module_kallsyms_lookup_name(name);
 }
 
-#ifdef CONFIG_LIVEPATCH
 /*
  * Iterate over all symbols in vmlinux.  For symbols from modules use
- * module_kallsyms_on_each_symbol instead.
+ * module_kallsyms_on_each_symbol_all instead.
  */
 int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
 				      unsigned long),
@@ -246,7 +245,6 @@  int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
 	}
 	return 0;
 }
-#endif /* CONFIG_LIVEPATCH */
 
 static unsigned long get_symbol_pos(unsigned long addr,
 				    unsigned long *symbolsize,
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 335d988bd811..3756071658fd 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -165,7 +165,7 @@  static int klp_find_object_symbol(const char *objname, const char *name,
 	};
 
 	if (objname)
-		module_kallsyms_on_each_symbol(klp_find_callback, &args);
+		module_kallsyms_on_each_symbol_all(klp_find_callback, &args);
 	else
 		kallsyms_on_each_symbol(klp_find_callback, &args);
 
diff --git a/kernel/module.c b/kernel/module.c
index 84a9141a5e15..88a24dd8f4bd 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4473,13 +4473,54 @@  unsigned long module_kallsyms_lookup_name(const char *name)
 	return ret;
 }
 
-#ifdef CONFIG_LIVEPATCH
-int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
+static int __module_kallsyms_on_each_symbol(struct mod_kallsyms *kallsyms,
+					    struct module *mod,
+					    int (*fn)(void *, const char *,
+						      struct module *, unsigned long),
+					    void *data)
+{
+	unsigned long i;
+	int ret = 0;
+
+	for (i = 0; i < kallsyms->num_symtab; i++) {
+		const Elf_Sym *sym = &kallsyms->symtab[i];
+
+		if (sym->st_shndx == SHN_UNDEF)
+			continue;
+
+		ret = fn(data, kallsyms_symbol_name(kallsyms, i),
+			 mod, kallsyms_symbol_value(sym));
+		if (ret != 0)
+			break;
+	}
+
+	return ret;
+}
+
+int module_kallsyms_on_each_symbol(struct module *mod,
+				   int (*fn)(void *, const char *,
 					     struct module *, unsigned long),
 				   void *data)
+{
+	struct mod_kallsyms *kallsyms;
+	int ret = 0;
+
+	mutex_lock(&module_mutex);
+	/* We hold module_mutex: no need for rcu_dereference_sched */
+	kallsyms = mod->kallsyms;
+	if (mod->state != MODULE_STATE_UNFORMED)
+		ret = __module_kallsyms_on_each_symbol(kallsyms, mod, fn, data);
+	mutex_unlock(&module_mutex);
+
+	return ret;
+}
+
+#ifdef CONFIG_LIVEPATCH
+int module_kallsyms_on_each_symbol_all(int (*fn)(void *, const char *,
+						 struct module *, unsigned long),
+				       void *data)
 {
 	struct module *mod;
-	unsigned int i;
 	int ret = 0;
 
 	mutex_lock(&module_mutex);
@@ -4489,19 +4530,10 @@  int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
 
 		if (mod->state == MODULE_STATE_UNFORMED)
 			continue;
-		for (i = 0; i < kallsyms->num_symtab; i++) {
-			const Elf_Sym *sym = &kallsyms->symtab[i];
-
-			if (sym->st_shndx == SHN_UNDEF)
-				continue;
-
-			ret = fn(data, kallsyms_symbol_name(kallsyms, i),
-				 mod, kallsyms_symbol_value(sym));
-			if (ret != 0)
-				goto out;
-		}
+		ret = __module_kallsyms_on_each_symbol(kallsyms, mod, fn, data);
+		if (ret != 0)
+			break;
 	}
-out:
 	mutex_unlock(&module_mutex);
 	return ret;
 }