All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] powerpc: sstep: Fix load and update instructions
@ 2020-11-19  5:41 Sandipan Das
  2020-11-19  5:41 ` [PATCH 2/2] powerpc: sstep: Fix store " Sandipan Das
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Sandipan Das @ 2020-11-19  5:41 UTC (permalink / raw)
  To: mpe; +Cc: ravi.bangoria, jniethe5, paulus, naveen.n.rao, linuxppc-dev, dja

The Power ISA says that the fixed-point load and update
instructions must neither use R0 for the base address (RA)
nor have the destination (RT) and the base address (RA) as
the same register. In these cases, the instruction is
invalid. This applies to the following instructions.
  * Load Byte and Zero with Update (lbzu)
  * Load Byte and Zero with Update Indexed (lbzux)
  * Load Halfword and Zero with Update (lhzu)
  * Load Halfword and Zero with Update Indexed (lhzux)
  * Load Halfword Algebraic with Update (lhau)
  * Load Halfword Algebraic with Update Indexed (lhaux)
  * Load Word and Zero with Update (lwzu)
  * Load Word and Zero with Update Indexed (lwzux)
  * Load Word Algebraic with Update Indexed (lwaux)
  * Load Doubleword with Update (ldu)
  * Load Doubleword with Update Indexed (ldux)

However, the following behaviour is observed using some
invalid opcodes where RA = RT.

An userspace program using an invalid instruction word like
0xe9ce0001, i.e. "ldu r14, 0(r14)", runs and exits without
getting terminated abruptly. The instruction performs the
load operation but does not write the effective address to
the base address register. Attaching an uprobe at that
instruction's address results in emulation which writes the
effective address to the base register. Thus, the final value
of the base address register is different.

To remove any inconsistencies, this adds an additional check
for the aforementioned instructions to make sure that they
are treated as unknown by the emulation infrastructure when
RA = 0 or RA = RT. The kernel will then fallback to executing
the instruction on hardware.

Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
 arch/powerpc/lib/sstep.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 855457ed09b5..25a5436be6c6 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -2157,11 +2157,15 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 
 		case 23:	/* lwzx */
 		case 55:	/* lwzux */
+			if (u && (ra == 0 || ra == rd))
+				return -1;
 			op->type = MKOP(LOAD, u, 4);
 			break;
 
 		case 87:	/* lbzx */
 		case 119:	/* lbzux */
+			if (u && (ra == 0 || ra == rd))
+				return -1;
 			op->type = MKOP(LOAD, u, 1);
 			break;
 
@@ -2215,6 +2219,8 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 #ifdef __powerpc64__
 		case 21:	/* ldx */
 		case 53:	/* ldux */
+			if (u && (ra == 0 || ra == rd))
+				return -1;
 			op->type = MKOP(LOAD, u, 8);
 			break;
 
@@ -2236,18 +2242,24 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 
 		case 279:	/* lhzx */
 		case 311:	/* lhzux */
+			if (u && (ra == 0 || ra == rd))
+				return -1;
 			op->type = MKOP(LOAD, u, 2);
 			break;
 
 #ifdef __powerpc64__
 		case 341:	/* lwax */
 		case 373:	/* lwaux */
+			if (u && (ra == 0 || ra == rd))
+				return -1;
 			op->type = MKOP(LOAD, SIGNEXT | u, 4);
 			break;
 #endif
 
 		case 343:	/* lhax */
 		case 375:	/* lhaux */
+			if (u && (ra == 0 || ra == rd))
+				return -1;
 			op->type = MKOP(LOAD, SIGNEXT | u, 2);
 			break;
 
@@ -2540,12 +2552,16 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 
 	case 32:	/* lwz */
 	case 33:	/* lwzu */
+		if (u && (ra == 0 || ra == rd))
+			return -1;
 		op->type = MKOP(LOAD, u, 4);
 		op->ea = dform_ea(word, regs);
 		break;
 
 	case 34:	/* lbz */
 	case 35:	/* lbzu */
+		if (u && (ra == 0 || ra == rd))
+			return -1;
 		op->type = MKOP(LOAD, u, 1);
 		op->ea = dform_ea(word, regs);
 		break;
@@ -2564,12 +2580,16 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 
 	case 40:	/* lhz */
 	case 41:	/* lhzu */
+		if (u && (ra == 0 || ra == rd))
+			return -1;
 		op->type = MKOP(LOAD, u, 2);
 		op->ea = dform_ea(word, regs);
 		break;
 
 	case 42:	/* lha */
 	case 43:	/* lhau */
+		if (u && (ra == 0 || ra == rd))
+			return -1;
 		op->type = MKOP(LOAD, SIGNEXT | u, 2);
 		op->ea = dform_ea(word, regs);
 		break;
@@ -2659,6 +2679,8 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 			op->type = MKOP(LOAD, 0, 8);
 			break;
 		case 1:		/* ldu */
+			if (ra == 0 || ra == rd)
+				return -1;
 			op->type = MKOP(LOAD, UPDATE, 8);
 			break;
 		case 2:		/* lwa */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] powerpc: sstep: Fix store and update instructions
  2020-11-19  5:41 [PATCH 1/2] powerpc: sstep: Fix load and update instructions Sandipan Das
@ 2020-11-19  5:41 ` Sandipan Das
  2020-11-25 10:09 ` [PATCH 1/2] powerpc: sstep: Fix load " Ravi Bangoria
  2020-11-26 10:36 ` Ravi Bangoria
  2 siblings, 0 replies; 5+ messages in thread
