[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v4 6/9] livepatch: Add parsing for the symbol+0x<offset>
On 09/08/2016 10:22 AM, Konrad Rzeszutek Wilk wrote:
On Wed, Sep 07, 2016 at 02:10:43AM -0600, Jan Beulich wrote:
On 06.09.16 at 21:56, <konrad.wilk@xxxxxxxxxx> wrote:
On Wed, Aug 24, 2016 at 03:08:01AM -0600, Jan Beulich wrote:
On 24.08.16 at 04:22, <konrad.wilk@xxxxxxxxxx> wrote:
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -237,13 +237,34 @@ static const char *livepatch_symbols_lookup(unsigned long
addr,
static int resolve_old_address(struct livepatch_func *f,
const struct livepatch_elf *elf)
{
+ const char *s;
+ char *plus = NULL;
Pointless initializer.
We need that otherwise this part (which is at the bottom of this function):
if ( plus )
{
*plus = '+';
f->old_addr += offset;
}
May be invoked for symbols that that don't have the '+' in them.
I don't see how it would. This
plus = strchr(f->name, '+');
comes ahead of any paths leading to the code you quote.
Ah. Stale information - the earlier patch had 'slash' and 'plus' variables
to look for - and that was why I needed it.
But with the code you are quoting - it is not needed.
+ /* +<offset> */
+ plus = strchr(f->name, '+');
And I think you should prefer using the local variable here.
<nods>
Furthermore you're losing const here - does f->name really point
to memory that doesn't get mapped r/o?
Yes.
The 'struct livepatch_func' contains the the ->opaque array of 31 bytes
(from which we use 5 bytes) which the hypervisor uses to stash the original
instructions.
How does the patch name end up in (5 bytes of) the opaque field?
I was (ineptly) saying that the struct livepatch_func has fields that are
modified, hence it ends up in .data section.
Wait a minute. The f->name should have a relocation to point to .rodata
instead of .data! And that should have crashed when I modified it.
livepatch-build-tools puts the function names in a read only section
(.livepatch.strings), so the approach of fiddling with f->name probably
needs to change.
--
Ross Lagerwall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
