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: 7+ 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 16:02 ` Russell King - ARM Linux admin
2020-05-18 8:10 ` Andrew Jeffery
2020-05-17 21:48 ` Kees Cook
2020-05-18 0:49 ` Andrew Jeffery
2020-05-18 2:20 ` Masami Hiramatsu [this message]
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).