Re: [PATCH v5 13/28] xsplice, symbols: Implement symbol name resolution on address.

[Ross Lagerwall <ross.lagerwall@citrix.com>]

On 04/08/2016 06:38 PM, Jan Beulich wrote:
> (Did you drop all Cc-s for a reason?)

No. Re-adding CCs.

>
>>>> On 08.04.16 at 18:04, <ross.lagerwall@citrix.com> wrote:
>> On 04/01/2016 04:11 PM, Jan Beulich wrote:
>>>> +    nsyms = 0;
>>>> +    strtab_len = 0;
>>>> +    for ( i = 1; i < elf->nsym; i++ )
>>>> +    {
>>>> +        if ( is_core_symbol(elf, elf->sym + i) )
>>>> +        {
>>>> +            symtab[nsyms].name = strtab + strtab_len;
>>>> +            symtab[nsyms].size = elf->sym[i].sym->st_size;
>>>> +            symtab[nsyms].value = elf->sym[i].sym->st_value;
>>>> +            symtab[nsyms].new_symbol = 0; /* To be checked below. */
>>>> +            strtab_len += strlcpy(strtab + strtab_len, elf->sym[i].name,
>>>> +                                  KSYM_NAME_LEN) + 1;
>>>> +            nsyms++;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    for ( i = 0; i < nsyms; i++ )
>>>> +    {
>>>> +        bool_t found = 0;
>>>> +
>>>> +        for ( j = 0; j < payload->nfuncs; j++ )
>>>> +        {
>>>> +            if ( symtab[i].value == payload->funcs[j].new_addr )
>>>> +            {
>>>> +                found = 1;
>>>> +                break;
>>>> +            }
>>>> +        }
>>>> +
>>>> +        if ( !found )
>>>> +        {
>>>> +            if ( xsplice_symbols_lookup_by_name(symtab[i].name) )
>>>> +            {
>>>> +                printk(XENLOG_ERR "%s%s: duplicate new symbol: %s\n",
>>>> +                       XSPLICE, elf->name, symtab[i].name);
>>>> +                xfree(symtab);
>>>> +                xfree(strtab);
>>>> +                return -EEXIST;
>>>> +            }
>>>> +            symtab[i].new_symbol = 1;
>>>> +            dprintk(XENLOG_DEBUG, "%s%s: new symbol %s\n",
>>>> +                    XSPLICE, elf->name, symtab[i].name);
>>>> +        }
>>>> +        else
>>>> +        {
>>>> +            dprintk(XENLOG_DEBUG, "%s%s: overriding symbol %s\n",
>>>> +                    XSPLICE, elf->name, symtab[i].name);
>>>
>>> Since you don't do anything here - how is this an override of some
>>> sort?
>>
>> The binary patch that is being loaded is overriding a hypervisor symbol
>> or a symbol introduced in a previous patch. i.e. you're replacing the
>> old function with a different one. Binary patches can also introduce
>> completely new functions (just above).
>
> So you mean to say that in symbol lookup, patches take priority
> over the core binary? Once we get rid of linear ordering of
> patches, how would this yield deterministic results?
>

Yes. If a patch replaces some functions in the hypervisor, then when 
performing a symbol lookup you'd want to get the address of the function 
currently in use, which is the one from the patch. If the linear 
ordering restriction were removed, then the symbol lookup would simply 
need to be updated so that it still gets the address of the function 
currently in use (however that is determined).

There's also a different type of symbol lookup in the xsplice code: 
looking up the address of the symbol to be replaced. In this case, it is 
the original symbol thatr needs to be returned. This prevents having a 
chain of jumps if a function is patched multiple times.

-- 
Ross Lagerwall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel