MIME-version: 1.0
Content-type: multipart/mixed; boundary="Boundary_(ID_NPhyFSWPjSt0KWkHUaE7Ig)"
Date: Thu, 17 Sep 2009 20:40:10 -0400 (EDT)
From: Nicolas Pitre <nico@fluxnic.net>
To: =?ISO-8859-15?Q?Fr=E9d=E9ric_RISS?= <frederic.riss@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org,
       Linux Kernel list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] arm: Make kprobes unregistration SMP safe
In-reply-to: <1253078476.5021.65.camel@funkybook>
Message-id: <alpine.LFD.2.00.0909172039070.24133@xanadu.home>
References: <1253078476.5021.65.camel@funkybook>
User-Agent: Alpine 2.00 (LFD 1167 2008-08-23)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2700
Lines: 77

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--Boundary_(ID_NPhyFSWPjSt0KWkHUaE7Ig)
Content-type: TEXT/PLAIN; charset=ISO-8859-1
Content-transfer-encoding: 8BIT

On Wed, 16 Sep 2009, Fr?d?ric RISS wrote:

> ARM kprobes use an illegal instruction to trigger kprobes. In the
> current implementation, there's a race between the unregistration of a
> kprobe and the illegal instruction exception handler if they run at the
> same time on different cores. 
> 
> When reading the value of the undefined instruction, the exception
> handler might get the original legal instruction as just patched
> concurrently by arch_disarm_kprobe(). When this happen the kprobe
> handler won't run, and thus the exception handler will oops because it
> believe it just hit an undefined instruction in kernel space.
> 
> The following patch synchronizes the code patching in the kprobes
> unregistration using stop_machine and thus avoids the above race.
> 
> Signed-off-by: Frederic RISS <frederic.riss@gmail.com>

Acked-by: Nicolas Pitre <nico@fluxnic.net>

> ---
> 
> diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
> index d28513f..6ada87d 100644
> --- a/arch/arm/kernel/kprobes.c
> +++ b/arch/arm/kernel/kprobes.c
> @@ -22,6 +22,7 @@
>  #include <linux/kernel.h>
>  #include <linux/kprobes.h>
>  #include <linux/module.h>
> +#include <linux/stop_machine.h>
>  #include <linux/stringify.h>
>  #include <asm/traps.h>
>  #include <asm/cacheflush.h>
> @@ -83,10 +84,24 @@ void __kprobes arch_arm_kprobe(struct kprobe *p)
>  	flush_insns(p->addr, 1);
>  }
>  
> +/*
> + * The actual disarming is done here on each CPU and synchronized using
> + * stop_machine. This synchronization is necessary on SMP to avoid removing
> + * a probe between the moment the 'Undefined Instruction' exception is raised
> + * and the moment the exception handler reads the faulting instruction from
> + * memory.
> + */
> +int __kprobes __arch_disarm_kprobe(void *p)
> +{
> +	struct kprobe *kp = p;
> +	*kp->addr = kp->opcode;
> +	flush_insns(kp->addr, 1);
> +	return 0;
> +}
> +
>  void __kprobes arch_disarm_kprobe(struct kprobe *p)
>  {
> -	*p->addr = p->opcode;
> -	flush_insns(p->addr, 1);
> +	stop_machine(__arch_disarm_kprobe, p, &cpu_online_map);
>  }
>  
>  void __kprobes arch_remove_kprobe(struct kprobe *p)
> 
> 

--Boundary_(ID_NPhyFSWPjSt0KWkHUaE7Ig)--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/