[PATCH 4.8 30/35] perf/x86: Restore TASK_SIZE check on frame pointer

[Greg Kroah-Hartman]

4.8-stable review patch.  If anyone has any objections, please let me know.

--

From: Johannes Weiner 

commit ae31fe51a3cceaa0cabdb3058f69669ecb47f12e upstream.

The following commit:

  75925e1ad7f5 ("perf/x86: Optimize stack walk user accesses")

... switched from copy_from_user_nmi() to __copy_from_user_nmi() with a manual
access_ok() check.

Unfortunately, copy_from_user_nmi() does an explicit check against TASK_SIZE,
whereas the access_ok() uses whatever the current address limit of the task is.

We are getting NMIs when __probe_kernel_read() has switched to KERNEL_DS, and
then see vmalloc faults when we access what looks like pointers into vmalloc
space:

  [] WARNING: CPU: 3 PID: 3685731 at arch/x86/mm/fault.c:435 
vmalloc_fault+0x289/0x290
  [] CPU: 3 PID: 3685731 Comm: sh Tainted: GW   
4.6.0-5_fbk1_223_gdbf0f40 #1
  [] Call Trace:
  [][] dump_stack+0x4d/0x6c
  []  [] __warn+0xd3/0xf0
  []  [] warn_slowpath_null+0x1d/0x20
  []  [] vmalloc_fault+0x289/0x290
  []  [] __do_page_fault+0x330/0x490
  []  [] do_page_fault+0xc/0x10
  []  [] page_fault+0x22/0x30
  []  [] ? perf_callchain_user+0x100/0x2a0
  []  [] get_perf_callchain+0x17f/0x190
  []  [] perf_callchain+0x67/0x80
  []  [] perf_prepare_sample+0x2a0/0x370
  []  [] perf_event_output+0x20/0x60
  []  [] ? perf_event_update_userpage+0xc7/0x130
  []  [] __perf_event_overflow+0x181/0x1d0
  []  [] perf_event_overflow+0x14/0x20
  []  [] intel_pmu_handle_irq+0x1d3/0x490
  []  [] ? copy_user_enhanced_fast_string+0x7/0x10
  []  [] ? vunmap_page_range+0x1a1/0x2f0
  []  [] ? unmap_kernel_range_noflush+0x11/0x20
  []  [] ? ghes_copy_tofrom_phys+0x116/0x1f0
  []  [] ? x2apic_send_IPI_self+0x1d/0x20
  []  [] perf_event_nmi_handler+0x2d/0x50
  []  [] nmi_handle+0x61/0x110
  []  [] default_do_nmi+0x44/0x110
  []  [] do_nmi+0xdb/0x150
  []  [] end_repeat_nmi+0x1a/0x1e
  []  [] ? copy_user_enhanced_fast_string+0x7/0x10
  []  [] ? copy_user_enhanced_fast_string+0x7/0x10
  []  [] ? copy_user_enhanced_fast_string+0x7/0x10
  []  <>[] ? __probe_kernel_read+0x3e/0xa0

Fix this by moving the valid_user_frame() check to before the uaccess
that loads the return address and the pointer to the next frame.

Signed-off-by: Johannes Weiner 
Signed-off-by: Peter Zijlstra (Intel) 
Cc: Alexander Shishkin 
Cc: Arnaldo Carvalho de Melo 
Cc: Jiri Olsa 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Stephane Eranian 
Cc: Thomas Gleixner 
Cc: Vince Weaver 
Cc: linux-kernel@vger.kernel.org
Fixes: 75925e1ad7f5 ("perf/x86: Optimize stack walk user accesses")
Signed-off-by: Ingo Molnar 
Signed-off-by: Greg Kroah-Hartman 

