linux-kernel - Re: [PATCH] tile: support kprobes on tilegx

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 for Android: free password hash cracker in your pocket

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

Message-ID: <52087770.1080500@hitachi.com>
Date:	Mon, 12 Aug 2013 14:49:36 +0900
From:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
To:	Tony Lu <zlu@...era.com>
Cc:	linux-kernel@...r.kernel.org,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH] tile: support kprobes on tilegx

Hello Tony,

(2013/08/10 4:08), Tony Lu wrote:
> This change includes support for Kprobes, Jprobes and Return Probes.

Thank you for the effort, this looks good for the first step.

However, it seems this only supports instructions which doesn't touch
the execution path. I don't know tile ISA, but it should have jump/branch/
call/return etc. Those should be fixed after singlestep out-of-line in
resume_execution(). Or, kprobes should reject to probe on such instructions
by arch_prepare_kprobe().

> +int __kprobes arch_prepare_kprobe(struct kprobe *p)
> +{
> +	unsigned long addr = (unsigned long)p->addr;
> +
> +	if (addr & (sizeof(kprobe_opcode_t) - 1))
> +		return -EINVAL;
> +
> +	/* insn: must be on special executable page on tile. */
> +	p->ainsn.insn = get_insn_slot();
> +	if (!p->ainsn.insn)
> +		return -ENOMEM;
> +
> +	/*
> +	 * In the kprobe->ainsn.insn[] array we store the original
> +	 * instruction at index zero and a break trap instruction at
> +	 * index one.
> +	 */
> +	memcpy(&p->ainsn.insn[0], p->addr, sizeof(kprobe_opcode_t));
> +	p->ainsn.insn[1] = breakpoint2_insn;
> +	p->opcode = *p->addr;
> +
> +	return 0;
> +}
[...]
> +/*
> + * Called after single-stepping.  p->addr is the address of the
> + * instruction whose first byte has been replaced by the "break 0"
> + * temporarily put back the original opcode to single-step, we
> + * single-stepped a copy of the instruction.  The address of this
> + * copy is p->ainsn.insn.
> + *
> + * This function prepares to return from the post-single-step
> + * breakpoint trap.
> + */
> +static void __kprobes resume_execution(struct kprobe *p,
> +				       struct pt_regs *regs,
> +				       struct kprobe_ctlblk *kcb)
> +{
> +	unsigned long orig_pc = kcb->kprobe_saved_pc;
> +	regs->pc = orig_pc + 8;
> +}
> +

I recommend you to fix that, at least filter out non-supported instructions
by arch_prepare_kprobe(), because no other users don't know which
instruction they can probe. :)

Thank you again!

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: 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/