[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <200806231348.58317.rusty@rustcorp.com.au>
Date: Mon, 23 Jun 2008 13:48:58 +1000
From: Rusty Russell <rusty@...tcorp.com.au>
To: Jiri Kosina <jkosina@...e.cz>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
linux-kernel@...r.kernel.org, llipavsky@...e.cz
Subject: Re: [PATCH] module: make symbol_put_addr() work for all exported symbols
On Thursday 19 June 2008 23:11:54 Jiri Kosina wrote:
> symbol_put_addr() works only for exported function names (symbols present
> in text section). This for example means that
>
> symbol_put_addr(__symbol_get("any_exported_variable_name"))
>
> triggers a BUG, which really seems wrong.
Hi Jiri,
You're right, good catch!
> This patch introduces generic lookup_symbol_address(), which performs
> lookup on symbol tables of all modules according to the address, and makes
> symbol_put_addr() use this interface to find the module owner instead.
>
> Signed-off-by: Jiri Kosina <jkosina@...e.cz>
It might be better to centralize all these iterators, and create a proper
iterator function. Any chance you could rewrite it on top of this patch?
(Lightly tested)
Thanks!
Rusty.
module: generic each_symbol iterator function
Introduce an each_symbol() iterator to avoid duplicating the knowledge
about the 5 different sections containing symbols. Currently only
used by find_symbol(), but will be used by symbol_put_addr() too.
Signed-off-by: Rusty Russell <rusty@...tcorp.com.au>
diff -r 909eac180d25 kernel/module.c
--- a/kernel/module.c Fri Jun 20 15:56:43 2008 +1000
+++ b/kernel/module.c Mon Jun 23 13:40:43 2008 +1000
@@ -152,6 +152,167 @@ extern const unsigned long __start___kcr
#define symversion(base, idx) ((base != NULL) ? ((base) + (idx)) : NULL)
#endif
+struct symsearch {
+ const struct kernel_symbol *start, *stop;
+ const unsigned long *crcs;
+ enum {
+ NOT_GPL_ONLY,
+ GPL_ONLY,
+ WILL_BE_GPL_ONLY,
+ } licence;
+ bool unused;
+};
+
+static bool each_symbol_in_section(const struct symsearch *arr,
+ unsigned int arrsize,
+ struct module *owner,
+ bool (*fn)(const struct symsearch *syms,
+ struct module *owner,
+ unsigned int symnum, void *data),
+ void *data)
+{
+ unsigned int i, j;
+
+ for (j = 0; j < arrsize; j++) {
+ for (i = 0; i < arr[j].stop - arr[j].start; i++)
+ if (fn(&arr[j], owner, i, data))
+ return true;
+ }
+
+ return false;
+}
+
+/* Returns true as soon as fn returns true, otherwise false. */
+static bool each_symbol(bool (*fn)(const struct symsearch *arr,
+ struct module *owner,
+ unsigned int symnum, void *data),
+ void *data)
+{
+ struct module *mod;
+ const struct symsearch arr[] = {
+ { __start___ksymtab, __stop___ksymtab, __start___kcrctab,
+ NOT_GPL_ONLY, false },
+ { __start___ksymtab_gpl, __stop___ksymtab_gpl,
+ __start___kcrctab_gpl,
+ GPL_ONLY, false },
+ { __start___ksymtab_gpl_future, __stop___ksymtab_gpl_future,
+ __start___kcrctab_gpl_future,
+ WILL_BE_GPL_ONLY, false },
+ { __start___ksymtab_unused, __stop___ksymtab_unused,
+ __start___kcrctab_unused,
+ NOT_GPL_ONLY, true },
+ { __start___ksymtab_unused_gpl, __stop___ksymtab_unused_gpl,
+ __start___kcrctab_unused_gpl,
+ GPL_ONLY, true },
+ };
+
+ if (each_symbol_in_section(arr, ARRAY_SIZE(arr), NULL, fn, data))
+ return true;
+
+ list_for_each_entry(mod, &modules, list) {
+ struct symsearch arr[] = {
+ { mod->syms, mod->syms + mod->num_syms, mod->crcs,
+ NOT_GPL_ONLY, false },
+ { mod->gpl_syms, mod->gpl_syms + mod->num_gpl_syms,
+ mod->gpl_crcs,
+ GPL_ONLY, false },
+ { mod->gpl_future_syms,
+ mod->gpl_future_syms + mod->num_gpl_future_syms,
+ mod->gpl_future_crcs,
+ WILL_BE_GPL_ONLY, false },
+ { mod->unused_syms,
+ mod->unused_syms + mod->num_unused_syms,
+ mod->unused_crcs,
+ NOT_GPL_ONLY, true },
+ { mod->unused_gpl_syms,
+ mod->unused_gpl_syms + mod->num_unused_gpl_syms,
+ mod->unused_gpl_crcs,
+ GPL_ONLY, true },
+ };
+
+ if (each_symbol_in_section(arr, ARRAY_SIZE(arr), mod, fn, data))
+ return true;
+ }
+ return false;
+}
+
+struct find_symbol_arg
+{
+ /* Input */
+ const char *name;
+ bool gplok;
+ bool warn;
+
+ /* Output */
+ struct module *owner;
+ const unsigned long *crc;
+ unsigned long value;
+};
+
+static bool find_symbol_in_section(const struct symsearch *syms,
+ struct module *owner,
+ unsigned int symnum, void *data)
+{
+ struct find_symbol_arg *fsa = data;
+
+ if (strcmp(syms->start[symnum].name, fsa->name) != 0)
+ return false;
+
+ if (!fsa->gplok) {
+ if (syms->licence == GPL_ONLY)
+ return false;
+ if (syms->licence == WILL_BE_GPL_ONLY && fsa->warn) {
+ printk(KERN_WARNING "Symbol %s is being used "
+ "by a non-GPL module, which will not "
+ "be allowed in the future\n", fsa->name);
+ printk(KERN_WARNING "Please see the file "
+ "Documentation/feature-removal-schedule.txt "
+ "in the kernel source tree for more details.\n");
+ }
+ }
+
+ if (syms->unused && fsa->warn) {
+ printk(KERN_WARNING "Symbol %s is marked as UNUSED, "
+ "however this module is using it.\n", fsa->name);
+ printk(KERN_WARNING
+ "This symbol will go away in the future.\n");
+ printk(KERN_WARNING
+ "Please evalute if this is the right api to use and if "
+ "it really is, submit a report the linux kernel "
+ "mailinglist together with submitting your code for "
+ "inclusion.\n");
+ }
+
+ fsa->owner = owner;
+ fsa->crc = symversion(syms->crcs, symnum);
+ fsa->value = syms->start[symnum].value;
+ return true;
+}
+
+/* Find a symbol, return value, (optional) crc and (optional) module
+ * which owns it */
+static unsigned long find_symbol(const char *name,
+ struct module **owner,
+ const unsigned long **crc,
+ bool gplok,
+ bool warn)
+{
+ struct find_symbol_arg fsa;
+
+ fsa.name = name;
+ fsa.gplok = gplok;
+ fsa.warn = warn;
+
+ if (each_symbol(find_symbol_in_section, &fsa)) {
+ *owner = fsa.owner;
+ *crc = fsa.crc;
+ return fsa.value;
+ }
+
+ DEBUGP("Failed to find symbol %s\n", name);
+ return -ENOENT;
+}
+
/* lookup symbol in given range of kernel_symbols */
static const struct kernel_symbol *lookup_symbol(const char *name,
const struct kernel_symbol *start,
@@ -162,144 +323,6 @@ static const struct kernel_symbol *looku
if (strcmp(ks->name, name) == 0)
return ks;
return NULL;
-}
-
-static bool always_ok(bool gplok, bool warn, const char *name)
-{
- return true;
-}
-
-static bool printk_unused_warning(bool gplok, bool warn, const char *name)
-{
- if (warn) {
- printk(KERN_WARNING "Symbol %s is marked as UNUSED, "
- "however this module is using it.\n", name);
- printk(KERN_WARNING
- "This symbol will go away in the future.\n");
- printk(KERN_WARNING
- "Please evalute if this is the right api to use and if "
- "it really is, submit a report the linux kernel "
- "mailinglist together with submitting your code for "
- "inclusion.\n");
- }
- return true;
-}
-
-static bool gpl_only_unused_warning(bool gplok, bool warn, const char *name)
-{
- if (!gplok)
- return false;
- return printk_unused_warning(gplok, warn, name);
-}
-
-static bool gpl_only(bool gplok, bool warn, const char *name)
-{
- return gplok;
-}
-
-static bool warn_if_not_gpl(bool gplok, bool warn, const char *name)
-{
- if (!gplok && warn) {
- printk(KERN_WARNING "Symbol %s is being used "
- "by a non-GPL module, which will not "
- "be allowed in the future\n", name);
- printk(KERN_WARNING "Please see the file "
- "Documentation/feature-removal-schedule.txt "
- "in the kernel source tree for more details.\n");
- }
- return true;
-}
-
-struct symsearch {
- const struct kernel_symbol *start, *stop;
- const unsigned long *crcs;
- bool (*check)(bool gplok, bool warn, const char *name);
-};
-
-/* Look through this array of symbol tables for a symbol match which
- * passes the check function. */
-static const struct kernel_symbol *search_symarrays(const struct symsearch *arr,
- unsigned int num,
- const char *name,
- bool gplok,
- bool warn,
- const unsigned long **crc)
-{
- unsigned int i;
- const struct kernel_symbol *ks;
-
- for (i = 0; i < num; i++) {
- ks = lookup_symbol(name, arr[i].start, arr[i].stop);
- if (!ks || !arr[i].check(gplok, warn, name))
- continue;
-
- if (crc)
- *crc = symversion(arr[i].crcs, ks - arr[i].start);
- return ks;
- }
- return NULL;
-}
-
-/* Find a symbol, return value, (optional) crc and (optional) module
- * which owns it */
-static unsigned long find_symbol(const char *name,
- struct module **owner,
- const unsigned long **crc,
- bool gplok,
- bool warn)
-{
- struct module *mod;
- const struct kernel_symbol *ks;
- const struct symsearch arr[] = {
- { __start___ksymtab, __stop___ksymtab, __start___kcrctab,
- always_ok },
- { __start___ksymtab_gpl, __stop___ksymtab_gpl,
- __start___kcrctab_gpl, gpl_only },
- { __start___ksymtab_gpl_future, __stop___ksymtab_gpl_future,
- __start___kcrctab_gpl_future, warn_if_not_gpl },
- { __start___ksymtab_unused, __stop___ksymtab_unused,
- __start___kcrctab_unused, printk_unused_warning },
- { __start___ksymtab_unused_gpl, __stop___ksymtab_unused_gpl,
- __start___kcrctab_unused_gpl, gpl_only_unused_warning },
- };
-
- /* Core kernel first. */
- ks = search_symarrays(arr, ARRAY_SIZE(arr), name, gplok, warn, crc);
- if (ks) {
- if (owner)
- *owner = NULL;
- return ks->value;
- }
-
- /* Now try modules. */
- list_for_each_entry(mod, &modules, list) {
- struct symsearch arr[] = {
- { mod->syms, mod->syms + mod->num_syms, mod->crcs,
- always_ok },
- { mod->gpl_syms, mod->gpl_syms + mod->num_gpl_syms,
- mod->gpl_crcs, gpl_only },
- { mod->gpl_future_syms,
- mod->gpl_future_syms + mod->num_gpl_future_syms,
- mod->gpl_future_crcs, warn_if_not_gpl },
- { mod->unused_syms,
- mod->unused_syms + mod->num_unused_syms,
- mod->unused_crcs, printk_unused_warning },
- { mod->unused_gpl_syms,
- mod->unused_gpl_syms + mod->num_unused_gpl_syms,
- mod->unused_gpl_crcs, gpl_only_unused_warning },
- };
-
- ks = search_symarrays(arr, ARRAY_SIZE(arr),
- name, gplok, warn, crc);
- if (ks) {
- if (owner)
- *owner = mod;
- return ks->value;
- }
- }
-
- DEBUGP("Failed to find symbol %s\n", name);
- return -ENOENT;
}
/* Search for module by name: must hold module_mutex. */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists