linux-kernel - Re: [-mm][PATCH 0/6] (yet another) kprobes x86 code unification and boosters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4766CCD0.3030300@redhat.com>
Date:	Mon, 17 Dec 2007 14:24:00 -0500
From:	Masami Hiramatsu <mhiramat@...hat.com>
To:	Ingo Molnar <mingo@...e.hu>,
	Harvey Harrison <harvey.harrison@...il.com>
CC:	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	Jim Keniston <jkenisto@...ibm.com>,
	Roland McGrath <roland@...hat.com>,
	Arjan van de Ven <arjan@...radead.org>,
	prasanna@...ibm.com, anil.s.keshavamurthy@...el.com,
	davem@...emloft.net, systemtap-ml <systemtap@...rces.redhat.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [-mm][PATCH 0/6] (yet another) kprobes x86 code unification	and
 boosters

Hi,

Ingo Molnar wrote:
> * Masami Hiramatsu <mhiramat@...hat.com> wrote:
> 
>> Hello all,
>>
>> I developed a series of patches which unifies kprobes code on x86 and 
>> introduces boosters on x86-64. These patches can be applied to 
>> 2.6.24-rc4-mm1.
>>
>> The purpose of this patchset is unifying kprobes_[32|64].[c|h] to 
>> kprobes.[c|h] for simplifying code maintenance.
>>
>> I know these patches are conflicting with Harvey's patch. We need to 
>> solve that.
> 
> your series fixes the 64-bit crash that i was seeing, so i've picked it 
> up. Please work it out with Harvey which cleanups of him are not 
> included yet.

Absolutely sure.
I compared my patch and Harvey's.
These directions are almost same.

Harvey, I have found some differences and I'd like to fix that with you.
I think following comments and style cleanups in your patch are good to me.

> @@ -156,7 +157,7 @@ twobyte_has_modrm[256 / sizeof(unsigned long)] = {
>  #undef RF
>  
>  /* insert a jmp code */
> -static __always_inline void set_jmp_op(void *from, void *to)
> +static inline void set_jmp_op(void *from, void *to)
>  {
>  	struct __arch_jmp_op {
>  		char op;
> @@ -170,7 +171,7 @@ static __always_inline void set_jmp_op(void *from, void *to)
>  /*
>   * returns non-zero if opcodes can be boosted.
>   */
> -static __always_inline int can_boost(kprobe_opcode_t *opcodes)
> +static inline int can_boost(kprobe_opcode_t *opcodes)
>  {
>  	kprobe_opcode_t opcode;
>  	kprobe_opcode_t *orig_opcodes = opcodes;


> @@ -734,7 +740,7 @@ static int __kprobes post_kprobe_handler(struct pt_regs *regs)
>  	regs->flags |= kcb->kprobe_saved_flags;
>  	trace_hardirqs_fixup_flags(regs->flags);
>  
> -	/*Restore back the original saved kprobes variables and continue. */
> +	/* Restore the original saved kprobes variables and continue. */
>  	if (kcb->kprobe_status == KPROBE_REENTER) {
>  		restore_previous_kprobe(kcb);
>  		goto out;
> @@ -860,7 +866,7 @@ int __kprobes setjmp_pre_handler(struct kprobe *p, struct pt_regs *regs)
>  	addr = (unsigned long)(kcb->jprobe_saved_sp);
>  
>  	/*
> -	 * TBD: As Linus pointed out, gcc assumes that the callee
> +	 * As Linus pointed out, gcc assumes that the callee
>  	 * owns the argument space and could overwrite it, e.g.
>  	 * tailcall optimization. So, to be absolutely safe
>  	 * we also save and restore enough stack bytes to cover

And also, if you can unify x86/mm/extable_*.c and introduce fixup_exception() to 64-bit,
it is very helpful to remove ifdefs from kprobe_fault_handler().

Thank you,

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@...hat.com, masami.hiramatsu.pt@...achi.com

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