From: Masami Hiramatsu <mhiramat@kernel.org>
To: Kees Cook <keescook@chromium.org>
Cc: Andrew Jeffery <andrew@aj.id.au>,
linux-arm-kernel@lists.infradead.org, linux@armlinux.org.uk,
mhiramat@kernel.org, labbott@redhat.com,
mathieu.desnoyers@efficios.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ARM: kprobes: Avoid fortify_panic() when copying optprobe template
Date: Mon, 18 May 2020 11:20:59 +0900 [thread overview]
Message-ID: <20200518112059.c19899ffb17a4843bf4f74ab@kernel.org> (raw)
In-Reply-To: <202005171447.00CFE0C@keescook>
On Sun, 17 May 2020 14:48:52 -0700
Kees Cook <keescook@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 <mhiramat@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 <mhiramat@kernel.org>
WARNING: multiple messages have this Message-ID (diff)
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Kees Cook <keescook@chromium.org>
Cc: Andrew Jeffery <andrew@aj.id.au>,
linux@armlinux.org.uk, linux-kernel@vger.kernel.org,
mathieu.desnoyers@efficios.com, mhiramat@kernel.org,
labbott@redhat.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] ARM: kprobes: Avoid fortify_panic() when copying optprobe template
Date: Mon, 18 May 2020 11:20:59 +0900 [thread overview]
Message-ID: <20200518112059.c19899ffb17a4843bf4f74ab@kernel.org> (raw)
In-Reply-To: <202005171447.00CFE0C@keescook>
On Sun, 17 May 2020 14:48:52 -0700
Kees Cook <keescook@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 <mhiramat@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 <mhiramat@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-05-18 2:21 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-17 15:39 [PATCH] ARM: kprobes: Avoid fortify_panic() when copying optprobe template Andrew Jeffery
2020-05-17 15:39 ` Andrew Jeffery
2020-05-17 16:02 ` Russell King - ARM Linux admin
2020-05-17 16:02 ` Russell King - ARM Linux admin
2020-05-18 8:10 ` Andrew Jeffery
2020-05-18 8:10 ` Andrew Jeffery
2020-05-17 21:48 ` Kees Cook
2020-05-17 21:48 ` Kees Cook
2020-05-18 0:49 ` Andrew Jeffery
2020-05-18 0:49 ` Andrew Jeffery
2020-05-18 2:20 ` Masami Hiramatsu [this message]
2020-05-18 2:20 ` Masami Hiramatsu
2020-05-19 0:22 ` Andrew Jeffery
2020-05-19 0:22 ` Andrew Jeffery
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200518112059.c19899ffb17a4843bf4f74ab@kernel.org \
--to=mhiramat@kernel.org \
--cc=andrew@aj.id.au \
--cc=keescook@chromium.org \
--cc=labbott@redhat.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=mathieu.desnoyers@efficios.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.