[PATCH 0/3] perf/x86/intel/pt: Address filtering fixes for perf/urgent

[PATCH 0/3] perf/x86/intel/pt: Address filtering fixes for perf/urgent

[Alexander Shishkin]

Hi,

Hoping that it's not too late, here are fixes for issues that Adrian
found. All three are good for -stable afaict. Two of the bugs result
in a #GP and one in a misconfigured filter.

2/3 can be moved into the PT driver as well, but I decided that others
may benefit from it at least theoretically.

Alexander Shishkin (3):
  perf/x86/intel/pt: Fix an off-by-one in address filter configuration
  perf/x86: Tighten up the kernel_ip() check
  perf/x86/intel/pt: Do validate the size of a kernel address filter

 arch/x86/events/intel/pt.c   | 13 +++++++++----
 arch/x86/events/perf_event.h |  2 +-
 2 files changed, 10 insertions(+), 5 deletions(-)

-- 
2.9.3

[PATCH 1/3] perf/x86/intel/pt: Fix an off-by-one in address filter configuration

[Alexander Shishkin]

PT address filter configuration requires that a range is specified by
its first and last address, but at the moment we're obtaining the end
of the range by adding user specified size to its start, which is off
by one from what it actually needs to be.

Fix this and make sure that zero-sized filters don't pass the filter
validation.

Cc: stable@vger.kernel.org # 4.7
Reported-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/events/intel/pt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index 04bb5fb5a8..5ec0100e3f 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -1081,7 +1081,7 @@ static int pt_event_addr_filters_validate(struct list_head *filters)
 
 	list_for_each_entry(filter, filters, entry) {
 		/* PT doesn't support single address triggers */
-		if (!filter->range)
+		if (!filter->range || !filter->size)
 			return -EOPNOTSUPP;
 
 		if (!filter->inode && !kernel_ip(filter->offset))
@@ -1111,7 +1111,7 @@ static void pt_event_addr_filters_sync(struct perf_event *event)
 		} else {
 			/* apply the offset */
 			msr_a = filter->offset + offs[range];
-			msr_b = filter->size + msr_a;
+			msr_b = filter->size + msr_a - 1;
 		}
 
 		filters->filter[range].msr_a  = msr_a;
-- 
2.9.3

[PATCH 2/3] perf/x86: Tighten up the kernel_ip() check

[Alexander Shishkin]

The kernel_ip() filter is used mostly by the DS/LBR code to look at the
branch addresses, but Intel PT also uses it to validate the address
filter offsets for kernel addresses, for which it is not sufficient:
supplying something in bits 64:48 that's not a sign extension of the lower
address bits (like 0xf00d000000000000) throws a #GP.

In the interest of improving everybody's kernel address checks, this
patch adds address validation to kernel_ip().

Cc: stable@vger.kernel.org # 4.7
Reported-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/events/perf_event.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index 5874d8de1f..88fb389356 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -783,7 +783,7 @@ static inline bool kernel_ip(unsigned long ip)
 #ifdef CONFIG_X86_32
 	return ip > PAGE_OFFSET;
 #else
-	return (long)ip < 0;
+	return (long)ip < 0 && virt_addr_valid(ip);
 #endif
 }
 
-- 
2.9.3

Re: [PATCH 2/3] perf/x86: Tighten up the kernel_ip() check

[Peter Zijlstra]

On Thu, Sep 15, 2016 at 04:29:43PM +0300, Alexander Shishkin wrote:
> The kernel_ip() filter is used mostly by the DS/LBR code to look at the
> branch addresses, but Intel PT also uses it to validate the address
> filter offsets for kernel addresses, for which it is not sufficient:
> supplying something in bits 64:48 that's not a sign extension of the lower
> address bits (like 0xf00d000000000000) throws a #GP.
> 
> In the interest of improving everybody's kernel address checks, this
> patch adds address validation to kernel_ip().
> 
> Cc: stable@vger.kernel.org # 4.7
> Reported-by: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> ---
>  arch/x86/events/perf_event.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
> index 5874d8de1f..88fb389356 100644
> --- a/arch/x86/events/perf_event.h
> +++ b/arch/x86/events/perf_event.h
> @@ -783,7 +783,7 @@ static inline bool kernel_ip(unsigned long ip)
>  #ifdef CONFIG_X86_32
>  	return ip > PAGE_OFFSET;
>  #else
> -	return (long)ip < 0;
> +	return (long)ip < 0 && virt_addr_valid(ip);
>  #endif
>  }

I have a slight performance worry, this ends up being called a _lot_ on
older machines that have to do the PEBS LBR-fixup.

[PATCH 3/3] perf/x86/intel/pt: Do validate the size of a kernel address filter

[Alexander Shishkin]

Right now, the kernel address filters in PT are prone to integer overflow
that may happen in adding filter's size to its offset to obtain the end
of the range. Such an overflow would also throw a #GP in the PT event
configuration path.

Fix this by explicitly validating the result of this calculation.

Cc: stable@vger.kernel.org # 4.7
Reported-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/events/intel/pt.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index 5ec0100e3f..834ce06b00 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -1084,8 +1084,13 @@ static int pt_event_addr_filters_validate(struct list_head *filters)
 		if (!filter->range || !filter->size)
 			return -EOPNOTSUPP;
 
-		if (!filter->inode && !kernel_ip(filter->offset))
-			return -EINVAL;
+		if (!filter->inode) {
+			if (!kernel_ip(filter->offset))
+				return -EINVAL;
+
+			if (!kernel_ip(filter->offset + filter->size))
+				return -EINVAL;
+		}
 
 		if (++range > pt_cap_get(PT_CAP_num_address_ranges))
 			return -EOPNOTSUPP;
-- 
2.9.3