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  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:	Sun, 8 Nov 2009 23:01:07 +0530
From:	"K.Prasad" <>
To:	Frederic Weisbecker <>
Cc:	Ingo Molnar <>, LKML <>,
	Li Zefan <>,
	Alan Stern <>,
	Peter Zijlstra <>,
	Arnaldo Carvalho de Melo <>,
	Steven Rostedt <>,
	Jan Kiszka <>,
	Jiri Slaby <>, Avi Kivity <>,
	Paul Mackerras <>,
	Mike Galbraith <>,
	Masami Hiramatsu <>,
	Paul Mundt <>,
	Arjan van de Ven <>
Subject: Re: [PATCH 5/7] hw-breakpoints: Rewrite the hw-breakpoints layer
	on top of perf events

>  #include <asm/smpboot_hooks.h>
> @@ -328,7 +327,6 @@ notrace static void __cpuinit start_secondary(void *unused)
>  	x86_cpuinit.setup_percpu_clockev();
>  	wmb();
> -	load_debug_registers();
>  	cpu_idle();
>  }

So, will the breakpoint values be stored in per-cpu variables and be
restored when a cpu (from the list of for_each_possible_cpu) eventually
turns online due to cpu-hotplug or resume from hibernate (as originally
done by load_debug_registers())?

A few more observations....

int reserve_bp_slot(struct perf_event *bp)
	if (!bp->attr.pinned) {
		 * If there are already flexible counters here,
		 * there is at least one slot reserved for all
		 * of them. Just join the party.
		 * Otherwise, check there is at least one free slot
		if (!slots.flexible && slots.pinned == HBP_NUM) {
			ret = -ENOSPC;
			goto end;

	/* Flexible counters need to keep at least one slot */
	} else if (slots.pinned + (!!slots.flexible) == HBP_NUM) {
		ret = -ENOSPC;
		goto end;

It appears that you're reserving one slot for the non-pinned breakpoint
requests, which I'm afraid wouldn't play well with PPC64 (having one
DABR). Even with x86, I can't understand why we should allow the use of
only 3 registers for ptrace requests (which are all pinned) on every
system even if perf-events will never be used on them.

int __register_perf_hw_breakpoint(struct perf_event *bp)
	if (!bp->attr.disabled)
		ret = arch_validate_hwbkpt_settings(bp, bp->ctx->task);

	return ret;

Can't the arch_validate_() check be done unconditionally?

struct perf_event *
modify_user_hw_breakpoint(struct perf_event *bp,
			  unsigned long addr,
			  int len,
			  int type,
			  perf_callback_t triggered,
			  struct task_struct *tsk,
			  bool active)
	 * FIXME: do it without unregistering
	 * - We don't want to lose our slot
	 * - If the new bp is incorrect, don't lose the older one

	return register_user_hw_breakpoint(addr, len, type, triggered,
					   tsk, active);

Given that modify_user_hw_breakpoint() is used by ptrace, there's a risk
of breaking/changing ptrace's behaviour i.e. new ptrace failure
scenarios (while modifying DR0-DR3) which may not be acceptable to
user-space applications. I presume that you intend to address the FIXME
during the next few iterations itself...

void arch_uninstall_hw_breakpoint(struct perf_event *bp)
	dr7 = &__get_cpu_var(dr7);
	*dr7 &= ~encode_dr7(i, info->len, info->type);

	set_debugreg(*dr7, 7);

The &= ~encode_dr7 would unset the GE flag in DR7 (encoded as
DR_GLOBAL_SLOWDOWN) which, I think is unintended.

struct pmu perf_ops_bp = {
        .enable         = arch_install_hw_breakpoint,
        .disable        = arch_uninstall_hw_breakpoint,
        .read           = hw_breakpoint_pmu_read,
        .unthrottle     = hw_breakpoint_pmu_unthrottle

So the enable/disable points to those routines that clear the debug
register off its breakpoint values but still don't seem to release the
slot (even temporarily)....i.e. nr_bp_flexible is unchanged
Will perf-events automatically re-use the disabled slot for breakpoint
requests from other perf-event instances?


To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

Powered by blists - more mailing lists