linux-kernel - Re: [PATCH 4/6] hw-breakpoints: Rewrite the hw-breakpoints layer on top of perf events

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:	Mon, 02 Nov 2009 08:45:56 +0100
From:	Jan Kiszka <jan.kiszka@....de>
To:	Frederic Weisbecker <fweisbec@...il.com>
CC:	Ingo Molnar <mingo@...e.hu>, LKML <linux-kernel@...r.kernel.org>,
	Prasad <prasad@...ux.vnet.ibm.com>,
	Alan Stern <stern@...land.harvard.edu>,
	Peter Zijlstra <peterz@...radead.org>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Jiri Slaby <jirislaby@...il.com>,
	Li Zefan <lizf@...fujitsu.com>, Avi Kivity <avi@...hat.com>,
	Paul Mackerras <paulus@...ba.org>,
	Mike Galbraith <efault@....de>,
	Masami Hiramatsu <mhiramat@...hat.com>,
	Paul Mundt <lethal@...ux-sh.org>
Subject: Re: [PATCH 4/6] hw-breakpoints: Rewrite the hw-breakpoints layer
 on top of perf events

Frederic Weisbecker wrote:
> On Sun, Nov 01, 2009 at 11:09:03PM +0100, Jan Kiszka wrote:
>>> @@ -3643,14 +3644,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>>  	trace_kvm_entry(vcpu->vcpu_id);
>>>  	kvm_x86_ops->run(vcpu, kvm_run);
>>>  
>>> -	if (unlikely(vcpu->arch.switch_db_regs || test_thread_flag(TIF_DEBUG))) {
>>> -		set_debugreg(current->thread.debugreg[0], 0);
>>> -		set_debugreg(current->thread.debugreg[1], 1);
>>> -		set_debugreg(current->thread.debugreg[2], 2);
>>> -		set_debugreg(current->thread.debugreg[3], 3);
>>> -		set_debugreg(current->thread.debugreg6, 6);
>>> -		set_debugreg(current->thread.debugreg7, 7);
>>> -	}
>>> +	if (unlikely(vcpu->arch.switch_db_regs || test_thread_flag(TIF_DEBUG)))
>>> +		hw_breakpoint_restore();
>> TIF_DEBUG is only set on active ptrace hw-breakpoints, thus we miss
>> other types here, right? (Note: arch.switch_db_regs is guest-related,
>> thus does not help in this regard.)
>>
>> Jan
>>
> 
> 
> About this. vcpu->arch.switch_db_regs is guest related but it looks
> like the only thing I need to check.
> 
> I'm not sure when it is activated. Is it always done once the guest
> changes its debug registers? I suspect there is a corner case.

It's set when we had to write to some debugreg[0..4], either for use by
the guest itself or for debugging it from the host. It used to be the
only condition for switching on exit as we saved the registers on entry
(under the same condition). This was reworked recently to avoid the
entry saving.

> 
> Because since I can't anymore assume TIF_DEBUG covers every
> breakpoints uses, it means I'll need to maintain a refcount of
> breakpoints in use.
> Well, I have one already, but it is splitted into several refcounts
> (per task events, per cpu, non-pinned, etc...). And since
> vcpu_enter_guest() is a fast path, I'll need to maintain another global
> per cpu one, without lock or further operations to know if we need
> to save the debug registers, just a simple check.
> 

I'm not 100% sure right now if we still need "switch_db_reg" in case we
have a reliable indicator that the host requires properly set registers.
ATM I would dare to say, we don't, but I need to think about this again.

Jan


Download attachment "signature.asc" of type "application/pgp-signature" (258 bytes)