From: Sandipan Das @ 2020-11-19  5:41 UTC (permalink / raw)
  To: mpe; +Cc: ravi.bangoria, jniethe5, paulus, naveen.n.rao, linuxppc-dev, dja

The Power ISA says that the fixed-point store and update
instructions must not use R0 for the base address (RA).
In this case, the instruction is invalid. This applies
to the following instructions.
  * Store Byte with Update (stbu)
  * Store Byte with Update Indexed (stbux)
  * Store Halfword with Update (sthu)
  * Store Halfword with Update Indexed (sthux)
  * Store Word with Update (stwu)
  * Store Word with Update Indexed (stwux)
  * Store Doubleword with Update (stdu)
  * Store Doubleword with Update Indexed (stdux)

To remove any inconsistencies, this adds an additional check
for the aforementioned instructions to make sure that they
are treated as unknown by the emulation infrastructure when
RA = 0. The kernel will then fallback to executing the
instruction on hardware.

Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
 arch/powerpc/lib/sstep.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 25a5436be6c6..1c20c14f8757 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -2226,17 +2226,23 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 
 		case 149:	/* stdx */
 		case 181:	/* stdux */
+			if (u && ra == 0)
+				return -1;
 			op->type = MKOP(STORE, u, 8);
 			break;
 #endif
 
 		case 151:	/* stwx */
 		case 183:	/* stwux */
+			if (u && ra == 0)
+				return -1;
 			op->type = MKOP(STORE, u, 4);
 			break;
 
 		case 215:	/* stbx */
 		case 247:	/* stbux */
+			if (u && ra == 0)
+				return -1;
 			op->type = MKOP(STORE, u, 1);
 			break;
 
@@ -2265,6 +2271,8 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 
 		case 407:	/* sthx */
 		case 439:	/* sthux */
+			if (u && ra == 0)
+				return -1;
 			op->type = MKOP(STORE, u, 2);
 			break;
 
@@ -2568,12 +2576,16 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 
 	case 36:	/* stw */
 	case 37:	/* stwu */
+		if (u && ra == 0)
+			return -1;
 		op->type = MKOP(STORE, u, 4);
 		op->ea = dform_ea(word, regs);
 		break;
 
 	case 38:	/* stb */
 	case 39:	/* stbu */
+		if (u && ra == 0)
+			return -1;
 		op->type = MKOP(STORE, u, 1);
 		op->ea = dform_ea(word, regs);
 		break;
@@ -2596,6 +2608,8 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 
 	case 44:	/* sth */
 	case 45:	/* sthu */
+		if (u && ra == 0)
+			return -1;
 		op->type = MKOP(STORE, u, 2);
 		op->ea = dform_ea(word, regs);
 		break;
@@ -2746,6 +2760,8 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 			op->type = MKOP(STORE, 0, 8);
 			break;
 		case 1:		/* stdu */
+			if (ra == 0)
+				return -1;
 			op->type = MKOP(STORE, UPDATE, 8);
 			break;
 		case 2:		/* stq */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] powerpc: sstep: Fix load and update instructions
  2020-11-19  5:41 [PATCH 1/2] powerpc: sstep: Fix load and update instructions Sandipan Das
  2020-11-19  5:41 ` [PATCH 2/2] powerpc: sstep: Fix store " Sandipan Das
@ 2020-11-25 10:09 ` Ravi Bangoria
  2020-11-26  9:06   ` Sandipan Das
  2020-11-26 10:36 ` Ravi Bangoria
  2 siblings, 1 reply; 5+ messages in thread
