On Mon, 16 Nov 2015, Chris J Arges wrote:

> @@ -281,29 +243,26 @@ static int klp_write_object_relocations(struct module 
> *pmod,
>               return -EINVAL;
>  
>       for (reloc = obj->relocs; reloc->name; reloc++) {
> -             if (!klp_is_module(obj)) {
> -                     ret = klp_verify_vmlinux_symbol(reloc->name,
> -                                                     reloc->val);
> -                     if (ret)
> -                             return ret;
> -             } else {
> -                     /* module, reloc->val needs to be discovered */
> -                     if (reloc->external)
> -                             ret = klp_find_external_symbol(pmod,
> -                                                            reloc->name,
> -                                                            &reloc->val);
> -                     else
> -                             ret = klp_find_object_symbol(obj->mod->name,
> -                                                          reloc->name,
> -                                                          0, &reloc->val);
> -                     if (ret)
> -                             return ret;
> +             if (reloc->sympos && (reloc->sympos != 1)) {
> +                     pr_err("symbol '%s' is external and has sympos %lu\n",
> +                            reloc->name, reloc->sympos);
> +                     return -EINVAL;
>               }

Oh, this check should not be here, right? I think it needs to be under 'if 
(reloc->external) {'.

And

if (reloc->sympos && (reloc->sympos != 1)) {

can be written as

if (reloc->sympos > 1) {

Anyway, I am not sure if this is the right thing to do. I know Petr 
suggested it, but maybe it would be sufficient just to error out when 
reloc->sympos is specified for external symbols. Despite there is a valid 
value (== 1). Like it had been done before. Josh came up with the issue. 
So what is your opinion, Josh? It is a nitpick, but nevertheless.

Miroslav

> +             /* discover the address of the referenced symbol */
> +             if (reloc->external) {
> +                     ret = klp_find_external_symbol(pmod, reloc->name, &val);
> +             } else
> +                     ret = klp_find_object_symbol(obj->mod->name,
> +                                                  reloc->name,
> +                                                  reloc->sympos,
> +                                                  &val);
> +             if (ret)
> +                     return ret;
>               ret = klp_write_module_reloc(pmod, reloc->type, reloc->loc,
> -                                          reloc->val + reloc->addend);
> +                                          val + reloc->addend);
>               if (ret) {
>                       pr_err("relocation failed for symbol '%s' at 0x%016lx 
> (%d)\n",
> -                            reloc->name, reloc->val, ret);
> +                            reloc->name, val, ret);
>                       return ret;
>               }
>       }
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to