On Sun, 17 May 2020 14:48:52 -0700
Kees Cook <keesc...@chromium.org> wrote:

> On Mon, May 18, 2020 at 01:09:59AM +0930, Andrew Jeffery wrote:
> > As mentioned, a couple of attempts have been made to address the issue
> > by casting a pointer to optprobe_template_entry before providing it to
> > memcpy(), however gccs such as Ubuntu 20.04's arm-linux-gnueabi-gcc
> > 9.3.0 (Ubuntu 9.3.0-10ubuntu1) see through these efforts.
> 
> Ah, dang. :P
> 
> How about converting them all to unsized arrays, which would also allow
> the code to drop the "&" everywhere, I think. This is untested:
> 

This looks good to me since it uses same technique in sections.h.

Acked-by: Masami Hiramatsu <mhira...@kernel.org>

Thank you!

> 
> diff --git a/arch/arm/include/asm/kprobes.h b/arch/arm/include/asm/kprobes.h
> index 213607a1f45c..e26a278d301a 100644
> --- a/arch/arm/include/asm/kprobes.h
> +++ b/arch/arm/include/asm/kprobes.h
> @@ -44,20 +44,20 @@ int kprobe_exceptions_notify(struct notifier_block *self,
>                            unsigned long val, void *data);
>  
>  /* optinsn template addresses */
> -extern __visible kprobe_opcode_t optprobe_template_entry;
> -extern __visible kprobe_opcode_t optprobe_template_val;
> -extern __visible kprobe_opcode_t optprobe_template_call;
> -extern __visible kprobe_opcode_t optprobe_template_end;
> -extern __visible kprobe_opcode_t optprobe_template_sub_sp;
> -extern __visible kprobe_opcode_t optprobe_template_add_sp;
> -extern __visible kprobe_opcode_t optprobe_template_restore_begin;
> -extern __visible kprobe_opcode_t optprobe_template_restore_orig_insn;
> -extern __visible kprobe_opcode_t optprobe_template_restore_end;
> +extern __visible kprobe_opcode_t optprobe_template_entry[];
> +extern __visible kprobe_opcode_t optprobe_template_val[];
> +extern __visible kprobe_opcode_t optprobe_template_call[];
> +extern __visible kprobe_opcode_t optprobe_template_end[];
> +extern __visible kprobe_opcode_t optprobe_template_sub_sp[];
> +extern __visible kprobe_opcode_t optprobe_template_add_sp[];
> +extern __visible kprobe_opcode_t optprobe_template_restore_begin[];
> +extern __visible kprobe_opcode_t optprobe_template_restore_orig_insn[];
> +extern __visible kprobe_opcode_t optprobe_template_restore_end[];
>  
>  #define MAX_OPTIMIZED_LENGTH 4
>  #define MAX_OPTINSN_SIZE                             \
> -     ((unsigned long)&optprobe_template_end -        \
> -      (unsigned long)&optprobe_template_entry)
> +     ((unsigned long)optprobe_template_end - \
> +      (unsigned long)optprobe_template_entry)
>  #define RELATIVEJUMP_SIZE    4
>  
>  struct arch_optimized_insn {
> diff --git a/arch/arm/probes/kprobes/opt-arm.c 
> b/arch/arm/probes/kprobes/opt-arm.c
> index 7a449df0b359..c78180172120 100644
> --- a/arch/arm/probes/kprobes/opt-arm.c
> +++ b/arch/arm/probes/kprobes/opt-arm.c
> @@ -85,21 +85,21 @@ asm (
>                       "optprobe_template_end:\n");
>  
>  #define TMPL_VAL_IDX \
> -     ((unsigned long *)&optprobe_template_val - (unsigned long 
> *)&optprobe_template_entry)
> +     ((unsigned long *)optprobe_template_val - (unsigned long 
> *)optprobe_template_entry)
>  #define TMPL_CALL_IDX \
> -     ((unsigned long *)&optprobe_template_call - (unsigned long 
> *)&optprobe_template_entry)
> +     ((unsigned long *)optprobe_template_call - (unsigned long 
> *)optprobe_template_entry)
>  #define TMPL_END_IDX \
> -     ((unsigned long *)&optprobe_template_end - (unsigned long 
> *)&optprobe_template_entry)
> +     ((unsigned long *)optprobe_template_end - (unsigned long 
> *)optprobe_template_entry)
>  #define TMPL_ADD_SP \
> -     ((unsigned long *)&optprobe_template_add_sp - (unsigned long 
> *)&optprobe_template_entry)
> +     ((unsigned long *)optprobe_template_add_sp - (unsigned long 
> *)optprobe_template_entry)
>  #define TMPL_SUB_SP \
> -     ((unsigned long *)&optprobe_template_sub_sp - (unsigned long 
> *)&optprobe_template_entry)
> +     ((unsigned long *)optprobe_template_sub_sp - (unsigned long 
> *)optprobe_template_entry)
>  #define TMPL_RESTORE_BEGIN \
> -     ((unsigned long *)&optprobe_template_restore_begin - (unsigned long 
> *)&optprobe_template_entry)
> +     ((unsigned long *)optprobe_template_restore_begin - (unsigned long 
> *)optprobe_template_entry)
>  #define TMPL_RESTORE_ORIGN_INSN \
> -     ((unsigned long *)&optprobe_template_restore_orig_insn - (unsigned long 
> *)&optprobe_template_entry)
> +     ((unsigned long *)optprobe_template_restore_orig_insn - (unsigned long 
> *)optprobe_template_entry)
>  #define TMPL_RESTORE_END \
> -     ((unsigned long *)&optprobe_template_restore_end - (unsigned long 
> *)&optprobe_template_entry)
> +     ((unsigned long *)optprobe_template_restore_end - (unsigned long 
> *)optprobe_template_entry)
>  
>  /*
>   * ARM can always optimize an instruction when using ARM ISA, except
> @@ -234,7 +234,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe 
> *op, struct kprobe *or
>       }
>  
>       /* Copy arch-dep-instance from template. */
> -     memcpy(code, (unsigned long *)&optprobe_template_entry,
> +     memcpy(code, (unsigned long *)optprobe_template_entry,
>                       TMPL_END_IDX * sizeof(kprobe_opcode_t));
>  
>       /* Adjust buffer according to instruction. */
> 
> -- 
> Kees Cook


-- 
Masami Hiramatsu <mhira...@kernel.org>

Reply via email to