lkml.org 
[lkml]   [2009]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [PATCH 3/7] Ksplice: Make find_symbol return a struct kernel_symbol
    Date
    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;
    }

    \
     
     \ /
      Last update: 2009-02-04 10:29    [W:2.577 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site