diff mbox series

[1/2] arm: CVE-2020-6096: fix memcpy and memmove for negative length [BZ #25620]

Message ID 01fb1a67a04f5db000744dcbd2b475a7211de3ee.1594144582.git.fweimer@redhat.com
State Committed
Headers
Series [1/2] arm: CVE-2020-6096: fix memcpy and memmove for negative length [BZ #25620] |

Commit Message

Florian Weimer July 7, 2020, 6:08 p.m. UTC 
  From: Evgeny Eremin <e.eremin@omprussia.ru>

Unsigned branch instructions could be used for r2 to fix the wrong
behavior when a negative length is passed to memcpy and memmove.
This commit fixes the generic arm implementation of memcpy amd memmove.

---
Please double-check for correct attribution and commit message
contents.  Thanks.  I will submit the NEWS update and XFAIL removal
separately.

 sysdeps/arm/memcpy.S  | 24 ++++++++++--------------
 sysdeps/arm/memmove.S | 24 ++++++++++--------------
 2 files changed, 20 insertions(+), 28 deletions(-)

Comments

Evgeny Eremin July 7, 2020, 8:56 p.m. UTC | #1 
On Tue, Jul 07, 2020 at 08:08:22PM +0200, Florian Weimer wrote:
> From: Evgeny Eremin <e.eremin@omprussia.ru>
> 
> Unsigned branch instructions could be used for r2 to fix the wrong
> behavior when a negative length is passed to memcpy and memmove.
> This commit fixes the generic arm implementation of memcpy amd memmove.
> 
> ---
> Please double-check for correct attribution and commit message
> contents.  Thanks.  I will submit the NEWS update and XFAIL removal
> separately.
> 
> ...

LGTM, thanks.
Carlos O'Donell July 7, 2020, 9:30 p.m. UTC | #2 
On 7/7/20 2:08 PM, Florian Weimer wrote:
> From: Evgeny Eremin <e.eremin@omprussia.ru>
> 
> Unsigned branch instructions could be used for r2 to fix the wrong
> behavior when a negative length is passed to memcpy and memmove.
> This commit fixes the generic arm implementation of memcpy amd memmove.

As a GNU Maintainer for glibc I have some visibility into the tickets
filed for FSF copyright assignment to glibc.

For the record both Open Mobile Platform LLC and Evgeny Eremin
have FSF copyright assignment on file for glibc. We can accept any
and all patches. Thank you all for your patience with the CLA process.

I can also confirm (and sorry for not responding more quickly on the
other thread) that all discussions between Open Mobile Platform LLC
were had with *@omprussia.ru accounts and the FSF Copyright &
Licensing Associate as part of ticket [gnu.org #1543679].

In this case *@omprussia.ru emails are by default considered to be
employees of Open Mobile Platform LLC for the purposes of the
copyright assignment, but we all know that might be inacccurate and
we trust everyone to maintain correct protocols for copyright
assignment.
Florian Weimer July 8, 2020, 5:50 a.m. UTC | #3 
* Evgeny Eremin:

> On Tue, Jul 07, 2020 at 08:08:22PM +0200, Florian Weimer wrote:
>> From: Evgeny Eremin <e.eremin@omprussia.ru>
>> 
>> Unsigned branch instructions could be used for r2 to fix the wrong
>> behavior when a negative length is passed to memcpy and memmove.
>> This commit fixes the generic arm implementation of memcpy amd memmove.
>> 
>> ---
>> Please double-check for correct attribution and commit message
>> contents.  Thanks.  I will submit the NEWS update and XFAIL removal
>> separately.
>> 
>> ...
>
> LGTM, thanks.

Is the other patch okay as well?  Thanks.
Alexander Anisimov July 8, 2020, 11:49 a.m. UTC | #4 
Yes, it's, patch for multiarch and commit message LGTM, too. Thanks.
Alexander Anisimov July 8, 2020, 12:09 p.m. UTC | #5 
>> On Tue, Jul 07, 2020 at 08:08:22PM +0200, Florian Weimer wrote:
>>> From: Evgeny Eremin <e.eremin@omprussia.ru>
>>>
>>> Unsigned branch instructions could be used for r2 to fix the wrong
>>> behavior when a negative length is passed to memcpy and memmove.
>>> This commit fixes the generic arm implementation of memcpy amd memmove.
>>>
>>> ---
>>> Please double-check for correct attribution and commit message
>>> contents.  Thanks.  I will submit the NEWS update and XFAIL removal
>>> separately.
>>>
>>> ...
>>
>> LGTM, thanks.

> Is the other patch okay as well?  Thanks.

Yes, it's, patch for multiarch and commit message LGTM, too. Thanks.
diff mbox series

Patch

diff --git a/sysdeps/arm/memcpy.S b/sysdeps/arm/memcpy.S
index 510e8adaf2..bcfbc51d99 100644
--- a/sysdeps/arm/memcpy.S
+++ b/sysdeps/arm/memcpy.S
@@ -68,7 +68,7 @@  ENTRY(memcpy)
 		cfi_remember_state
 
 		subs	r2, r2, #4
-		blt	8f
+		blo	8f
 		ands	ip, r0, #3
 	PLD(	pld	[r1, #0]		)
 		bne	9f
@@ -82,7 +82,7 @@  ENTRY(memcpy)
 		cfi_rel_offset (r6, 4)
 		cfi_rel_offset (r7, 8)
 		cfi_rel_offset (r8, 12)
-		blt	5f
+		blo	5f
 
 	CALGN(	ands	ip, r1, #31		)
 	CALGN(	rsb	r3, ip, #32		)
@@ -98,9 +98,9 @@  ENTRY(memcpy)
 #endif
 
 	PLD(	pld	[r1, #0]		)
-2:	PLD(	subs	r2, r2, #96		)
+2:	PLD(	cmp	r2, #96			)
 	PLD(	pld	[r1, #28]		)
-	PLD(	blt	4f			)
+	PLD(	blo	4f			)
 	PLD(	pld	[r1, #60]		)
 	PLD(	pld	[r1, #92]		)
 
@@ -108,9 +108,7 @@  ENTRY(memcpy)
 4:		ldmia	r1!, {r3, r4, r5, r6, r7, r8, ip, lr}
 		subs	r2, r2, #32
 		stmia	r0!, {r3, r4, r5, r6, r7, r8, ip, lr}
-		bge	3b
-	PLD(	cmn	r2, #96			)
-	PLD(	bge	4b			)
+		bhs	3b
 
 5:		ands	ip, r2, #28
 		rsb	ip, ip, #32
@@ -222,7 +220,7 @@  ENTRY(memcpy)
 		strbge	r4, [r0], #1
 		subs	r2, r2, ip
 		strb	lr, [r0], #1
-		blt	8b
+		blo	8b
 		ands	ip, r1, #3
 		beq	1b
 
@@ -236,7 +234,7 @@  ENTRY(memcpy)
 		.macro	forward_copy_shift pull push
 
 		subs	r2, r2, #28
-		blt	14f
+		blo	14f
 
 	CALGN(	ands	ip, r1, #31		)
 	CALGN(	rsb	ip, ip, #32		)
@@ -253,9 +251,9 @@  ENTRY(memcpy)
 		cfi_rel_offset (r10, 16)
 
 	PLD(	pld	[r1, #0]		)
-	PLD(	subs	r2, r2, #96		)
+	PLD(	cmp	r2, #96			)
 	PLD(	pld	[r1, #28]		)
-	PLD(	blt	13f			)
+	PLD(	blo	13f			)
 	PLD(	pld	[r1, #60]		)
 	PLD(	pld	[r1, #92]		)
 
@@ -280,9 +278,7 @@  ENTRY(memcpy)
 		mov	ip, ip, PULL #\pull
 		orr	ip, ip, lr, PUSH #\push
 		stmia	r0!, {r3, r4, r5, r6, r7, r8, r10, ip}
-		bge	12b
-	PLD(	cmn	r2, #96			)
-	PLD(	bge	13b			)
+		bhs	12b
 
 		pop	{r5 - r8, r10}
 		cfi_adjust_cfa_offset (-20)
diff --git a/sysdeps/arm/memmove.S b/sysdeps/arm/memmove.S
index 954037ef3a..0d07b76ee6 100644
--- a/sysdeps/arm/memmove.S
+++ b/sysdeps/arm/memmove.S
@@ -85,7 +85,7 @@  ENTRY(memmove)
 		add	r1, r1, r2
 		add	r0, r0, r2
 		subs	r2, r2, #4
-		blt	8f
+		blo	8f
 		ands	ip, r0, #3
 	PLD(	pld	[r1, #-4]		)
 		bne	9f
@@ -99,7 +99,7 @@  ENTRY(memmove)
 		cfi_rel_offset (r6, 4)
 		cfi_rel_offset (r7, 8)
 		cfi_rel_offset (r8, 12)
-		blt	5f
+		blo     5f
 
 	CALGN(	ands	ip, r1, #31		)
 	CALGN(	sbcsne	r4, ip, r2		)  @ C is always set here
@@ -114,9 +114,9 @@  ENTRY(memmove)
 #endif
 
 	PLD(	pld	[r1, #-4]		)
-2:	PLD(	subs	r2, r2, #96		)
+2:	PLD(	cmp	r2, #96			)
 	PLD(	pld	[r1, #-32]		)
-	PLD(	blt	4f			)
+	PLD(    blo     4f                      )
 	PLD(	pld	[r1, #-64]		)
 	PLD(	pld	[r1, #-96]		)
 
@@ -124,9 +124,7 @@  ENTRY(memmove)
 4:		ldmdb	r1!, {r3, r4, r5, r6, r7, r8, ip, lr}
 		subs	r2, r2, #32
 		stmdb	r0!, {r3, r4, r5, r6, r7, r8, ip, lr}
-		bge	3b
-	PLD(	cmn	r2, #96			)
-	PLD(	bge	4b			)
+		bhs     3b
 
 5:		ands	ip, r2, #28
 		rsb	ip, ip, #32
@@ -237,7 +235,7 @@  ENTRY(memmove)
 		strbge	r4, [r0, #-1]!
 		subs	r2, r2, ip
 		strb	lr, [r0, #-1]!
-		blt	8b
+		blo	8b
 		ands	ip, r1, #3
 		beq	1b
 
@@ -251,7 +249,7 @@  ENTRY(memmove)
 		.macro	backward_copy_shift push pull
 
 		subs	r2, r2, #28
-		blt	14f
+		blo	14f
 
 	CALGN(	ands	ip, r1, #31		)
 	CALGN(	rsb	ip, ip, #32		)
@@ -268,9 +266,9 @@  ENTRY(memmove)
 		cfi_rel_offset (r10, 16)
 
 	PLD(	pld	[r1, #-4]		)
-	PLD(	subs	r2, r2, #96		)
+	PLD(	cmp	r2, #96			)
 	PLD(	pld	[r1, #-32]		)
-	PLD(	blt	13f			)
+	PLD(	blo	13f			)
 	PLD(	pld	[r1, #-64]		)
 	PLD(	pld	[r1, #-96]		)
 
@@ -295,9 +293,7 @@  ENTRY(memmove)
 		mov     r4, r4, PUSH #\push
 		orr     r4, r4, r3, PULL #\pull
 		stmdb   r0!, {r4 - r8, r10, ip, lr}
-		bge	12b
-	PLD(	cmn	r2, #96			)
-	PLD(	bge	13b			)
+		bhs	12b
 
 		pop	{r5 - r8, r10}
 		cfi_adjust_cfa_offset (-20)