[Arm] Fix range check for SMC immediate operand.

Message ID fefa466d-cb0a-b4a7-0108-703d26ffd067@arm.com
State New
Headers show
Series
  • [Arm] Fix range check for SMC immediate operand.
Related show

Commit Message

Barnaby Wilks June 26, 2019, 3:11 p.m.
Hello,

This patch fixes a bug where an immediate operand larger than 4 bits (0xF) could be passed
to the SMC (Secure Monitor Call) instruction.

For example, this code is invalid:
smc #0x6951

The code would previously check for and encode for up to 16 bit immediate values, however
this immediate should instead be only a 4 bit value
(as documented herehttps://static.docs.arm.com/ddi0406/c/DDI0406C_C_arm_architecture_reference_manual.pdf  ).

Fixed this by adding range checks in the relevant areas and also removing code that would
encode more than the first 4 bits of the immediate (code that is now redundant, as any immediate operand
larger than 0xF would error now anyway).

Added testcases to check that the error is thrown for invalid instructions (for arm and thumb),
as well as positive tests checking that any immediate operands less than 0xF are accepted.

Cross compiled and regtested on arm-none-eabi and arm-none-linux-gnueabihf.

I don't have write access, so if it's OK then could someone commit on my behalf?

Thanks,
Barney

gas/ChangeLog:

2019-06-26  Barnaby Wilks<barnaby.wilks@arm.com>

	* config/tc-arm.c (do_smc): Add range check for immediate operand.
	(do_t_smc): Add range check for immediate operand. Remove
	obsolete immediate encoding.
	(md_apply_fix): Fix range check. Remove obsolete immediate encoding.
	* testsuite/gas/arm/arch6zk.d: Fix test.
	* testsuite/gas/arm/arch6zk.s: Fix test.
	* testsuite/gas/arm/smc-bad.d: New test.
	* testsuite/gas/arm/smc-bad.l: New test.
	* testsuite/gas/arm/smc-bad.s: New test.
	* testsuite/gas/arm/thumb32.d: Fix test.
	* testsuite/gas/arm/thumb32.s: Fix test.

Comments

Nick Clifton June 27, 2019, 1:08 p.m. | #1
Hi Barnaby,

> I don't have write access, so if it's OK then could someone commit on my behalf?


Approved and applied.

Cheers
  Nick

> gas/ChangeLog:

> 2019-06-26  Barnaby Wilks<barnaby.wilks@arm.com>

> 

> 	* config/tc-arm.c (do_smc): Add range check for immediate operand.

> 	(do_t_smc): Add range check for immediate operand. Remove

> 	obsolete immediate encoding.

> 	(md_apply_fix): Fix range check. Remove obsolete immediate encoding.

> 	* testsuite/gas/arm/arch6zk.d: Fix test.

> 	* testsuite/gas/arm/arch6zk.s: Fix test.

> 	* testsuite/gas/arm/smc-bad.d: New test.

> 	* testsuite/gas/arm/smc-bad.l: New test.

> 	* testsuite/gas/arm/smc-bad.s: New test.

> 	* testsuite/gas/arm/thumb32.d: Fix test.

> 	* testsuite/gas/arm/thumb32.s: Fix test.

>

Patch

diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 679361d951e27853f58db5d00cbf7dea8707144e..b70028f7c2e31c179196b3c9ea9665fbf4b0159f 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -10247,6 +10247,9 @@  do_shift (void)
 static void
 do_smc (void)
 {
+  unsigned int value = inst.relocs[0].exp.X_add_number;
+  constraint (value > 0xf, _("immediate too large (bigger than 0xF)"));
+
   inst.relocs[0].type = BFD_RELOC_ARM_SMC;
   inst.relocs[0].pc_rel = 0;
 }
@@ -13877,10 +13880,11 @@  do_t_smc (void)
 	      _("SMC is not permitted on this architecture"));
   constraint (inst.relocs[0].exp.X_op != O_constant,
 	      _("expression too complex"));
+  constraint (value > 0xf, _("immediate too large (bigger than 0xF)"));
+
   inst.relocs[0].type = BFD_RELOC_UNUSED;
-  inst.instruction |= (value & 0xf000) >> 12;
-  inst.instruction |= (value & 0x0ff0);
   inst.instruction |= (value & 0x000f) << 16;
+
   /* PR gas/15623: SMC instructions must be last in an IT block.  */
   set_pred_insn_type_last ();
 }
