Re: [PATCH v2 03/13] uprobes: allow arch access to xol slot

From: Oleg Nesterov
Date: Tue Oct 29 2013 - 11:38:56 EST


On 10/22, David Long wrote:
>
> On 10/19/13 12:36, Oleg Nesterov wrote:
>>
>> How about
>>
>> void __weak arch_uprobe_xol_copy(...)
>> {
>> copy_to_page(...);
>> }
>>
>> then just
>>
>> - copy_to_page(...);
>> + arch_uprobe_xol_copy(...);
>>
>> ?
>>
>
> I was trying to avoid duplicating the VM calls in the
> architecture-specific implementations,

Yes, I understand... But this uglifies the generic code.

> but maybe that is the cleaner way
> to do it after all. I've made changes as suggested above.

Thanks. and this uglifies arm code I guess ;)


David. Perhaps we can avoid the new hook altogether? What if we do
the simple change below (it ignores powerpc) ?

Then arm can add "unsigned long ixol[2]" into its arch_uprobe, and
arch_uprobe_analyze_insn() can initialize this member correctly.

What do you think?

Oleg.

--- x/arch/x86/include/asm/uprobes.h
+++ x/arch/x86/include/asm/uprobes.h
@@ -35,7 +35,10 @@ typedef u8 uprobe_opcode_t;

struct arch_uprobe {
u16 fixups;
- u8 insn[MAX_UINSN_BYTES];
+ union {
+ u8 insn[MAX_UINSN_BYTES];
+ u8 ixol[MAX_UINSN_BYTES];
+ }
#ifdef CONFIG_X86_64
unsigned long rip_rela_target_address;
#endif
--- x/kernel/events/uprobes.c
+++ x/kernel/events/uprobes.c
@@ -1264,7 +1264,8 @@ static unsigned long xol_get_insn_slot(s
return 0;

/* Initialize the slot */
- copy_to_page(area->page, xol_vaddr, uprobe->arch.insn, MAX_UINSN_BYTES);
+ copy_to_page(area->page, xol_vaddr,
+ uprobe->arch.ixol, sizeof(uprobe->arch.ixol));
/*
* We probably need flush_icache_user_range() but it needs vma.
* This should work on supported architectures too.

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