linux-kernel - Re: [PATCH v4 04/16] ARM: move generic thumb instruction parsing code to new files for use by other feature

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1387543562.3404.35.camel@linaro1.home>
Date:	Fri, 20 Dec 2013 12:46:02 +0000
From:	"Jon Medhurst (Tixy)" <tixy@...aro.org>
To:	David Long <dave.long@...aro.org>
Cc:	linux-arm-kernel@...ts.infradead.org,
	Russell King <linux@....linux.org.uk>,
	Rabin Vincent <rabin@....in>, Oleg Nesterov <oleg@...hat.com>,
	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
	Ingo Molnar <mingo@...hat.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>,
	davem@...emloft.net, Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Paul Mackerras <paulus@...ba.org>,
	Arnaldo Carvalho de Melo <acme@...stprotocols.net>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 04/16] ARM: move generic thumb instruction parsing
 code to new files for use by other feature

On Sun, 2013-12-15 at 23:08 -0500, David Long wrote:
> From: "David A. Long" <dave.long@...aro.org>
> 
> Move the thumb version of the kprobes instruction parsing code into more generic
> files from where it can be used by uprobes and possibly other subsystems. The
> symbol names will be made more generic in a subsequent part of this patchset.
> 
> Signed-off-by: David A. Long <dave.long@...aro.org>
> ---

I just have two comments about probes-thumb.h ...

[...]

> diff --git a/arch/arm/kernel/probes-thumb.h b/arch/arm/kernel/probes-thumb.h
> new file mode 100644
> index 0000000..3f39210
> --- /dev/null
> +++ b/arch/arm/kernel/probes-thumb.h
> @@ -0,0 +1,136 @@
> +/*
> + * arch/arm/kernel/probes-arm.h
> + *
> + * Copyright 2013 Linaro Ltd.
> + * Written by: David A. Long
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#ifndef _ARM_KERNEL_PROBES_THUMB_H
> +#define  _ARM_KERNEL_PROBES_THUMB_H
> +
> +/*
> + * True if current instruction is in an IT block.
> + */
> +#define in_it_block(cpsr)	((cpsr & 0x06000c00) != 0x00000000)
> +
> +/*
> + * Return the condition code to check for the currently executing instruction.
> + * This is in ITSTATE<7:4> which is in CPSR<15:12> but is only valid if
> + * in_it_block returns true.
> + */
> +#define current_cond(cpsr)	((cpsr >> 12) & 0xf)

Looks like you forgot to remove above two #defines from kprobes-thumb.c
when you moved them to this header file.

Also...

> +enum probes_t32_action {
> +	PROBES_T32_EMULATE_NONE,
> +	PROBES_T32_SIMULATE_NOP,
> +	PROBES_T32_LDMSTM,
> +	PROBES_T32_LDRDSTRD,
> +	PROBES_T32_TABLE_BRANCH,
> +	PROBES_T32_TST,
> +	PROBES_T32_CMP,
> +	PROBES_T32_MOV,
> +	PROBES_T32_ADDSUB,
> +	PROBES_T32_LOGICAL,
> +	PROBES_T32_ADDWSUBW_PC,
> +	PROBES_T32_ADDWSUBW,
> +	PROBES_T32_MOVW,
> +	PROBES_T32_SAT,
> +	PROBES_T32_BITFIELD,
> +	PROBES_T32_SEV,
> +	PROBES_T32_WFE,
> +	PROBES_T32_MRS,
> +	PROBES_T32_BRANCH_COND,
> +	PROBES_T32_BRANCH,
> +	PROBES_T32_PLDI,
> +	PROBES_T32_LDR_LIT,
> +	PROBES_T32_LDRSTR,
> +	PROBES_T32_SIGN_EXTEND,
> +	PROBES_T32_MEDIA,
> +	PROBES_T32_REVERSE,
> +	PROBES_T32_MUL_ADD,
> +	PROBES_T32_MUL_ADD2,
> +	PROBES_T32_MUL_ADD_LONG
> +};
> +
> +enum probes_t16_action {
> +	PROBES_T16_ADD_SP,
> +	PROBES_T16_CBZ,
> +	PROBES_T16_SIGN_EXTEND,
> +	PROBES_T16_PUSH,
> +	PROBES_T16_POP,
> +	PROBES_T16_SEV,
> +	PROBES_T16_WFE,
> +	PROBES_T16_IT,
> +	PROBES_T16_CMP,
> +	PROBES_T16_ADDSUB,
> +	PROBES_T16_LOGICAL,
> +	PROBES_T16_BLX,
> +	PROBES_T16_HIREGOPS,
> +	PROBES_T16_LDR_LIT,
> +	PROBES_T16_LDRHSTRH,
> +	PROBES_T16_LDRSTR,
> +	PROBES_T16_ADR,
> +	PROBES_T16_LDMSTM,
> +	PROBES_T16_BRANCH_COND,
> +	PROBES_T16_BRANCH
> +};
> +

I think the above two enums belong in the next patch in the series, i.e.
"[PATCH v4 05/16] ARM: use a function table for determining instruction
interpreter actions"

-- 
Tixy


--
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/