Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 15 Jun 2018 12:36:08 +0100
From:   Mark Rutland <mark.rutland@arm.com>
To:     Jin Yao <yao.jin@linux.intel.com>
Cc:     acme@kernel.org, jolsa@kernel.org, peterz@infradead.org,
        mingo@redhat.com, alexander.shishkin@linux.intel.com,
        me@kylehuey.com, Linux-kernel@vger.kernel.org,
        vincent.weaver@maine.edu, will.deacon@arm.com, eranian@google.com,
        namhyung@kernel.org, ak@linux.intel.com, kan.liang@intel.com,
        yao.jin@intel.com
Subject: Re: [PATCH v1 1/2] perf/core: Use sysctl to turn on/off dropping
 leaked kernel samples
Message-ID: <20180615113608.6m74sm7gpl5p6oqe@lakrids.cambridge.arm.com>
References: <1529057003-2212-1-git-send-email-yao.jin@linux.intel.com>
 <1529057003-2212-2-git-send-email-yao.jin@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1529057003-2212-2-git-send-email-yao.jin@linux.intel.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jun 15, 2018 at 06:03:22PM +0800, Jin Yao wrote:
> When doing sampling, for example:
> 
> perf record -e cycles:u ...
> 
> On workloads that do a lot of kernel entry/exits we see kernel
> samples, even though :u is specified. This is due to skid existing.
> 
> This might be a security issue because it can leak kernel addresses even
> though kernel sampling support is disabled.
> 
> One patch "perf/core: Drop kernel samples even though :u is specified"
> was posted in last year but it was reverted because it introduced a
> regression issue that broke the rr-project, which used sampling
> events to receive a signal on overflow. These signals were critical
> to the correct operation of rr.
> 
> See '6a8a75f32357 ("Revert "perf/core: Drop kernel samples even
> though :u is specified"")' for detail.
> 
> Now the idea is to use sysctl to control the dropping of leaked
> kernel samples.
> 
> /sys/devices/cpu/perf_allow_sample_leakage:
> 
> 0 - default, drop the leaked kernel samples.
> 1 - don't drop the leaked kernel samples.

Does this need to be conditional at all?

At least for sampling the GPRs, we could do something like below
unconditionally, which seems sufficient for my test cases.

Mark.

---->8----
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 67612ce359ad..79a21531d57c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6359,6 +6359,24 @@ perf_callchain(struct perf_event *event, struct pt_regs *regs)
        return callchain ?: &__empty_callchain;
 }
 
+static struct pt_regs *perf_get_sample_regs(struct perf_event *event, struct pt_regs *regs)
+{
+       /*
+        * Due to interrupt latency (AKA "skid"), we may enter the kernel
+        * before taking an overflow, even if the PMU is only counting user
+        * events.
+        *
+        * If we're not counting kernel events, always use the user regs when
+        * sampling.
+        *
+        * TODO: how does this interact with guest sampling?
+        */
+       if (event->attr.exclude_kernel && !user_mode(regs))
+               return task_pt_regs(current);
+
+       return regs;
+}
+
 void perf_prepare_sample(struct perf_event_header *header,
                         struct perf_sample_data *data,
                         struct perf_event *event,
@@ -6366,6 +6384,8 @@ void perf_prepare_sample(struct perf_event_header *header,
 {
        u64 sample_type = event->attr.sample_type;
 
+       regs = perf_get_sample_regs(event, regs);
+
        header->type = PERF_RECORD_SAMPLE;
        header->size = sizeof(*header) + event->header_size;