linux-kernel - Re: [PATCH V7 2/2] perf/x86: Reset the dirty counter to prevent the leak for an RDPMC task

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 14 May 2021 16:44:14 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     "Liang, Kan" <kan.liang@...ux.intel.com>
Cc:     mingo@...hat.com, linux-kernel@...r.kernel.org, robh@...nel.org,
        ak@...ux.intel.com, acme@...nel.org, mark.rutland@....com,
        luto@...capital.net, eranian@...gle.com, namhyung@...nel.org
Subject: Re: [PATCH V7 2/2] perf/x86: Reset the dirty counter to prevent the
 leak for an RDPMC task

On Thu, May 13, 2021 at 06:14:08PM -0400, Liang, Kan wrote:
> On 5/13/2021 11:02 AM, Peter Zijlstra wrote:
> > On Thu, May 13, 2021 at 07:23:02AM -0700, kan.liang@...ux.intel.com wrote:
> > 
> > > +	if (x86_pmu.sched_task && event->hw.target) {
> > > +		atomic_inc(&event->pmu->sched_cb_usage);
> > > +		local_irq_save(flags);
> > > +		x86_pmu_clear_dirty_counters();
> > > +		local_irq_restore(flags);
> > > +	}
> > 
> > So what happens if our mmap() happens after we've already created two
> > (or more) threads in the process, all of who already have a counter (or
> > more) on?
> > 
> > Shouldn't this be something like?
> 
> That's not enough.
> 
> I implemented a test case as below:
> - The main thread A creates a new thread B.
> - Bind the thread A to CPU 0. Then the thread A opens a event, mmap, enable
> the event, and sleep.
> - Bind the thread B to CPU 1. Wait until the event in the thread A is
> enabled. Then RDPMC can read the counters on CPU 1.

This?

	A				B

	clone(CLONE_THREAD) --->	
	set_affine(0)
					set_affine(1)
					while (!event-enabled)
						;
	event = perf_event_open()
	mmap(event)
	ioctl(event, IOC_ENABLE); --->
					RDPMC

	sleep(n)
	  schedule(INTERRUPTIBLE)
	  /* idle */


> In the x86_pmu_event_mapped(), we do on_each_cpu_mask(mm_cpumask(mm),
> cr4_update_pce, NULL, 1);
> The RDPMC from thread B on CPU 1 is not forbidden.
> Since the counter is not created in thread B, the sched_task() never gets a
> chance to be invoked. The dirty counter is not cleared.

Per-task counters from CPU1 that ran before B ran?

> To fix it, I think we have to move the cr4_update_pce() to the context
> switch, and update it only when the RDPMC task is scheduled. But it probably
> brings some overhead.

We have CR4:PCE updates in the context switch path, see
switch_mm_irqs_off() -> cr4_update_pce_mm().

Doing the clear there might actually make sense and avoids this frobbing
of ->sched_task(). When we call cr4_update_pce_mm(), and @mm has rdpmc
on, clear dirty or something like that.

Worth a try.