Skip to contentSkip to conversation

perf/x86: fix broken LBR fixup code

PATCH In review Stephane Eranian 3 participants 4 messages Jun 11, 2012 – Jun 14, 2012 x86

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 perf/x86: fix broken LBR fixup code

I noticed that the LBR fixups were not working anymore on programs where they used to. I tracked this down to a recent change to copy_from_user_nmi().

commit db0dc75d6403b6663c0eab4c6ccb672eb9b2ed72

Author: Arun Sharma <[email protected]>
Date:   Fri Apr 20 15:41:36 2012 -0700

perf/x86: Check user address explicitly in copy_from_user_nmi()

This commit added a call to __range_not_ok() to the copy_from_user_nmi() routine. The problem is that the logic of the test must be reversed. __range_not_ok() returns 0 if the range is VALID. We want to return early from copy_from_user_nmi() if the range is NOT valid.

---

arch/x86/lib/usercopy.c +1 -1
index 677b1ed..4f74d94 100644
@@ -22,7 +22,7 @@ copy_from_user_nmi(void *to, const void __user *from, unsigned long n)
 	void *map;
 	int ret;
 
-	if (__range_not_ok(from, n, TASK_SIZE) == 0)
+	if (__range_not_ok(from, n, TASK_SIZE))
 		return len;
 
 	do {
#1 lore raw
Quoted 8 lines · Stephane Eranian

I noticed that the LBR fixups were not working anymore on programs where they used to. I tracked this down to a recent change to copy_from_user_nmi().

commit db0dc75d6403b6663c0eab4c6ccb672eb9b2ed72

Author: Arun Sharma <[email protected]>
Date:   Fri Apr 20 15:41:36 2012 -0700
Quoted 7 lines

perf/x86: Check user address explicitly in copy_from_user_nmi()

This commit added a call to __range_not_ok() to the copy_from_user_nmi() routine. The problem is that the logic of the test must be reversed. __range_not_ok() returns 0 if the range is VALID. We want to return early from copy_from_user_nmi() if the range is NOT valid.

D'0h.. Thanks!

#2 lore raw
Quoted 11 lines · Peter Zijlstra
Quoted 10 lines · Stephane Eranian

I noticed that the LBR fixups were not working anymore on programs where they used to. I tracked this down to a recent change to copy_from_user_nmi().

commit db0dc75d6403b6663c0eab4c6ccb672eb9b2ed72

Author: Arun Sharma<[email protected]>
Date:   Fri Apr 20 15:41:36 2012 -0700

perf/x86: Check user address explicitly in copy_from_user_nmi()

Quoted 7 lines
Quoted 5 lines

This commit added a call to __range_not_ok() to the copy_from_user_nmi() routine. The problem is that the logic of the test must be reversed. __range_not_ok() returns 0 if the range is VALID. We want to return early from copy_from_user_nmi() if the range is NOT valid.

D'0h.. Thanks!

My bad. I was fooled by all the kernel addresses that were unwound properly. Didn't notice the broken unwinding in user space before I sent the patch. The fix looks good. Thanks.

-Arun

#3 lore raw

[tip:perf/urgent] perf/x86: Fix broken LBR fixup code

In reply to Stephane Eranian
Commit-ID:  25f42985825dd93f0593efe454e54c2aa13f7830
Gitweb:     http://git.kernel.org/tip/25f42985825dd93f0593efe454e54c2aa13f7830
Author:     Stephane Eranian <[email protected]>
AuthorDate: Mon, 11 Jun 2012 15:44:26 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 13 Jun 2012 15:00:28 +0200

perf/x86: Fix broken LBR fixup code

I noticed that the LBR fixups were not working anymore on programs where they used to. I tracked this down to a recent change to copy_from_user_nmi():

db0dc75d640 ("perf/x86: Check user address explicitly in copy_from_user_nmi()")

This commit added a call to __range_not_ok() to the copy_from_user_nmi() routine. The problem is that the logic of the test must be reversed. __range_not_ok() returns 0 if the range is VALID. We want to return early from copy_from_user_nmi() if the range is NOT valid.

---
 arch/x86/lib/usercopy.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/x86/lib/usercopy.c +1 -1
index 677b1ed..4f74d94 100644
@@ -22,7 +22,7 @@ copy_from_user_nmi(void *to, const void __user *from, unsigned long n)
 	void *map;
 	int ret;
 
-	if (__range_not_ok(from, n, TASK_SIZE) == 0)
+	if (__range_not_ok(from, n, TASK_SIZE))
 		return len;
 
 	do {