@@ -27765,11 +27769,12 @@  md_apply_fix (fixS *	fixP,
       break;
 
     case BFD_RELOC_ARM_SMC:
-      if (((unsigned long) value) > 0xffff)
+      if (((unsigned long) value) > 0xf)
 	as_bad_where (fixP->fx_file, fixP->fx_line,
 		      _("invalid smc expression"));
+
       newval = md_chars_to_number (buf, INSN_SIZE);
-      newval |= (value & 0xf) | ((value & 0xfff0) << 4);
+      newval |= (value & 0xf);
       md_number_to_chars (buf, newval, INSN_SIZE);
       break;
 
diff --git a/gas/testsuite/gas/arm/arch6zk.d b/gas/testsuite/gas/arm/arch6zk.d
index 5ec8def063e1e76897289b7786cefeb5c632d3d9..f9ae025295f562a1987dc5abf71134d7a137f036 100644
--- a/gas/testsuite/gas/arm/arch6zk.d
+++ b/gas/testsuite/gas/arm/arch6zk.d
@@ -24,6 +24,6 @@  Disassembly of section .text:
 0+040 <[^>]*> e320f002 ?	wfe
 0+044 <[^>]*> e320f003 ?	wfi
 0+048 <[^>]*> e320f001 ?	yield
-0+04c <[^>]*> e16ec371 ?	smc	60465.*
-0+050 <[^>]*> 11613c7e ?	smcne	5070.*
+0+04c <[^>]*> e1600071 ?	smc	1.*
+0+050 <[^>]*> 1160007e ?	smcne	14.*
 #...
diff --git a/gas/testsuite/gas/arm/arch6zk.s b/gas/testsuite/gas/arm/arch6zk.s
index 19c2c6525a014591f9b32274160b3ef502e44987..db2e0b29e0d2d7ab046c357f520e16d1987b7bf2 100644
--- a/gas/testsuite/gas/arm/arch6zk.s
+++ b/gas/testsuite/gas/arm/arch6zk.s
@@ -23,8 +23,8 @@  label:
 	wfi
 	yield
 	# ARMV6Z instructions
-	smc 0xec31
-	smcne 0x13ce
+	smc 0x1
+	smcne 0xe
 
 	# Ensure output is 32-byte aligned as required for arm-aout.
 	.p2align 5
diff --git a/gas/testsuite/gas/arm/smc-bad.d b/gas/testsuite/gas/arm/smc-bad.d
new file mode 100644
index 0000000000000000000000000000000000000000..ca3a5a5335130decf1ea92a685ca1c026fc01c04
--- /dev/null
+++ b/gas/testsuite/gas/arm/smc-bad.d
@@ -0,0 +1,3 @@ 
+# name: Invalid SMC operand test
+# source: smc-bad.s
+# error_output: smc-bad.l
diff --git a/gas/testsuite/gas/arm/smc-bad.l b/gas/testsuite/gas/arm/smc-bad.l
new file mode 100644
index 0000000000000000000000000000000000000000..ac206e0673df0bfa617c6d34d9ed02023e337d54
--- /dev/null
+++ b/gas/testsuite/gas/arm/smc-bad.l
@@ -0,0 +1,8 @@ 
+.*smc-bad.s: Assembler messages:
+.*smc-bad.s:2: Error: immediate too large \(bigger than 0xF\) -- `smc #0xfefe'
+.*smc-bad.s:3: Error: immediate too large \(bigger than 0xF\) -- `smc #0x12'
+.*smc-bad.s:4: Error: immediate too large \(bigger than 0xF\) -- `smc 123'
+.*smc-bad.s:7: Error: immediate too large \(bigger than 0xF\) -- `smc #0xdfd'
+.*smc-bad.s:8: Error: immediate too large \(bigger than 0xF\) -- `smc #0x43'
+.*smc-bad.s:9: Error: immediate too large \(bigger than 0xF\) -- `smc 4343343'
+.*smc-bad.s:14: Error: immediate too large \(bigger than 0xF\) -- `smc #0x6951'
diff --git a/gas/testsuite/gas/arm/smc-bad.s b/gas/testsuite/gas/arm/smc-bad.s
new file mode 100644
index 0000000000000000000000000000000000000000..04453ae7aadff34b26055271a0590ed2218f410c
--- /dev/null
+++ b/gas/testsuite/gas/arm/smc-bad.s
@@ -0,0 +1,14 @@ 
+.arm
+	smc #0xfefe
+	smc #0x12
+	smc 123
+
+.thumb
+	smc #0xdfd
+	smc #0x43
+	smc 4343343
+
+.arm
+.syntax unified
+.cpu cortex-a8
+	smc #0x6951
diff --git a/gas/testsuite/gas/arm/thumb32.d b/gas/testsuite/gas/arm/thumb32.d
index 8a812623feef4fae7134fab4ac0b7ef89824e604..f265f5af0c150b91a9e2d2e94cd21b65c134944e 100644
--- a/gas/testsuite/gas/arm/thumb32.d
+++ b/gas/testsuite/gas/arm/thumb32.d
@@ -844,7 +844,7 @@  Disassembly of section .text:
 0[0-9a-f]+ <[^>]+> ea4f 0132 	mov.w	r1, r2, rrx
 0[0-9a-f]+ <[^>]+> ea5f 0334 	movs.w	r3, r4, rrx
 0[0-9a-f]+ <[^>]+> f7f0 8000 	smc	#0
-0[0-9a-f]+ <[^>]+> f7fd 8bca 	smc	#43981	; 0xabcd
+0[0-9a-f]+ <[^>]+> f7fd 8000 	smc	#13
 0[0-9a-f]+ <[^>]+> fb10 0000 	smlabb	r0, r0, r0, r0
 0[0-9a-f]+ <[^>]+> fb10 0900 	smlabb	r9, r0, r0, r0
 0[0-9a-f]+ <[^>]+> fb19 0000 	smlabb	r0, r9, r0, r0
diff --git a/gas/testsuite/gas/arm/thumb32.s b/gas/testsuite/gas/arm/thumb32.s
index d6dbdd651fe5604af16469305fd2d51b04608c1e..5f8cb8337ed9ae031ec44797dd18bd7565143dca 100644
--- a/gas/testsuite/gas/arm/thumb32.s
+++ b/gas/testsuite/gas/arm/thumb32.s
@@ -637,7 +637,7 @@  rrx:
 	.arch_extension sec
 smc:
 	smc	#0
-	smc	#0xabcd
+	smc	#0xd
 
 smla:
 	smlabb	r0, r0, r0, r0