* [PATCH] x86/optprobe: Fix OPTPROBE vs UACCESS
@ 2020-03-05 9:21 Peter Zijlstra
2020-03-05 10:40 ` Masami Hiramatsu
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Peter Zijlstra @ 2020-03-05 9:21 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: linux-kernel, x86, Steven Rostedt
While looking at an objtool UACCESS warning, it suddenly occurred to me
that it is entirely possible to have an OPTPROBE right in the middle of
an UACCESS region.
In this case we must of course clear FLAGS.AC while running the KPROBE.
Luckily the trampoline already saves/restores [ER]FLAGS, so all we need
to do is inject a CLAC. Unfortunately we cannot use ALTERNATIVE() in the
trampoline text, so we have to frob that manually.
Fixes: ca0bbc70f147 ("sched/x86_64: Don't save flags on context switch")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/x86/include/asm/kprobes.h | 1 +
arch/x86/kernel/kprobes/opt.c | 14 ++++++++++++++
2 files changed, 15 insertions(+)
diff --git a/arch/x86/include/asm/kprobes.h b/arch/x86/include/asm/kprobes.h
index 95b1f053bd96..073eb7ad2f56 100644
--- a/arch/x86/include/asm/kprobes.h
+++ b/arch/x86/include/asm/kprobes.h
@@ -36,6 +36,7 @@ typedef u8 kprobe_opcode_t;
/* optinsn template addresses */
extern __visible kprobe_opcode_t optprobe_template_entry[];
+extern __visible kprobe_opcode_t optprobe_template_clac[];
extern __visible kprobe_opcode_t optprobe_template_val[];
extern __visible kprobe_opcode_t optprobe_template_call[];
extern __visible kprobe_opcode_t optprobe_template_end[];
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 3f45b5c43a71..7a3416c9d0dc 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -92,6 +92,9 @@ asm (
/* We don't bother saving the ss register */
" pushq %rsp\n"
" pushfq\n"
+ ".global optprobe_template_clac\n"
+ "optprobe_template_clac:\n"
+ ASM_NOP3
SAVE_REGS_STRING
" movq %rsp, %rsi\n"
".global optprobe_template_val\n"
@@ -111,6 +114,9 @@ asm (
#else /* CONFIG_X86_32 */
" pushl %esp\n"
" pushfl\n"
+ ".global optprobe_template_clac\n"
+ "optprobe_template_clac:\n"
+ ASM_NOP3
SAVE_REGS_STRING
" movl %esp, %edx\n"
".global optprobe_template_val\n"
@@ -134,6 +140,8 @@ asm (
void optprobe_template_func(void);
STACK_FRAME_NON_STANDARD(optprobe_template_func);
+#define TMPL_CLAC_IDX \
+ ((long)optprobe_template_clac - (long)optprobe_template_entry)
#define TMPL_MOVE_IDX \
((long)optprobe_template_val - (long)optprobe_template_entry)
#define TMPL_CALL_IDX \
@@ -389,6 +397,12 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op,
op->optinsn.size = ret;
len = TMPL_END_IDX + op->optinsn.size;
+ if (static_cpu_has(X86_FEATURE_SMAP)) {
+ buf[TMPL_CLAC_IDX+0] = 0x0f;
+ buf[TMPL_CLAC_IDX+1] = 0x01;
+ buf[TMPL_CLAC_IDX+2] = 0xca;
+ }
+
/* Set probe information */
synthesize_set_arg1(buf + TMPL_MOVE_IDX, (unsigned long)op);
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] x86/optprobe: Fix OPTPROBE vs UACCESS
2020-03-05 9:21 [PATCH] x86/optprobe: Fix OPTPROBE vs UACCESS Peter Zijlstra
@ 2020-03-05 10:40 ` Masami Hiramatsu
2020-03-05 11:59 ` Peter Zijlstra
2020-03-06 0:56 ` kbuild test robot
2020-03-20 12:58 ` [tip: perf/core] " tip-bot2 for Peter Zijlstra
2 siblings, 1 reply; 6+ messages in thread
From: Masami Hiramatsu @ 2020-03-05 10:40 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: linux-kernel, x86, Steven Rostedt
Hi Peter,
On Thu, 5 Mar 2020 10:21:30 +0100
Peter Zijlstra <peterz@infradead.org> wrote:
>
> While looking at an objtool UACCESS warning, it suddenly occurred to me
> that it is entirely possible to have an OPTPROBE right in the middle of
> an UACCESS region.
>
> In this case we must of course clear FLAGS.AC while running the KPROBE.
> Luckily the trampoline already saves/restores [ER]FLAGS, so all we need
> to do is inject a CLAC. Unfortunately we cannot use ALTERNATIVE() in the
> trampoline text, so we have to frob that manually.
Good catch! so this prevents optprobe handler to access user space
avoiding SMAP feature.
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Thank you,
>
> Fixes: ca0bbc70f147 ("sched/x86_64: Don't save flags on context switch")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> arch/x86/include/asm/kprobes.h | 1 +
> arch/x86/kernel/kprobes/opt.c | 14 ++++++++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/arch/x86/include/asm/kprobes.h b/arch/x86/include/asm/kprobes.h
> index 95b1f053bd96..073eb7ad2f56 100644
> --- a/arch/x86/include/asm/kprobes.h
> +++ b/arch/x86/include/asm/kprobes.h
> @@ -36,6 +36,7 @@ typedef u8 kprobe_opcode_t;
>
> /* optinsn template addresses */
> extern __visible kprobe_opcode_t optprobe_template_entry[];
> +extern __visible kprobe_opcode_t optprobe_template_clac[];
> extern __visible kprobe_opcode_t optprobe_template_val[];
> extern __visible kprobe_opcode_t optprobe_template_call[];
> extern __visible kprobe_opcode_t optprobe_template_end[];
> diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
> index 3f45b5c43a71..7a3416c9d0dc 100644
> --- a/arch/x86/kernel/kprobes/opt.c
> +++ b/arch/x86/kernel/kprobes/opt.c
> @@ -92,6 +92,9 @@ asm (
> /* We don't bother saving the ss register */
> " pushq %rsp\n"
> " pushfq\n"
> + ".global optprobe_template_clac\n"
> + "optprobe_template_clac:\n"
> + ASM_NOP3
> SAVE_REGS_STRING
> " movq %rsp, %rsi\n"
> ".global optprobe_template_val\n"
> @@ -111,6 +114,9 @@ asm (
> #else /* CONFIG_X86_32 */
> " pushl %esp\n"
> " pushfl\n"
> + ".global optprobe_template_clac\n"
> + "optprobe_template_clac:\n"
> + ASM_NOP3
> SAVE_REGS_STRING
> " movl %esp, %edx\n"
> ".global optprobe_template_val\n"
> @@ -134,6 +140,8 @@ asm (
> void optprobe_template_func(void);
> STACK_FRAME_NON_STANDARD(optprobe_template_func);
>
> +#define TMPL_CLAC_IDX \
> + ((long)optprobe_template_clac - (long)optprobe_template_entry)
> #define TMPL_MOVE_IDX \
> ((long)optprobe_template_val - (long)optprobe_template_entry)
> #define TMPL_CALL_IDX \
> @@ -389,6 +397,12 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op,
> op->optinsn.size = ret;
> len = TMPL_END_IDX + op->optinsn.size;
>
> + if (static_cpu_has(X86_FEATURE_SMAP)) {
> + buf[TMPL_CLAC_IDX+0] = 0x0f;
> + buf[TMPL_CLAC_IDX+1] = 0x01;
> + buf[TMPL_CLAC_IDX+2] = 0xca;
> + }
> +
> /* Set probe information */
> synthesize_set_arg1(buf + TMPL_MOVE_IDX, (unsigned long)op);
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] x86/optprobe: Fix OPTPROBE vs UACCESS
2020-03-05 10:40 ` Masami Hiramatsu
@ 2020-03-05 11:59 ` Peter Zijlstra
0 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2020-03-05 11:59 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: linux-kernel, x86, Steven Rostedt
On Thu, Mar 05, 2020 at 07:40:06PM +0900, Masami Hiramatsu wrote:
> Hi Peter,
>
> On Thu, 5 Mar 2020 10:21:30 +0100
> Peter Zijlstra <peterz@infradead.org> wrote:
>
> >
> > While looking at an objtool UACCESS warning, it suddenly occurred to me
> > that it is entirely possible to have an OPTPROBE right in the middle of
> > an UACCESS region.
> >
> > In this case we must of course clear FLAGS.AC while running the KPROBE.
> > Luckily the trampoline already saves/restores [ER]FLAGS, so all we need
> > to do is inject a CLAC. Unfortunately we cannot use ALTERNATIVE() in the
> > trampoline text, so we have to frob that manually.
>
> Good catch! so this prevents optprobe handler to access user space
> avoiding SMAP feature.
Yes that, but also worse, since the patch referenced by Fixes, x86_64 no
longer saves/restores FLAGS on context switch, and if the OPTPROBE were
to (accidentally) call into schedule() (say through preempt_enable()),
the next task could also run without SMAP for a while.
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/optprobe: Fix OPTPROBE vs UACCESS
2020-03-05 9:21 [PATCH] x86/optprobe: Fix OPTPROBE vs UACCESS Peter Zijlstra
2020-03-05 10:40 ` Masami Hiramatsu
@ 2020-03-06 0:56 ` kbuild test robot
2020-03-09 16:42 ` Peter Zijlstra
2020-03-20 12:58 ` [tip: perf/core] " tip-bot2 for Peter Zijlstra
2 siblings, 1 reply; 6+ messages in thread
From: kbuild test robot @ 2020-03-06 0:56 UTC (permalink / raw)
To: Peter Zijlstra
Cc: kbuild-all, Masami Hiramatsu, linux-kernel, x86, Steven Rostedt
[-- Attachment #1: Type: text/plain, Size: 1222 bytes --]
Hi Peter,
I love your patch! Perhaps something to improve:
[auto build test WARNING on tip/x86/core]
[also build test WARNING on tip/auto-latest linux/master linus/master v5.6-rc4 next-20200305]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Peter-Zijlstra/x86-optprobe-Fix-OPTPROBE-vs-UACCESS/20200305-234916
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 248ed51048c40d36728e70914e38bffd7821da57
config: x86_64-randconfig-a003-20200305 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-5) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> arch/x86/kernel/kprobes/opt.o: warning: objtool: arch_prepare_optimized_kprobe()+0x156: unreachable instruction
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30184 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] x86/optprobe: Fix OPTPROBE vs UACCESS
2020-03-06 0:56 ` kbuild test robot
@ 2020-03-09 16:42 ` Peter Zijlstra
0 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2020-03-09 16:42 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Masami Hiramatsu, linux-kernel, x86, Steven Rostedt
On Fri, Mar 06, 2020 at 08:56:15AM +0800, kbuild test robot wrote:
> All warnings (new ones prefixed by >>):
>
> >> arch/x86/kernel/kprobes/opt.o: warning: objtool: arch_prepare_optimized_kprobe()+0x156: unreachable instruction
Duh.. I changed it so.
---
Subject: x86/optprobe: Fix OPTPROBE vs UACCESS
From: Peter Zijlstra <peterz@infradead.org>
Date: Thu, 5 Mar 2020 10:21:30 +0100
While looking at an objtool UACCESS warning, it suddenly occurred to me
that it is entirely possible to have an OPTPROBE right in the middle of
an UACCESS region.
In this case we must of course clear FLAGS.AC while running the KPROBE.
Luckily the trampoline already saves/restores [ER]FLAGS, so all we need
to do is inject a CLAC. Unfortunately we cannot use ALTERNATIVE() in the
trampoline text, so we have to frob that manually.
Fixes: ca0bbc70f147 ("sched/x86_64: Don't save flags on context switch")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Link: https://lkml.kernel.org/r/20200305092130.GU2596@hirez.programming.kicks-ass.net
---
arch/x86/include/asm/kprobes.h | 1 +
arch/x86/kernel/kprobes/opt.c | 24 ++++++++++++++++++++++++
2 files changed, 25 insertions(+)
--- a/arch/x86/include/asm/kprobes.h
+++ b/arch/x86/include/asm/kprobes.h
@@ -36,6 +36,7 @@ typedef u8 kprobe_opcode_t;
/* optinsn template addresses */
extern __visible kprobe_opcode_t optprobe_template_entry[];
+extern __visible kprobe_opcode_t optprobe_template_clac[];
extern __visible kprobe_opcode_t optprobe_template_val[];
extern __visible kprobe_opcode_t optprobe_template_call[];
extern __visible kprobe_opcode_t optprobe_template_end[];
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -72,6 +72,20 @@ unsigned long __recover_optprobed_insn(k
return (unsigned long)buf;
}
+static void synthesize_clac(kprobe_opcode_t *addr)
+{
+ /*
+ * Can't be static_cpu_has() due to how objtool treats this feature bit.
+ * This isn't a fast path anyway.
+ */
+ if (!boot_cpu_has(X86_FEATURE_SMAP))
+ return;
+
+ addr[0] = 0x0f;
+ addr[1] = 0x01;
+ addr[2] = 0xca;
+}
+
/* Insert a move instruction which sets a pointer to eax/rdi (1st arg). */
static void synthesize_set_arg1(kprobe_opcode_t *addr, unsigned long val)
{
@@ -93,6 +107,9 @@ asm (
/* We don't bother saving the ss register */
" pushq %rsp\n"
" pushfq\n"
+ ".global optprobe_template_clac\n"
+ "optprobe_template_clac:\n"
+ ASM_NOP3
SAVE_REGS_STRING
" movq %rsp, %rsi\n"
".global optprobe_template_val\n"
@@ -112,6 +129,9 @@ asm (
#else /* CONFIG_X86_32 */
" pushl %esp\n"
" pushfl\n"
+ ".global optprobe_template_clac\n"
+ "optprobe_template_clac:\n"
+ ASM_NOP3
SAVE_REGS_STRING
" movl %esp, %edx\n"
".global optprobe_template_val\n"
@@ -135,6 +155,8 @@ asm (
void optprobe_template_func(void);
STACK_FRAME_NON_STANDARD(optprobe_template_func);
+#define TMPL_CLAC_IDX \
+ ((long)optprobe_template_clac - (long)optprobe_template_entry)
#define TMPL_MOVE_IDX \
((long)optprobe_template_val - (long)optprobe_template_entry)
#define TMPL_CALL_IDX \
@@ -391,6 +413,8 @@ int arch_prepare_optimized_kprobe(struct
op->optinsn.size = ret;
len = TMPL_END_IDX + op->optinsn.size;
+ synthesize_clac(buf + TMPL_CLAC_IDX);
+
/* Set probe information */
synthesize_set_arg1(buf + TMPL_MOVE_IDX, (unsigned long)op);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip: perf/core] x86/optprobe: Fix OPTPROBE vs UACCESS
2020-03-05 9:21 [PATCH] x86/optprobe: Fix OPTPROBE vs UACCESS Peter Zijlstra
2020-03-05 10:40 ` Masami Hiramatsu
2020-03-06 0:56 ` kbuild test robot
@ 2020-03-20 12:58 ` tip-bot2 for Peter Zijlstra
2 siblings, 0 replies; 6+ messages in thread
From: tip-bot2 for Peter Zijlstra @ 2020-03-20 12:58 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Peter Zijlstra (Intel), Masami Hiramatsu, x86, LKML
The following commit has been merged into the perf/core branch of tip:
Commit-ID: d8a738689794c42c3844284b99ddf165d10a724e
Gitweb: https://git.kernel.org/tip/d8a738689794c42c3844284b99ddf165d10a724e
Author: Peter Zijlstra <peterz@infradead.org>
AuthorDate: Thu, 05 Mar 2020 10:21:30 +01:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 20 Mar 2020 13:06:22 +01:00
x86/optprobe: Fix OPTPROBE vs UACCESS
While looking at an objtool UACCESS warning, it suddenly occurred to me
that it is entirely possible to have an OPTPROBE right in the middle of
an UACCESS region.
In this case we must of course clear FLAGS.AC while running the KPROBE.
Luckily the trampoline already saves/restores [ER]FLAGS, so all we need
to do is inject a CLAC. Unfortunately we cannot use ALTERNATIVE() in the
trampoline text, so we have to frob that manually.
Fixes: ca0bbc70f147 ("sched/x86_64: Don't save flags on context switch")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Link: https://lkml.kernel.org/r/20200305092130.GU2596@hirez.programming.kicks-ass.net
---
arch/x86/include/asm/kprobes.h | 1 +
arch/x86/kernel/kprobes/opt.c | 25 +++++++++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/arch/x86/include/asm/kprobes.h b/arch/x86/include/asm/kprobes.h
index 95b1f05..073eb7a 100644
--- a/arch/x86/include/asm/kprobes.h
+++ b/arch/x86/include/asm/kprobes.h
@@ -36,6 +36,7 @@ typedef u8 kprobe_opcode_t;
/* optinsn template addresses */
extern __visible kprobe_opcode_t optprobe_template_entry[];
+extern __visible kprobe_opcode_t optprobe_template_clac[];
extern __visible kprobe_opcode_t optprobe_template_val[];
extern __visible kprobe_opcode_t optprobe_template_call[];
extern __visible kprobe_opcode_t optprobe_template_end[];
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 3f45b5c..ea13f68 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -71,6 +71,21 @@ found:
return (unsigned long)buf;
}
+static void synthesize_clac(kprobe_opcode_t *addr)
+{
+ /*
+ * Can't be static_cpu_has() due to how objtool treats this feature bit.
+ * This isn't a fast path anyway.
+ */
+ if (!boot_cpu_has(X86_FEATURE_SMAP))
+ return;
+
+ /* Replace the NOP3 with CLAC */
+ addr[0] = 0x0f;
+ addr[1] = 0x01;
+ addr[2] = 0xca;
+}
+
/* Insert a move instruction which sets a pointer to eax/rdi (1st arg). */
static void synthesize_set_arg1(kprobe_opcode_t *addr, unsigned long val)
{
@@ -92,6 +107,9 @@ asm (
/* We don't bother saving the ss register */
" pushq %rsp\n"
" pushfq\n"
+ ".global optprobe_template_clac\n"
+ "optprobe_template_clac:\n"
+ ASM_NOP3
SAVE_REGS_STRING
" movq %rsp, %rsi\n"
".global optprobe_template_val\n"
@@ -111,6 +129,9 @@ asm (
#else /* CONFIG_X86_32 */
" pushl %esp\n"
" pushfl\n"
+ ".global optprobe_template_clac\n"
+ "optprobe_template_clac:\n"
+ ASM_NOP3
SAVE_REGS_STRING
" movl %esp, %edx\n"
".global optprobe_template_val\n"
@@ -134,6 +155,8 @@ asm (
void optprobe_template_func(void);
STACK_FRAME_NON_STANDARD(optprobe_template_func);
+#define TMPL_CLAC_IDX \
+ ((long)optprobe_template_clac - (long)optprobe_template_entry)
#define TMPL_MOVE_IDX \
((long)optprobe_template_val - (long)optprobe_template_entry)
#define TMPL_CALL_IDX \
@@ -389,6 +412,8 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op,
op->optinsn.size = ret;
len = TMPL_END_IDX + op->optinsn.size;
+ synthesize_clac(buf + TMPL_CLAC_IDX);
+
/* Set probe information */
synthesize_set_arg1(buf + TMPL_MOVE_IDX, (unsigned long)op);
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-20 12:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-05 9:21 [PATCH] x86/optprobe: Fix OPTPROBE vs UACCESS Peter Zijlstra
2020-03-05 10:40 ` Masami Hiramatsu
2020-03-05 11:59 ` Peter Zijlstra
2020-03-06 0:56 ` kbuild test robot
2020-03-09 16:42 ` Peter Zijlstra
2020-03-20 12:58 ` [tip: perf/core] " tip-bot2 for Peter Zijlstra
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).