Messages in this thread Patch in this message | | | From | Rusty Russell <> | Subject | Re: [PATCH 3/7] Ksplice: Make find_symbol return a struct kernel_symbol | Date | Wed, 4 Feb 2009 19:56:13 +1030 |
| |
On Saturday 06 December 2008 10:33:56 Jeff Arnold wrote: > From: Tim Abbott <tabbott@mit.edu> > > Ksplice needs access to the kernel_symbol structure in order to support > modifications to the exported symbol table.
Applied, with some fixes:
> preempt_disable(); > - if (IS_ERR_VALUE(find_symbol(symbol, &owner, NULL, true, false))) > + if (find_symbol(symbol, &owner, NULL, true, false) == NULL) > BUG();
I skipped the == NULL part; if (find_symbol... reads quite well. Same with three or so other places.
> - ret = find_symbol(name, &owner, &crc, > + sym = find_symbol(name, &owner, &crc, > !(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)), true); > - if (!IS_ERR_VALUE(ret)) { > - /* use_module can fail due to OOM, > - or module initialization or unloading */ > - if (!check_version(sechdrs, versindex, name, mod, crc) || > - !use_module(mod, owner)) > - ret = -EINVAL; > - } > - return ret; > + /* use_module can fail due to OOM, > + or module initialization or unloading */ > + if (!check_version(sechdrs, versindex, name, mod, crc) || > + !use_module(mod, owner)) > + sym = NULL; > + return sym; > }
If sym is NULL those other values aren't valid! You can't take out the branch....
> - return (void *)value; > + return sym == NULL ? (void *)0 : (void *)sym->value; > }
One NULL too many, and one too few: return sym ? (void *)sym->value : NULL;
Here's the nett diff:
diff --git a/kernel/module.c b/kernel/module.c --- a/kernel/module.c +++ b/kernel/module.c @@ -860,7 +860,7 @@ void __symbol_put(const char *symbol) struct module *owner; preempt_disable(); - if (find_symbol(symbol, &owner, NULL, true, false) == NULL) + if (!find_symbol(symbol, &owner, NULL, true, false)) BUG(); module_put(owner); preempt_enable(); @@ -1023,7 +1023,7 @@ static inline int check_modstruct_versio { const unsigned long *crc; - if (find_symbol("struct_module", NULL, &crc, true, false) == NULL) + if (!find_symbol("struct_module", NULL, &crc, true, false)) BUG(); return check_version(sechdrs, versindex, "struct_module", mod, crc); } @@ -1077,9 +1077,11 @@ static const struct kernel_symbol *resol !(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)), true); /* use_module can fail due to OOM, or module initialization or unloading */ - if (!check_version(sechdrs, versindex, name, mod, crc) || - !use_module(mod, owner)) - sym = NULL; + if (sym) { + if (!check_version(sechdrs, versindex, name, mod, crc) || + !use_module(mod, owner)) + sym = NULL; + } return sym; } @@ -1481,11 +1483,11 @@ void *__symbol_get(const char *symbol) preempt_disable(); sym = find_symbol(symbol, &owner, NULL, true, true); - if (sym != NULL && strong_try_module_get(owner)) + if (sym && strong_try_module_get(owner)) sym = NULL; preempt_enable(); - return sym == NULL ? (void *)0 : (void *)sym->value; + return sym ? (void *)sym->value : NULL; } EXPORT_SYMBOL_GPL(__symbol_get); @@ -1513,8 +1515,7 @@ static int verify_export_symbols(struct for (i = 0; i < ARRAY_SIZE(arr); i++) { for (s = arr[i].sym; s < arr[i].sym + arr[i].num; s++) { - if (find_symbol(s->name, &owner, NULL, true, false) - != NULL) { + if (find_symbol(s->name, &owner, NULL, true, false)) { printk(KERN_ERR "%s: exports duplicate symbol %s" " (owned by %s)\n", @@ -1561,7 +1562,7 @@ static int simplify_symbols(Elf_Shdr *se ksym = resolve_symbol(sechdrs, versindex, strtab + sym[i].st_name, mod); /* Ok if resolved. */ - if (ksym != NULL) { + if (ksym) { sym[i].st_value = ksym->value; break; }
| |