Re: [PATCH v2 2/4] kprobes: split blacklist into common and arch

2013-04-05 Thread Vineet Gupta
On 04/04/2013 06:21 PM, Oskar Andero wrote:
> Some blackpoints are only valid for specific architectures. To let each
> architecture specify its own blackpoints the list has been split in two
> lists: common and arch. The common list is kept in kernel/kprobes.c and
> the arch list is kept in the arch/ directory.
> 
> Cc: Masami Hiramatsu 
> Cc: David S. Miller 
> Cc: linux-a...@vger.kernel.org
> Signed-off-by: Oskar Andero 
> ---
>  arch/arc/kernel/kprobes.c  |  3 ++
>  arch/arm/kernel/kprobes.c  |  2 +
>  arch/avr32/kernel/kprobes.c|  3 ++
>  arch/ia64/kernel/kprobes.c |  3 ++
>  arch/mips/kernel/kprobes.c |  3 ++
>  arch/mn10300/kernel/kprobes.c  |  2 +
>  arch/powerpc/kernel/kprobes.c  |  3 ++
>  arch/s390/kernel/kprobes.c |  3 ++
>  arch/sh/kernel/kprobes.c   |  3 ++
>  arch/sparc/kernel/kprobes.c|  3 ++
>  arch/x86/kernel/kprobes/core.c |  3 ++
>  kernel/kprobes.c   | 85 
> +++---
>  12 files changed, 86 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/arc/kernel/kprobes.c b/arch/arc/kernel/kprobes.c
> index 3bfeacb..894eee6 100644
> --- a/arch/arc/kernel/kprobes.c
> +++ b/arch/arc/kernel/kprobes.c
> @@ -24,6 +24,9 @@
>  DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
>  DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
>  
> +const char * const arch_kprobes_blacksyms[] = {};
> +const size_t arch_kprobes_blacksyms_size = 
> ARRAY_SIZE(arch_kprobes_blacksyms);
> +
>  int __kprobes arch_prepare_kprobe(struct kprobe *p)
>  {
>   /* Attempt to probe at unaligned address */
> diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
> index 170e9f3..772d9ec 100644
> --- a/arch/arm/kernel/kprobes.c
> +++ b/arch/arm/kernel/kprobes.c
> @@ -46,6 +46,8 @@
>  DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
>  DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
>  
> +const char * const arch_kprobes_blacksyms[] = {};
> +const size_t arch_kprobes_blacksyms_size = 
> ARRAY_SIZE(arch_kprobes_blacksyms);
>  
>  int __kprobes arch_prepare_kprobe(struct kprobe *p)
>  {
> diff --git a/arch/avr32/kernel/kprobes.c b/arch/avr32/kernel/kprobes.c
> index f820e9f..3b02c1e 100644
> --- a/arch/avr32/kernel/kprobes.c
> +++ b/arch/avr32/kernel/kprobes.c
> @@ -24,6 +24,9 @@ static struct pt_regs jprobe_saved_regs;
>  
>  struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
>  
> +const char * const arch_kprobes_blacksyms[] = {};
> +const size_t arch_kprobes_blacksyms_size = 
> ARRAY_SIZE(arch_kprobes_blacksyms);
> +
>  int __kprobes arch_prepare_kprobe(struct kprobe *p)
>  {
>   int ret = 0;
> diff --git a/arch/ia64/kernel/kprobes.c b/arch/ia64/kernel/kprobes.c
> index f8280a7..239f2fd 100644
> --- a/arch/ia64/kernel/kprobes.c
> +++ b/arch/ia64/kernel/kprobes.c
> @@ -42,6 +42,9 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
>  
>  struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
>  
> +const char * const arch_kprobes_blacksyms[] = {};
> +const size_t arch_kprobes_blacksyms_size = 
> ARRAY_SIZE(arch_kprobes_blacksyms);
> +
>  enum instruction_type {A, I, M, F, B, L, X, u};
>  static enum instruction_type bundle_encoding[32][3] = {
>{ M, I, I },   /* 00 */
> diff --git a/arch/mips/kernel/kprobes.c b/arch/mips/kernel/kprobes.c
> index 12bc4eb..de6a1aa 100644
> --- a/arch/mips/kernel/kprobes.c
> +++ b/arch/mips/kernel/kprobes.c
> @@ -53,6 +53,9 @@ static const union mips_instruction breakpoint2_insn = {
>  DEFINE_PER_CPU(struct kprobe *, current_kprobe);
>  DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
>  
> +const char * const arch_kprobes_blacksyms[] = {};
> +const size_t arch_kprobes_blacksyms_size = 
> ARRAY_SIZE(arch_kprobes_blacksyms);
> +
>  static int __kprobes insn_has_delayslot(union mips_instruction insn)
>  {
>   switch (insn.i_format.opcode) {
> diff --git a/arch/mn10300/kernel/kprobes.c b/arch/mn10300/kernel/kprobes.c
> index 0311a7f..ed57094 100644
> --- a/arch/mn10300/kernel/kprobes.c
> +++ b/arch/mn10300/kernel/kprobes.c
> @@ -41,6 +41,8 @@ static unsigned long cur_kprobe_bp_addr;
>  
>  DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
>  
> +const char * const arch_kprobes_blacksyms[] = {};
> +const size_t arch_kprobes_blacksyms_size = 
> ARRAY_SIZE(arch_kprobes_blacksyms);
>  
>  /* singlestep flag bits */
>  #define SINGLESTEP_BRANCH 1
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 11f5b03..b18ba45 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -47,6 +47,9 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
>  
>  struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
>  
> +const char * const arch_kprobes_blacksyms[] = {};
> +const size_t arch_kprobes_blacksyms_size = 
> ARRAY_SIZE(arch_kprobes_blacksyms);
> +
>  int __kprobes arch_prepare_kprobe(struct kprobe *p)
>  {
>   int ret = 0;
> diff --git 

Re: [PATCH v2 2/4] kprobes: split blacklist into common and arch

2013-04-05 Thread Vineet Gupta
On 04/04/2013 06:21 PM, Oskar Andero wrote:
 Some blackpoints are only valid for specific architectures. To let each
 architecture specify its own blackpoints the list has been split in two
 lists: common and arch. The common list is kept in kernel/kprobes.c and
 the arch list is kept in the arch/ directory.
 
 Cc: Masami Hiramatsu masami.hiramatsu...@hitachi.com
 Cc: David S. Miller da...@davemloft.net
 Cc: linux-a...@vger.kernel.org
 Signed-off-by: Oskar Andero oskar.and...@sonymobile.com
 ---
  arch/arc/kernel/kprobes.c  |  3 ++
  arch/arm/kernel/kprobes.c  |  2 +
  arch/avr32/kernel/kprobes.c|  3 ++
  arch/ia64/kernel/kprobes.c |  3 ++
  arch/mips/kernel/kprobes.c |  3 ++
  arch/mn10300/kernel/kprobes.c  |  2 +
  arch/powerpc/kernel/kprobes.c  |  3 ++
  arch/s390/kernel/kprobes.c |  3 ++
  arch/sh/kernel/kprobes.c   |  3 ++
  arch/sparc/kernel/kprobes.c|  3 ++
  arch/x86/kernel/kprobes/core.c |  3 ++
  kernel/kprobes.c   | 85 
 +++---
  12 files changed, 86 insertions(+), 30 deletions(-)
 
 diff --git a/arch/arc/kernel/kprobes.c b/arch/arc/kernel/kprobes.c
 index 3bfeacb..894eee6 100644
 --- a/arch/arc/kernel/kprobes.c
 +++ b/arch/arc/kernel/kprobes.c
 @@ -24,6 +24,9 @@
  DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
  DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
  
 +const char * const arch_kprobes_blacksyms[] = {};
 +const size_t arch_kprobes_blacksyms_size = 
 ARRAY_SIZE(arch_kprobes_blacksyms);
 +
  int __kprobes arch_prepare_kprobe(struct kprobe *p)
  {
   /* Attempt to probe at unaligned address */
 diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
 index 170e9f3..772d9ec 100644
 --- a/arch/arm/kernel/kprobes.c
 +++ b/arch/arm/kernel/kprobes.c
 @@ -46,6 +46,8 @@
  DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
  DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
  
 +const char * const arch_kprobes_blacksyms[] = {};
 +const size_t arch_kprobes_blacksyms_size = 
 ARRAY_SIZE(arch_kprobes_blacksyms);
  
  int __kprobes arch_prepare_kprobe(struct kprobe *p)
  {
 diff --git a/arch/avr32/kernel/kprobes.c b/arch/avr32/kernel/kprobes.c
 index f820e9f..3b02c1e 100644
 --- a/arch/avr32/kernel/kprobes.c
 +++ b/arch/avr32/kernel/kprobes.c
 @@ -24,6 +24,9 @@ static struct pt_regs jprobe_saved_regs;
  
  struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
  
 +const char * const arch_kprobes_blacksyms[] = {};
 +const size_t arch_kprobes_blacksyms_size = 
 ARRAY_SIZE(arch_kprobes_blacksyms);
 +
  int __kprobes arch_prepare_kprobe(struct kprobe *p)
  {
   int ret = 0;
 diff --git a/arch/ia64/kernel/kprobes.c b/arch/ia64/kernel/kprobes.c
 index f8280a7..239f2fd 100644
 --- a/arch/ia64/kernel/kprobes.c
 +++ b/arch/ia64/kernel/kprobes.c
 @@ -42,6 +42,9 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
  
  struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
  
 +const char * const arch_kprobes_blacksyms[] = {};
 +const size_t arch_kprobes_blacksyms_size = 
 ARRAY_SIZE(arch_kprobes_blacksyms);
 +
  enum instruction_type {A, I, M, F, B, L, X, u};
  static enum instruction_type bundle_encoding[32][3] = {
{ M, I, I },   /* 00 */
 diff --git a/arch/mips/kernel/kprobes.c b/arch/mips/kernel/kprobes.c
 index 12bc4eb..de6a1aa 100644
 --- a/arch/mips/kernel/kprobes.c
 +++ b/arch/mips/kernel/kprobes.c
 @@ -53,6 +53,9 @@ static const union mips_instruction breakpoint2_insn = {
  DEFINE_PER_CPU(struct kprobe *, current_kprobe);
  DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
  
 +const char * const arch_kprobes_blacksyms[] = {};
 +const size_t arch_kprobes_blacksyms_size = 
 ARRAY_SIZE(arch_kprobes_blacksyms);
 +
  static int __kprobes insn_has_delayslot(union mips_instruction insn)
  {
   switch (insn.i_format.opcode) {
 diff --git a/arch/mn10300/kernel/kprobes.c b/arch/mn10300/kernel/kprobes.c
 index 0311a7f..ed57094 100644
 --- a/arch/mn10300/kernel/kprobes.c
 +++ b/arch/mn10300/kernel/kprobes.c
 @@ -41,6 +41,8 @@ static unsigned long cur_kprobe_bp_addr;
  
  DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
  
 +const char * const arch_kprobes_blacksyms[] = {};
 +const size_t arch_kprobes_blacksyms_size = 
 ARRAY_SIZE(arch_kprobes_blacksyms);
  
  /* singlestep flag bits */
  #define SINGLESTEP_BRANCH 1
 diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
 index 11f5b03..b18ba45 100644
 --- a/arch/powerpc/kernel/kprobes.c
 +++ b/arch/powerpc/kernel/kprobes.c
 @@ -47,6 +47,9 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
  
  struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
  
 +const char * const arch_kprobes_blacksyms[] = {};
 +const size_t arch_kprobes_blacksyms_size = 
 ARRAY_SIZE(arch_kprobes_blacksyms);
 +
  int __kprobes arch_prepare_kprobe(struct kprobe *p)
  {
   int ret = 0;
 diff --git a/arch/s390/kernel/kprobes.c b/arch/s390/kernel/kprobes.c

Re: [PATCH v2 2/4] kprobes: split blacklist into common and arch

2013-04-04 Thread Masami Hiramatsu
(2013/04/04 21:51), Oskar Andero wrote:
> Some blackpoints are only valid for specific architectures. To let each
> architecture specify its own blackpoints the list has been split in two
> lists: common and arch. The common list is kept in kernel/kprobes.c and
> the arch list is kept in the arch/ directory.
> 

Here I missed one racing issue.

> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 0a270e5..7654278 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
[...]
>  
>  /* it can take some time ( > 100ms ) to initialise the
>   * blacklist so we delay this until we actually need it
>   */
>  static void init_kprobe_blacklist(void)
>  {
> - int i;
> - unsigned long offset = 0, size = 0;
> - char *modname, namebuf[128];
> - const char *symbol_name;
> - void *addr;
> - struct kprobe_blackpoint *kb;
> + int i, j = 0;
>  
>   mutex_lock(_mutex);
> - if (kprobe_blacklist_initialized)
> + if (kprobe_blacklist)
>   goto out;
>  
> + kprobe_blacklist_size = common_kprobes_blacksyms_size +
> + arch_kprobes_blacksyms_size;
> + kprobe_blacklist = kzalloc(sizeof(*kprobe_blacklist) *
> +kprobe_blacklist_size, GFP_KERNEL);

If you'd like to use kprobe_blacklist itself as an initialized flag,
you must prepare the "blacklist" local pointer to allocate and
initialize entries.

> @@ -151,7 +175,6 @@ static void init_kprobe_blacklist(void)
>  kretprobe_blacklist[i].name);
>   }
>   }
> - kprobe_blacklist_initialized = 1;

And after initialized, assign blacklist to kprobe_blacklist.

Without that, other thread may refer the uninitialized (but allocated)
black list.

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com


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


Re: [PATCH v2 2/4] kprobes: split blacklist into common and arch

2013-04-04 Thread Masami Hiramatsu
(2013/04/04 21:51), Oskar Andero wrote:
 Some blackpoints are only valid for specific architectures. To let each
 architecture specify its own blackpoints the list has been split in two
 lists: common and arch. The common list is kept in kernel/kprobes.c and
 the arch list is kept in the arch/ directory.
 

Here I missed one racing issue.

 diff --git a/kernel/kprobes.c b/kernel/kprobes.c
 index 0a270e5..7654278 100644
 --- a/kernel/kprobes.c
 +++ b/kernel/kprobes.c
[...]
  
  /* it can take some time (  100ms ) to initialise the
   * blacklist so we delay this until we actually need it
   */
  static void init_kprobe_blacklist(void)
  {
 - int i;
 - unsigned long offset = 0, size = 0;
 - char *modname, namebuf[128];
 - const char *symbol_name;
 - void *addr;
 - struct kprobe_blackpoint *kb;
 + int i, j = 0;
  
   mutex_lock(kprobe_mutex);
 - if (kprobe_blacklist_initialized)
 + if (kprobe_blacklist)
   goto out;
  
 + kprobe_blacklist_size = common_kprobes_blacksyms_size +
 + arch_kprobes_blacksyms_size;
 + kprobe_blacklist = kzalloc(sizeof(*kprobe_blacklist) *
 +kprobe_blacklist_size, GFP_KERNEL);

If you'd like to use kprobe_blacklist itself as an initialized flag,
you must prepare the blacklist local pointer to allocate and
initialize entries.

 @@ -151,7 +175,6 @@ static void init_kprobe_blacklist(void)
  kretprobe_blacklist[i].name);
   }
   }
 - kprobe_blacklist_initialized = 1;

And after initialized, assign blacklist to kprobe_blacklist.

Without that, other thread may refer the uninitialized (but allocated)
black list.

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com


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