From: Ravi Bangoria @ 2020-11-25 10:09 UTC (permalink / raw)
  To: Sandipan Das
  Cc: Ravi Bangoria, jniethe5, paulus, naveen.n.rao, linuxppc-dev, dja


> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index 855457ed09b5..25a5436be6c6 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -2157,11 +2157,15 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
>   
>   		case 23:	/* lwzx */
>   		case 55:	/* lwzux */
> +			if (u && (ra == 0 || ra == rd))
> +				return -1;

I guess you also need to split case 23 and 55?

- Ravi

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] powerpc: sstep: Fix load and update instructions
  2020-11-25 10:09 ` [PATCH 1/2] powerpc: sstep: Fix load " Ravi Bangoria
@ 2020-11-26  9:06   ` Sandipan Das
  0 siblings, 0 replies; 5+ messages in thread
From: Sandipan Das @ 2020-11-26  9:06 UTC (permalink / raw)
  To: Ravi Bangoria; +Cc: jniethe5, paulus, naveen.n.rao, linuxppc-dev, dja

Hi,

On 25/11/20 3:39 pm, Ravi Bangoria wrote:
> 
>> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
>> index 855457ed09b5..25a5436be6c6 100644
>> --- a/arch/powerpc/lib/sstep.c
>> +++ b/arch/powerpc/lib/sstep.c
>> @@ -2157,11 +2157,15 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
>>             case 23:    /* lwzx */
>>           case 55:    /* lwzux */
>> +            if (u && (ra == 0 || ra == rd))
>> +                return -1;
> 
> I guess you also need to split case 23 and 55?
> 

'u' takes care of that. It will be set for lwzux but not lwzx.

- Sandipan

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] powerpc: sstep: Fix load and update instructions
  2020-11-19  5:41 [PATCH 1/2] powerpc: sstep: Fix load and update instructions Sandipan Das
  2020-11-19  5:41 ` [PATCH 2/2] powerpc: sstep: Fix store " Sandipan Das
  2020-11-25 10:09 ` [PATCH 1/2] powerpc: sstep: Fix load " Ravi Bangoria
@ 2020-11-26 10:36 ` Ravi Bangoria
  2 siblings, 0 replies; 5+ messages in thread
From: Ravi Bangoria @ 2020-11-26 10:36 UTC (permalink / raw)
  To: Sandipan Das, mpe
  Cc: Ravi Bangoria, jniethe5, paulus, naveen.n.rao, linuxppc-dev, dja



On 11/19/20 11:11 AM, Sandipan Das wrote:
> The Power ISA says that the fixed-point load and update
> instructions must neither use R0 for the base address (RA)
> nor have the destination (RT) and the base address (RA) as
> the same register. In these cases, the instruction is
> invalid. This applies to the following instructions.
>    * Load Byte and Zero with Update (lbzu)
>    * Load Byte and Zero with Update Indexed (lbzux)
>    * Load Halfword and Zero with Update (lhzu)
>    * Load Halfword and Zero with Update Indexed (lhzux)
>    * Load Halfword Algebraic with Update (lhau)
>    * Load Halfword Algebraic with Update Indexed (lhaux)
>    * Load Word and Zero with Update (lwzu)
>    * Load Word and Zero with Update Indexed (lwzux)
>    * Load Word Algebraic with Update Indexed (lwaux)
>    * Load Doubleword with Update (ldu)
>    * Load Doubleword with Update Indexed (ldux)
> 
> However, the following behaviour is observed using some
> invalid opcodes where RA = RT.
> 
> An userspace program using an invalid instruction word like
> 0xe9ce0001, i.e. "ldu r14, 0(r14)", runs and exits without
> getting terminated abruptly. The instruction performs the
> load operation but does not write the effective address to
> the base address register. Attaching an uprobe at that
> instruction's address results in emulation which writes the
> effective address to the base register. Thus, the final value
> of the base address register is different.
> 
> To remove any inconsistencies, this adds an additional check
> for the aforementioned instructions to make sure that they
> are treated as unknown by the emulation infrastructure when
> RA = 0 or RA = RT. The kernel will then fallback to executing
> the instruction on hardware.
> 
> Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>

For the series:
Reviewed-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-11-26 10:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-19  5:41 [PATCH 1/2] powerpc: sstep: Fix load and update instructions Sandipan Das
2020-11-19  5:41 ` [PATCH 2/2] powerpc: sstep: Fix store " Sandipan Das
2020-11-25 10:09 ` [PATCH 1/2] powerpc: sstep: Fix load " Ravi Bangoria
2020-11-26  9:06   ` Sandipan Das
2020-11-26 10:36 ` Ravi Bangoria

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.