---
 arch/x86/events/core.c |   10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2344,7 +2344,7 @@ perf_callchain_user32(struct pt_regs *re
frame.next_frame = 0;
frame.return_address = 0;
 
-   if (!access_ok(VERIFY_READ, fp, 8))
+   if (!valid_user_frame(fp, sizeof(frame)))
break;
 
bytes = __copy_from_user_nmi(_frame, fp, 4);
@@ -2354,9 +2354,6 @@ perf_callchain_user32(struct pt_regs *re
if (bytes != 0)
break;
 
-   if (!valid_user_frame(fp, sizeof(frame)))
-   break;
-
perf_callchain_store(entry, cs_base + frame.return_address);
fp = compat_ptr(ss_base + frame.next_frame);
}
@@ -2405,7 +2402,7 @@ perf_callchain_user(struct perf_callchai
frame.next_frame = NULL;
frame.return_address = 0;
 
-   if (!access_ok(VERIFY_READ, fp, sizeof(*fp) * 2))
+   if (!valid_user_frame(fp, sizeof(frame)))
break;
 
bytes = __copy_from_user_nmi(_frame, fp, 
sizeof(*fp));
@@ -2415,9 +2412,6 @@ perf_callchain_user(struct perf_callchai
if (bytes != 0)
break;
 
-   if (!valid_user_frame(fp, sizeof(frame)))
-   break;
-
perf_callchain_store(entry, frame.return_address);
fp = (void __user *)frame.next_frame;
}

[PATCH 4.8 30/35] perf/x86: Restore TASK_SIZE check on frame pointer

[Greg Kroah-Hartman]

4.8-stable review patch.  If anyone has any objections, please let me know.

--

From: Johannes Weiner 

commit ae31fe51a3cceaa0cabdb3058f69669ecb47f12e upstream.

The following commit:

  75925e1ad7f5 ("perf/x86: Optimize stack walk user accesses")

... switched from copy_from_user_nmi() to __copy_from_user_nmi() with a manual
access_ok() check.

Unfortunately, copy_from_user_nmi() does an explicit check against TASK_SIZE,
whereas the access_ok() uses whatever the current address limit of the task is.

We are getting NMIs when __probe_kernel_read() has switched to KERNEL_DS, and
then see vmalloc faults when we access what looks like pointers into vmalloc
space:

  [] WARNING: CPU: 3 PID: 3685731 at arch/x86/mm/fault.c:435 
vmalloc_fault+0x289/0x290
  [] CPU: 3 PID: 3685731 Comm: sh Tainted: GW   
4.6.0-5_fbk1_223_gdbf0f40 #1
  [] Call Trace:
  [][] dump_stack+0x4d/0x6c
  []  [] __warn+0xd3/0xf0
  []  [] warn_slowpath_null+0x1d/0x20
  []  [] vmalloc_fault+0x289/0x290
  []  [] __do_page_fault+0x330/0x490
  []  [] do_page_fault+0xc/0x10
  []  [] page_fault+0x22/0x30
  []  [] ? perf_callchain_user+0x100/0x2a0
  []  [] get_perf_callchain+0x17f/0x190
  []  [] perf_callchain+0x67/0x80
  []  [] perf_prepare_sample+0x2a0/0x370
  []  [] perf_event_output+0x20/0x60
  []  [] ? perf_event_update_userpage+0xc7/0x130
  []  [] __perf_event_overflow+0x181/0x1d0
  []  [] perf_event_overflow+0x14/0x20
  []  [] intel_pmu_handle_irq+0x1d3/0x490
  []  [] ? copy_user_enhanced_fast_string+0x7/0x10
  []  [] ? vunmap_page_range+0x1a1/0x2f0
  []  [] ? unmap_kernel_range_noflush+0x11/0x20
  []  [] ? ghes_copy_tofrom_phys+0x116/0x1f0
  []  [] ? x2apic_send_IPI_self+0x1d/0x20
  []  [] perf_event_nmi_handler+0x2d/0x50
  []  [] nmi_handle+0x61/0x110
  []  [] default_do_nmi+0x44/0x110
  []  [] do_nmi+0xdb/0x150
  []  [] end_repeat_nmi+0x1a/0x1e
  []  [] ? copy_user_enhanced_fast_string+0x7/0x10
  []  [] ? copy_user_enhanced_fast_string+0x7/0x10
  []  [] ? copy_user_enhanced_fast_string+0x7/0x10
  []  <>[] ? __probe_kernel_read+0x3e/0xa0

Fix this by moving the valid_user_frame() check to before the uaccess
that loads the return address and the pointer to the next frame.

Signed-off-by: Johannes Weiner 
Signed-off-by: Peter Zijlstra (Intel) 
Cc: Alexander Shishkin 
Cc: Arnaldo Carvalho de Melo 
Cc: Jiri Olsa 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Stephane Eranian 
Cc: Thomas Gleixner 
Cc: Vince Weaver 
Cc: linux-kernel@vger.kernel.org
Fixes: 75925e1ad7f5 ("perf/x86: Optimize stack walk user accesses")
Signed-off-by: Ingo Molnar 
Signed-off-by: Greg Kroah-Hartman 

---
 arch/x86/events/core.c |   10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2344,7 +2344,7 @@ perf_callchain_user32(struct pt_regs *re
frame.next_frame = 0;
frame.return_address = 0;
 
-   if (!access_ok(VERIFY_READ, fp, 8))
+   if (!valid_user_frame(fp, sizeof(frame)))
break;
 
bytes = __copy_from_user_nmi(_frame, fp, 4);
@@ -2354,9 +2354,6 @@ perf_callchain_user32(struct pt_regs *re
if (bytes != 0)
break;
 
-   if (!valid_user_frame(fp, sizeof(frame)))
-   break;
-
perf_callchain_store(entry, cs_base + frame.return_address);
fp = compat_ptr(ss_base + frame.next_frame);
}
@@ -2405,7 +2402,7 @@ perf_callchain_user(struct perf_callchai
frame.next_frame = NULL;
frame.return_address = 0;
 
-   if (!access_ok(VERIFY_READ, fp, sizeof(*fp) * 2))
+   if (!valid_user_frame(fp, sizeof(frame)))
break;
 
bytes = __copy_from_user_nmi(_frame, fp, 
sizeof(*fp));
@@ -2415,9 +2412,6 @@ perf_callchain_user(struct perf_callchai
if (bytes != 0)
break;
 
-   if (!valid_user_frame(fp, sizeof(frame)))
-   break;
-
perf_callchain_store(entry, frame.return_address);
fp = (void __user *)frame.next_frame;
}