Re: [BUGFIX PATCH bpf-next] error-injection: Fix to prohibit jump optimization
On Mon, 12 Mar 2018 11:44:21 +0100
Daniel Borkmann <dan...@iogearbox.net> wrote:
> Hi Masami,
>
> On 03/12/2018 11:27 AM, Masami Hiramatsu wrote:
> > On Mon, 12 Mar 2018 19:00:49 +0900
> > Masami Hiramatsu <mhira...@kernel.org> wrote:
> >
> >> Since the kprobe which was optimized by jump can not change
> >> the execution path, the kprobe for error-injection must not
> >> be optimized. To prohibit it, set a dummy post-handler as
> >> officially stated in Documentation/kprobes.txt.
> >
> > Note that trace-probe based BPF is not affected, because it
> > ensures the trace-probe is based on ftrace, which is not
> > jump optimized.
>
> Thanks for the fix! I presume this should go via bpf instead of bpf-next
> tree since 4b1a29a7f542 ("error-injection: Support fault injection framework")
> is in Linus' tree as well. Unless there are objection I would rather route
> it that way so it would be for 4.16.
Ah, right! It should go into 4.16. It should be applicable cleanly either tree
since there is only the above commit on kernel/fail_function.c :)
Thanks,
>
> Thanks,
> Daniel
>
> > Thanks,
> >
> >>
> >> Fixes: 4b1a29a7f542 ("error-injection: Support fault injection framework")
> >> Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
> >> ---
> >> kernel/fail_function.c | 10 ++
> >> 1 file changed, 10 insertions(+)
> >>
> >> diff --git a/kernel/fail_function.c b/kernel/fail_function.c
> >> index 21b0122cb39c..1d5632d8bbcc 100644
> >> --- a/kernel/fail_function.c
> >> +++ b/kernel/fail_function.c
> >> @@ -14,6 +14,15 @@
> >>
> >> static int fei_kprobe_handler(struct kprobe *kp, struct pt_regs *regs);
> >>
> >> +static void fei_post_handler(struct kprobe *kp, struct pt_regs *regs,
> >> + unsigned long flags)
> >> +{
> >> + /*
> >> + * A dummy post handler is required to prohibit optimizing, because
> >> + * jump optimization does not support execution path overriding.
> >> + */
> >> +}
> >> +
> >> struct fei_attr {
> >>struct list_head list;
> >>struct kprobe kp;
> >> @@ -56,6 +65,7 @@ static struct fei_attr *fei_attr_new(const char *sym,
> >> unsigned long addr)
> >>return NULL;
> >>}
> >>attr->kp.pre_handler = fei_kprobe_handler;
> >> + attr->kp.post_handler = fei_post_handler;
> >>attr->retval = adjust_error_retval(addr, 0);
> >>INIT_LIST_HEAD(>list);
> >>}
> >>
> >
> >
>
--
Masami Hiramatsu <mhira...@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUGFIX PATCH bpf-next] error-injection: Fix to prohibit jump optimization
On Mon, 12 Mar 2018 19:00:49 +0900
Masami Hiramatsu <mhira...@kernel.org> wrote:
> Since the kprobe which was optimized by jump can not change
> the execution path, the kprobe for error-injection must not
> be optimized. To prohibit it, set a dummy post-handler as
> officially stated in Documentation/kprobes.txt.
Note that trace-probe based BPF is not affected, because it
ensures the trace-probe is based on ftrace, which is not
jump optimized.
Thanks,
>
> Fixes: 4b1a29a7f542 ("error-injection: Support fault injection framework")
> Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
> ---
> kernel/fail_function.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/kernel/fail_function.c b/kernel/fail_function.c
> index 21b0122cb39c..1d5632d8bbcc 100644
> --- a/kernel/fail_function.c
> +++ b/kernel/fail_function.c
> @@ -14,6 +14,15 @@
>
> static int fei_kprobe_handler(struct kprobe *kp, struct pt_regs *regs);
>
> +static void fei_post_handler(struct kprobe *kp, struct pt_regs *regs,
> + unsigned long flags)
> +{
> + /*
> + * A dummy post handler is required to prohibit optimizing, because
> + * jump optimization does not support execution path overriding.
> + */
> +}
> +
> struct fei_attr {
> struct list_head list;
> struct kprobe kp;
> @@ -56,6 +65,7 @@ static struct fei_attr *fei_attr_new(const char *sym,
> unsigned long addr)
> return NULL;
> }
> attr->kp.pre_handler = fei_kprobe_handler;
> + attr->kp.post_handler = fei_post_handler;
> attr->retval = adjust_error_retval(addr, 0);
> INIT_LIST_HEAD(>list);
> }
>
--
Masami Hiramatsu <mhira...@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUGFIX PATCH bpf-next] error-injection: Fix to prohibit jump optimization
Since the kprobe which was optimized by jump can not change
the execution path, the kprobe for error-injection must not
be optimized. To prohibit it, set a dummy post-handler as
officially stated in Documentation/kprobes.txt.
Fixes: 4b1a29a7f542 ("error-injection: Support fault injection framework")
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
---
kernel/fail_function.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/kernel/fail_function.c b/kernel/fail_function.c
index 21b0122cb39c..1d5632d8bbcc 100644
--- a/kernel/fail_function.c
+++ b/kernel/fail_function.c
@@ -14,6 +14,15 @@
static int fei_kprobe_handler(struct kprobe *kp, struct pt_regs *regs);
+static void fei_post_handler(struct kprobe *kp, struct pt_regs *regs,
+unsigned long flags)
+{
+ /*
+* A dummy post handler is required to prohibit optimizing, because
+* jump optimization does not support execution path overriding.
+*/
+}
+
struct fei_attr {
struct list_head list;
struct kprobe kp;
@@ -56,6 +65,7 @@ static struct fei_attr *fei_attr_new(const char *sym,
unsigned long addr)
return NULL;
}
attr->kp.pre_handler = fei_kprobe_handler;
+ attr->kp.post_handler = fei_post_handler;
attr->retval = adjust_error_retval(addr, 0);
INIT_LIST_HEAD(>list);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH bpf-next v5 5/5] error-injection: Support fault injection framework
On Sat, 13 Jan 2018 22:28:29 +0900
Akinobu Mita <akinobu.m...@gmail.com> wrote:
> 2018-01-13 2:56 GMT+09:00 Masami Hiramatsu <mhira...@kernel.org>:
> > Support in-kernel fault-injection framework via debugfs.
> > This allows you to inject a conditional error to specified
> > function using debugfs interfaces.
> >
> > Here is the result of test script described in
> > Documentation/fault-injection/fault-injection.txt
> >
> > ===
> > # ./test_fail_function.sh
> > 1+0 records in
> > 1+0 records out
> > 1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.0227404 s, 46.1 MB/s
> > btrfs-progs v4.4
> > See http://btrfs.wiki.kernel.org for more information.
> >
> > Label: (null)
> > UUID: bfa96010-12e9-4360-aed0-42eec7af5798
> > Node size: 16384
> > Sector size:4096
> > Filesystem size:1001.00MiB
> > Block group profiles:
> > Data: single8.00MiB
> > Metadata: DUP 58.00MiB
> > System: DUP 12.00MiB
> > SSD detected: no
> > Incompat features: extref, skinny-metadata
> > Number of devices: 1
> > Devices:
> > ID SIZE PATH
> > 1 1001.00MiB /dev/loop2
> >
> > mount: mount /dev/loop2 on /opt/tmpmnt failed: Cannot allocate memory
> > SUCCESS!
> > ===
> >
> >
> > Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
> > Reviewed-by: Josef Bacik <jba...@fb.com>
> > ---
> > Changes in v3:
> >- Check and adjust error value for each target function
> >- Clear kporbe flag for reuse
> >- Add more documents and example
> > Changes in v5:
> >- Support multi-function error injection
> > ---
> > Documentation/fault-injection/fault-injection.txt | 68
> > kernel/Makefile |1
> > kernel/fail_function.c| 349
> > +
> > lib/Kconfig.debug | 10 +
> > 4 files changed, 428 insertions(+)
> > create mode 100644 kernel/fail_function.c
> >
> > diff --git a/Documentation/fault-injection/fault-injection.txt
> > b/Documentation/fault-injection/fault-injection.txt
> > index 918972babcd8..f4a32463ca48 100644
> > --- a/Documentation/fault-injection/fault-injection.txt
> > +++ b/Documentation/fault-injection/fault-injection.txt
> > @@ -30,6 +30,12 @@ o fail_mmc_request
> >injects MMC data errors on devices permitted by setting
> >debugfs entries under /sys/kernel/debug/mmc0/fail_mmc_request
> >
> > +o fail_function
> > +
> > + injects error return on specific functions, which are marked by
> > + ALLOW_ERROR_INJECTION() macro, by setting debugfs entries
> > + under /sys/kernel/debug/fail_function. No boot option supported.
> > +
> > Configure fault-injection capabilities behavior
> > ---
> >
> > @@ -123,6 +129,29 @@ configuration of fault-injection capabilities.
> > default is 'N', setting it to 'Y' will disable failure injections
> > when dealing with private (address space) futexes.
> >
> > +- /sys/kernel/debug/fail_function/inject:
> > +
> > + Format: { 'function-name' | '!function-name' | '' }
> > + specifies the target function of error injection by name.
> > + If the function name leads '!' prefix, given function is
> > + removed from injection list. If nothing specified ('')
> > + injection list is cleared.
> > +
> > +- /sys/kernel/debug/fail_function/injectable:
> > +
> > + (read only) shows error injectable functions and what type of
> > + error values can be specified. The error type will be one of
> > + below;
> > + - NULL: retval must be 0.
> > + - ERRNO: retval must be -1 to -MAX_ERRNO (-4096).
> > + - ERR_NULL: retval must be 0 or -1 to -MAX_ERRNO (-4096).
> > +
> > +- /sys/kernel/debug/fail_function//retval:
> > +
> > + specifies the "error" return value to inject to the given
> > + function for given function. This will be created when
> > + user specifies new injection entry.
> > +
> > o Boot option
> >
> > In order to inject faults while debugfs is not available (early boot time),
> > @@ -268,6 +297,45 @@ trap "echo 0 >
> > /sys/kernel/debug/$FAILTYPE/probability"
[PATCH bpf-next v5 3/5] error-injection: Separate error-injection from kprobe
Since error-injection framework is not limited to be used
by kprobes, nor bpf. Other kernel subsystems can use it
freely for checking safeness of error-injection, e.g.
livepatch, ftrace etc.
So this separate error-injection framework from kprobes.
Some differences has been made:
- "kprobe" word is removed from any APIs/structures.
- BPF_ALLOW_ERROR_INJECTION() is renamed to
ALLOW_ERROR_INJECTION() since it is not limited for BPF too.
- CONFIG_FUNCTION_ERROR_INJECTION is the config item of this
feature. It is automatically enabled if the arch supports
error injection feature for kprobe or ftrace etc.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
Reviewed-by: Josef Bacik <jba...@fb.com>
---
Changes in v3:
- Fix a build error for asmlinkage on i386 by including compiler.h
- Fix "CONFIG_FUNCTION_ERROR_INJECT" typo.
- Separate CONFIG_MODULES dependent code
- Add CONFIG_KPROBES dependency for arch_deref_entry_point()
- Call error-injection init function in late_initcall stage.
- Fix read-side mutex lock
- Some cosmetic cleanups
Changes in v4:
- Include error-injection.h for each ALLOW_ERROR_INJECTION
macro user, instead of bpf.h
Changes in v5:
- Fix within_error_injection_list to return correct result.
---
arch/Kconfig |2
arch/x86/Kconfig |2
arch/x86/include/asm/error-injection.h | 13 ++
arch/x86/kernel/kprobes/core.c | 14 --
arch/x86/lib/Makefile |1
arch/x86/lib/error-inject.c| 19 +++
fs/btrfs/disk-io.c |4 -
fs/btrfs/free-space-cache.c|4 -
include/asm-generic/error-injection.h | 20 +++
include/asm-generic/vmlinux.lds.h | 14 +-
include/linux/bpf.h| 11 --
include/linux/error-injection.h| 21 +++
include/linux/kprobes.h|1
include/linux/module.h |6 -
kernel/kprobes.c | 163
kernel/module.c|8 +
kernel/trace/Kconfig |2
kernel/trace/bpf_trace.c |4 -
kernel/trace/trace_kprobe.c|3
lib/Kconfig.debug |4 +
lib/Makefile |1
lib/error-inject.c | 213
22 files changed, 317 insertions(+), 213 deletions(-)
create mode 100644 arch/x86/include/asm/error-injection.h
create mode 100644 arch/x86/lib/error-inject.c
create mode 100644 include/asm-generic/error-injection.h
create mode 100644 include/linux/error-injection.h
create mode 100644 lib/error-inject.c
diff --git a/arch/Kconfig b/arch/Kconfig
index d3f4aaf9cb7a..97376accfb14 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -196,7 +196,7 @@ config HAVE_OPTPROBES
config HAVE_KPROBES_ON_FTRACE
bool
-config HAVE_KPROBE_OVERRIDE
+config HAVE_FUNCTION_ERROR_INJECTION
bool
config HAVE_NMI
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 45dc6233f2b9..366b19cb79b7 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -154,7 +154,7 @@ config X86
select HAVE_KERNEL_XZ
select HAVE_KPROBES
select HAVE_KPROBES_ON_FTRACE
- select HAVE_KPROBE_OVERRIDE
+ select HAVE_FUNCTION_ERROR_INJECTION
select HAVE_KRETPROBES
select HAVE_KVM
select HAVE_LIVEPATCH if X86_64
diff --git a/arch/x86/include/asm/error-injection.h
b/arch/x86/include/asm/error-injection.h
new file mode 100644
index ..47b7a1296245
--- /dev/null
+++ b/arch/x86/include/asm/error-injection.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ERROR_INJECTION_H
+#define _ASM_ERROR_INJECTION_H
+
+#include
+#include
+#include
+#include
+
+asmlinkage void just_return_func(void);
+void override_function_with_return(struct pt_regs *regs);
+
+#endif /* _ASM_ERROR_INJECTION_H */
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index b02a377d5905..bd36f3c33cd0 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -1183,17 +1183,3 @@ int arch_trampoline_kprobe(struct kprobe *p)
{
return 0;
}
-
-asmlinkage void override_func(void);
-asm(
- ".type override_func, @function\n"
- "override_func:\n"
- " ret\n"
- ".size override_func, .-override_func\n"
-);
-
-void arch_kprobe_override_function(struct pt_regs *regs)
-{
- regs->ip = (unsigned long)_func;
-}
-NOKPROBE_SYMBOL(arch_kprobe_override_function);
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index 7b181b61170e..171377b83be1 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -26,6 +26,7 @@ lib-y += memcpy_$(BITS).o
lib-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o
lib-$(CONFIG_INSTRUCTION_DECODER)
[PATCH bpf-next v5 5/5] error-injection: Support fault injection framework
Support in-kernel fault-injection framework via debugfs.
This allows you to inject a conditional error to specified
function using debugfs interfaces.
Here is the result of test script described in
Documentation/fault-injection/fault-injection.txt
===
# ./test_fail_function.sh
1+0 records in
1+0 records out
1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.0227404 s, 46.1 MB/s
btrfs-progs v4.4
See http://btrfs.wiki.kernel.org for more information.
Label: (null)
UUID: bfa96010-12e9-4360-aed0-42eec7af5798
Node size: 16384
Sector size:4096
Filesystem size:1001.00MiB
Block group profiles:
Data: single8.00MiB
Metadata: DUP 58.00MiB
System: DUP 12.00MiB
SSD detected: no
Incompat features: extref, skinny-metadata
Number of devices: 1
Devices:
IDSIZE PATH
1 1001.00MiB /dev/loop2
mount: mount /dev/loop2 on /opt/tmpmnt failed: Cannot allocate memory
SUCCESS!
===
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
Reviewed-by: Josef Bacik <jba...@fb.com>
---
Changes in v3:
- Check and adjust error value for each target function
- Clear kporbe flag for reuse
- Add more documents and example
Changes in v5:
- Support multi-function error injection
---
Documentation/fault-injection/fault-injection.txt | 68
kernel/Makefile |1
kernel/fail_function.c| 349 +
lib/Kconfig.debug | 10 +
4 files changed, 428 insertions(+)
create mode 100644 kernel/fail_function.c
diff --git a/Documentation/fault-injection/fault-injection.txt
b/Documentation/fault-injection/fault-injection.txt
index 918972babcd8..f4a32463ca48 100644
--- a/Documentation/fault-injection/fault-injection.txt
+++ b/Documentation/fault-injection/fault-injection.txt
@@ -30,6 +30,12 @@ o fail_mmc_request
injects MMC data errors on devices permitted by setting
debugfs entries under /sys/kernel/debug/mmc0/fail_mmc_request
+o fail_function
+
+ injects error return on specific functions, which are marked by
+ ALLOW_ERROR_INJECTION() macro, by setting debugfs entries
+ under /sys/kernel/debug/fail_function. No boot option supported.
+
Configure fault-injection capabilities behavior
---
@@ -123,6 +129,29 @@ configuration of fault-injection capabilities.
default is 'N', setting it to 'Y' will disable failure injections
when dealing with private (address space) futexes.
+- /sys/kernel/debug/fail_function/inject:
+
+ Format: { 'function-name' | '!function-name' | '' }
+ specifies the target function of error injection by name.
+ If the function name leads '!' prefix, given function is
+ removed from injection list. If nothing specified ('')
+ injection list is cleared.
+
+- /sys/kernel/debug/fail_function/injectable:
+
+ (read only) shows error injectable functions and what type of
+ error values can be specified. The error type will be one of
+ below;
+ - NULL: retval must be 0.
+ - ERRNO: retval must be -1 to -MAX_ERRNO (-4096).
+ - ERR_NULL: retval must be 0 or -1 to -MAX_ERRNO (-4096).
+
+- /sys/kernel/debug/fail_function//retval:
+
+ specifies the "error" return value to inject to the given
+ function for given function. This will be created when
+ user specifies new injection entry.
+
o Boot option
In order to inject faults while debugfs is not available (early boot time),
@@ -268,6 +297,45 @@ trap "echo 0 > /sys/kernel/debug/$FAILTYPE/probability"
SIGINT SIGTERM EXIT
echo "Injecting errors into the module $module... (interrupt to stop)"
sleep 100
+--
+
+o Inject open_ctree error while btrfs mount
+
+#!/bin/bash
+
+rm -f testfile.img
+dd if=/dev/zero of=testfile.img bs=1M seek=1000 count=1
+DEVICE=$(losetup --show -f testfile.img)
+mkfs.btrfs -f $DEVICE
+mkdir -p tmpmnt
+
+FAILTYPE=fail_function
+FAILFUNC=open_ctree
+echo $FAILFUNC > /sys/kernel/debug/$FAILTYPE/inject
+echo -12 > /sys/kernel/debug/$FAILTYPE/$FAILFUNC/retval
+echo N > /sys/kernel/debug/$FAILTYPE/task-filter
+echo 100 > /sys/kernel/debug/$FAILTYPE/probability
+echo 0 > /sys/kernel/debug/$FAILTYPE/interval
+echo -1 > /sys/kernel/debug/$FAILTYPE/times
+echo 0 > /sys/kernel/debug/$FAILTYPE/space
+echo 1 > /sys/kernel/debug/$FAILTYPE/verbose
+
+mount -t btrfs $DEVICE tmpmnt
+if [ $? -ne 0 ]
+then
+ echo "SUCCESS!"
+else
+ echo "FAILED!"
+ umount tmpmnt
+fi
+
+echo > /sys/kernel/debug/$FAILTYPE/inject
+
+rmdir tmpmnt
+losetup -d $DEVICE
+rm testfile.img
+
+
Tool to run command with failslab or fail_
[PATCH bpf-next v5 4/5] error-injection: Add injectable error types
Add injectable error types for each error-injectable function.
One motivation of error injection test is to find software flaws,
mistakes or mis-handlings of expectable errors. If we find such
flaws by the test, that is a program bug, so we need to fix it.
But if the tester miss input the error (e.g. just return success
code without processing anything), it causes unexpected behavior
even if the caller is correctly programmed to handle any errors.
That is not what we want to test by error injection.
To clarify what type of errors the caller must expect for each
injectable function, this introduces injectable error types:
- EI_ETYPE_NULL : means the function will return NULL if it
fails. No ERR_PTR, just a NULL.
- EI_ETYPE_ERRNO : means the function will return -ERRNO
if it fails.
- EI_ETYPE_ERRNO_NULL : means the function will return -ERRNO
(ERR_PTR) or NULL.
ALLOW_ERROR_INJECTION() macro is expanded to get one of
NULL, ERRNO, ERRNO_NULL to record the error type for
each function. e.g.
ALLOW_ERROR_INJECTION(open_ctree, ERRNO)
This error types are shown in debugfs as below.
/ # cat /sys/kernel/debug/error_injection/list
open_ctree [btrfs]ERRNO
io_ctl_init [btrfs] ERRNO
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
Reviewed-by: Josef Bacik <jba...@fb.com>
---
fs/btrfs/disk-io.c|2 +-
fs/btrfs/free-space-cache.c |2 +-
include/asm-generic/error-injection.h | 23 +++---
include/asm-generic/vmlinux.lds.h |2 +-
include/linux/error-injection.h |6 +
include/linux/module.h|3 ++
lib/error-inject.c| 43 -
7 files changed, 66 insertions(+), 15 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 9798e21ebe9d..83e2349e1362 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3124,7 +3124,7 @@ int open_ctree(struct super_block *sb,
goto fail_block_groups;
goto retry_root_backup;
}
-ALLOW_ERROR_INJECTION(open_ctree);
+ALLOW_ERROR_INJECTION(open_ctree, ERRNO);
static void btrfs_end_buffer_write_sync(struct buffer_head *bh, int uptodate)
{
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index ef847699031a..586bb06472bb 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -333,7 +333,7 @@ static int io_ctl_init(struct btrfs_io_ctl *io_ctl, struct
inode *inode,
return 0;
}
-ALLOW_ERROR_INJECTION(io_ctl_init);
+ALLOW_ERROR_INJECTION(io_ctl_init, ERRNO);
static void io_ctl_free(struct btrfs_io_ctl *io_ctl)
{
diff --git a/include/asm-generic/error-injection.h
b/include/asm-generic/error-injection.h
index 08352c9d9f97..296c65442f00 100644
--- a/include/asm-generic/error-injection.h
+++ b/include/asm-generic/error-injection.h
@@ -3,17 +3,32 @@
#define _ASM_GENERIC_ERROR_INJECTION_H
#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
+enum {
+ EI_ETYPE_NONE, /* Dummy value for undefined case */
+ EI_ETYPE_NULL, /* Return NULL if failure */
+ EI_ETYPE_ERRNO, /* Return -ERRNO if failure */
+ EI_ETYPE_ERRNO_NULL,/* Return -ERRNO or NULL if failure */
+};
+
+struct error_injection_entry {
+ unsigned long addr;
+ int etype;
+};
+
#ifdef CONFIG_FUNCTION_ERROR_INJECTION
/*
* Whitelist ganerating macro. Specify functions which can be
* error-injectable using this macro.
*/
-#define ALLOW_ERROR_INJECTION(fname) \
-static unsigned long __used\
+#define ALLOW_ERROR_INJECTION(fname, _etype) \
+static struct error_injection_entry __used \
__attribute__((__section__("_error_injection_whitelist"))) \
- _eil_addr_##fname = (unsigned long)fname;
+ _eil_addr_##fname = { \
+ .addr = (unsigned long)fname, \
+ .etype = EI_ETYPE_##_etype, \
+ };
#else
-#define ALLOW_ERROR_INJECTION(fname)
+#define ALLOW_ERROR_INJECTION(fname, _etype)
#endif
#endif
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index f2068cca5206..ebe544e048cd 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -137,7 +137,7 @@
#endif
#ifdef CONFIG_FUNCTION_ERROR_INJECTION
-#define ERROR_INJECT_WHITELIST() . = ALIGN(8); \
+#define ERROR_INJECT_WHITELIST() STRUCT_ALIGN(); \
VMLINUX_SYMBOL(__start_error_injection_whitelist) = .;\
KEEP(*(_error_injection_whitelist)) \
VMLINUX_SYMBOL(__
[PATCH bpf-next v5 2/5] tracing/kprobe: bpf: Compare instruction pointer with original one
Compare instruction pointer with original one on the
stack instead using per-cpu bpf_kprobe_override flag.
This patch also consolidates reset_current_kprobe() and
preempt_enable_no_resched() blocks. Those can be done
in one place.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
Reviewed-by: Josef Bacik <jba...@fb.com>
---
kernel/trace/bpf_trace.c|1 -
kernel/trace/trace_kprobe.c | 21 +++--
2 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 1966ad3bf3e0..24ed6363e00f 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -83,7 +83,6 @@ EXPORT_SYMBOL_GPL(trace_call_bpf);
#ifdef CONFIG_BPF_KPROBE_OVERRIDE
BPF_CALL_2(bpf_override_return, struct pt_regs *, regs, unsigned long, rc)
{
- __this_cpu_write(bpf_kprobe_override, 1);
regs_set_return_value(regs, rc);
arch_kprobe_override_function(regs);
return 0;
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 3c8deb977a8b..b8c90441bc87 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -42,8 +42,6 @@ struct trace_kprobe {
(offsetof(struct trace_kprobe, tp.args) + \
(sizeof(struct probe_arg) * (n)))
-DEFINE_PER_CPU(int, bpf_kprobe_override);
-
static nokprobe_inline bool trace_kprobe_is_return(struct trace_kprobe *tk)
{
return tk->rp.handler != NULL;
@@ -1205,6 +1203,7 @@ kprobe_perf_func(struct trace_kprobe *tk, struct pt_regs
*regs)
int rctx;
if (bpf_prog_array_valid(call)) {
+ unsigned long orig_ip = instruction_pointer(regs);
int ret;
ret = trace_call_bpf(call, regs);
@@ -1212,12 +1211,13 @@ kprobe_perf_func(struct trace_kprobe *tk, struct
pt_regs *regs)
/*
* We need to check and see if we modified the pc of the
* pt_regs, and if so clear the kprobe and return 1 so that we
-* don't do the instruction skipping. Also reset our state so
-* we are clean the next pass through.
+* don't do the single stepping.
+* The ftrace kprobe handler leaves it up to us to re-enable
+* preemption here before returning if we've modified the ip.
*/
- if (__this_cpu_read(bpf_kprobe_override)) {
- __this_cpu_write(bpf_kprobe_override, 0);
+ if (orig_ip != instruction_pointer(regs)) {
reset_current_kprobe();
+ preempt_enable_no_resched();
return 1;
}
if (!ret)
@@ -1325,15 +1325,8 @@ static int kprobe_dispatcher(struct kprobe *kp, struct
pt_regs *regs)
if (tk->tp.flags & TP_FLAG_TRACE)
kprobe_trace_func(tk, regs);
#ifdef CONFIG_PERF_EVENTS
- if (tk->tp.flags & TP_FLAG_PROFILE) {
+ if (tk->tp.flags & TP_FLAG_PROFILE)
ret = kprobe_perf_func(tk, regs);
- /*
-* The ftrace kprobe handler leaves it up to us to re-enable
-* preemption here before returning if we've modified the ip.
-*/
- if (ret)
- preempt_enable_no_resched();
- }
#endif
return ret;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH bpf-next v5 1/5] tracing/kprobe: bpf: Check error injectable event is on function entry
Check whether error injectable event is on function entry or not.
Currently it checks the event is ftrace-based kprobes or not,
but that is wrong. It should check if the event is on the entry
of target function. Since error injection will override a function
to just return with modified return value, that operation must
be done before the target function starts making stackframe.
As a side effect, bpf error injection is no need to depend on
function-tracer. It can work with sw-breakpoint based kprobe
events too.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
Reviewed-by: Josef Bacik <jba...@fb.com>
---
Changes in v3:
- Move arch_ftrace_kprobe_override_function() to
core.c because it is no longer depending on ftrace.
- Fix a bug to skip passing kprobes target name to
kprobe_on_func_entry(). Passing both @addr and @symbol
to that function will result in failure.
---
arch/x86/include/asm/kprobes.h |4 +---
arch/x86/kernel/kprobes/core.c | 14 ++
arch/x86/kernel/kprobes/ftrace.c | 14 --
kernel/trace/Kconfig |2 --
kernel/trace/bpf_trace.c |8
kernel/trace/trace_kprobe.c |9 ++---
kernel/trace/trace_probe.h | 12 ++--
7 files changed, 31 insertions(+), 32 deletions(-)
diff --git a/arch/x86/include/asm/kprobes.h b/arch/x86/include/asm/kprobes.h
index 36abb23a7a35..367d99cff426 100644
--- a/arch/x86/include/asm/kprobes.h
+++ b/arch/x86/include/asm/kprobes.h
@@ -67,9 +67,7 @@ extern const int kretprobe_blacklist_size;
void arch_remove_kprobe(struct kprobe *p);
asmlinkage void kretprobe_trampoline(void);
-#ifdef CONFIG_KPROBES_ON_FTRACE
-extern void arch_ftrace_kprobe_override_function(struct pt_regs *regs);
-#endif
+extern void arch_kprobe_override_function(struct pt_regs *regs);
/* Architecture specific copy of original instruction*/
struct arch_specific_insn {
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index bd36f3c33cd0..b02a377d5905 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -1183,3 +1183,17 @@ int arch_trampoline_kprobe(struct kprobe *p)
{
return 0;
}
+
+asmlinkage void override_func(void);
+asm(
+ ".type override_func, @function\n"
+ "override_func:\n"
+ " ret\n"
+ ".size override_func, .-override_func\n"
+);
+
+void arch_kprobe_override_function(struct pt_regs *regs)
+{
+ regs->ip = (unsigned long)_func;
+}
+NOKPROBE_SYMBOL(arch_kprobe_override_function);
diff --git a/arch/x86/kernel/kprobes/ftrace.c b/arch/x86/kernel/kprobes/ftrace.c
index 1ea748d682fd..8dc0161cec8f 100644
--- a/arch/x86/kernel/kprobes/ftrace.c
+++ b/arch/x86/kernel/kprobes/ftrace.c
@@ -97,17 +97,3 @@ int arch_prepare_kprobe_ftrace(struct kprobe *p)
p->ainsn.boostable = false;
return 0;
}
-
-asmlinkage void override_func(void);
-asm(
- ".type override_func, @function\n"
- "override_func:\n"
- " ret\n"
- ".size override_func, .-override_func\n"
-);
-
-void arch_ftrace_kprobe_override_function(struct pt_regs *regs)
-{
- regs->ip = (unsigned long)_func;
-}
-NOKPROBE_SYMBOL(arch_ftrace_kprobe_override_function);
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index ae3a2d519e50..6400e1bf97c5 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -533,9 +533,7 @@ config FUNCTION_PROFILER
config BPF_KPROBE_OVERRIDE
bool "Enable BPF programs to override a kprobed function"
depends on BPF_EVENTS
- depends on KPROBES_ON_FTRACE
depends on HAVE_KPROBE_OVERRIDE
- depends on DYNAMIC_FTRACE_WITH_REGS
default n
help
Allows BPF to override the execution of a probed function and
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index f6d2327ecb59..1966ad3bf3e0 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -85,7 +85,7 @@ BPF_CALL_2(bpf_override_return, struct pt_regs *, regs,
unsigned long, rc)
{
__this_cpu_write(bpf_kprobe_override, 1);
regs_set_return_value(regs, rc);
- arch_ftrace_kprobe_override_function(regs);
+ arch_kprobe_override_function(regs);
return 0;
}
@@ -800,11 +800,11 @@ int perf_event_attach_bpf_prog(struct perf_event *event,
int ret = -EEXIST;
/*
-* Kprobe override only works for ftrace based kprobes, and only if they
-* are on the opt-in list.
+* Kprobe override only works if they are on the function entry,
+* and only if they are on the opt-in list.
*/
if (prog->kprobe_override &&
- (!trace_kprobe_ftrace(event->tp_event) ||
+ (!trace_kprobe_on_func_entry(event->tp_event) ||
!trace_kprobe_error_injectable(event->tp_event)))
[PATCH bpf-next v5 0/5] Separate error injection table from kprobes
Hi, Here are the 5th version of patches to moving error injection table from kprobes. This version fixes a bug and update fail-function to support multiple function error injection. Here is the previous version: https://patchwork.ozlabs.org/cover/858663/ Changes in v5: - [3/5] Fix a bug that within_error_injection returns false always. - [5/5] Update to support multiple function error injection. Thank you, --- Masami Hiramatsu (5): tracing/kprobe: bpf: Check error injectable event is on function entry tracing/kprobe: bpf: Compare instruction pointer with original one error-injection: Separate error-injection from kprobe error-injection: Add injectable error types error-injection: Support fault injection framework Documentation/fault-injection/fault-injection.txt | 68 arch/Kconfig |2 arch/x86/Kconfig |2 arch/x86/include/asm/error-injection.h| 13 + arch/x86/include/asm/kprobes.h|4 arch/x86/kernel/kprobes/ftrace.c | 14 - arch/x86/lib/Makefile |1 arch/x86/lib/error-inject.c | 19 + fs/btrfs/disk-io.c|4 fs/btrfs/free-space-cache.c |4 include/asm-generic/error-injection.h | 35 ++ include/asm-generic/vmlinux.lds.h | 14 - include/linux/bpf.h | 11 - include/linux/error-injection.h | 27 ++ include/linux/kprobes.h |1 include/linux/module.h|7 kernel/Makefile |1 kernel/fail_function.c| 349 + kernel/kprobes.c | 163 -- kernel/module.c |8 kernel/trace/Kconfig |4 kernel/trace/bpf_trace.c | 11 - kernel/trace/trace_kprobe.c | 33 +- kernel/trace/trace_probe.h| 12 - lib/Kconfig.debug | 14 + lib/Makefile |1 lib/error-inject.c| 242 +++ 27 files changed, 819 insertions(+), 245 deletions(-) create mode 100644 arch/x86/include/asm/error-injection.h create mode 100644 arch/x86/lib/error-inject.c create mode 100644 include/asm-generic/error-injection.h create mode 100644 include/linux/error-injection.h create mode 100644 kernel/fail_function.c create mode 100644 lib/error-inject.c -- Masami Hiramatsu (Linaro) -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH bpf-next v4 5/5] error-injection: Support fault injection framework
On Thu, 11 Jan 2018 23:44:57 +0900 Akinobu Mita <akinobu.m...@gmail.com> wrote: > 2018-01-11 9:51 GMT+09:00 Masami Hiramatsu <mhira...@kernel.org>: > > Support in-kernel fault-injection framework via debugfs. > > This allows you to inject a conditional error to specified > > function using debugfs interfaces. > > > > Here is the result of test script described in > > Documentation/fault-injection/fault-injection.txt > > > > === > > # ./test_fail_function.sh > > 1+0 records in > > 1+0 records out > > 1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.0227404 s, 46.1 MB/s > > btrfs-progs v4.4 > > See http://btrfs.wiki.kernel.org for more information. > > > > Label: (null) > > UUID: bfa96010-12e9-4360-aed0-42eec7af5798 > > Node size: 16384 > > Sector size:4096 > > Filesystem size:1001.00MiB > > Block group profiles: > > Data: single8.00MiB > > Metadata: DUP 58.00MiB > > System: DUP 12.00MiB > > SSD detected: no > > Incompat features: extref, skinny-metadata > > Number of devices: 1 > > Devices: > > ID SIZE PATH > > 1 1001.00MiB /dev/loop2 > > > > mount: mount /dev/loop2 on /opt/tmpmnt failed: Cannot allocate memory > > SUCCESS! > > === > > > > > > Signed-off-by: Masami Hiramatsu <mhira...@kernel.org> > > Reviewed-by: Josef Bacik <jba...@fb.com> > > --- > > Changes in v3: > >- Check and adjust error value for each target function > >- Clear kporbe flag for reuse > >- Add more documents and example > > --- > > Documentation/fault-injection/fault-injection.txt | 62 ++ > > kernel/Makefile |1 > > kernel/fail_function.c| 217 > > + > > lib/Kconfig.debug | 10 + > > 4 files changed, 290 insertions(+) > > create mode 100644 kernel/fail_function.c > > > > diff --git a/Documentation/fault-injection/fault-injection.txt > > b/Documentation/fault-injection/fault-injection.txt > > index 918972babcd8..4aecbceef9d2 100644 > > --- a/Documentation/fault-injection/fault-injection.txt > > +++ b/Documentation/fault-injection/fault-injection.txt > > @@ -30,6 +30,12 @@ o fail_mmc_request > >injects MMC data errors on devices permitted by setting > >debugfs entries under /sys/kernel/debug/mmc0/fail_mmc_request > > > > +o fail_function > > + > > + injects error return on specific functions, which are marked by > > + ALLOW_ERROR_INJECTION() macro, by setting debugfs entries > > + under /sys/kernel/debug/fail_function. No boot option supported. > > + > > Configure fault-injection capabilities behavior > > --- > > > > @@ -123,6 +129,24 @@ configuration of fault-injection capabilities. > > default is 'N', setting it to 'Y' will disable failure injections > > when dealing with private (address space) futexes. > > > > +- /sys/kernel/debug/fail_function/inject: > > + > > + specifies the target function of error injection by name. > > + > > +- /sys/kernel/debug/fail_function/retval: > > + > > + specifies the "error" return value to inject to the given > > + function. > > + > > Is it possible to inject errors into multiple functions at the same time? Yes, it is. > If so, it will be more useful to support it in the fault injection, too. > Because some kind of bugs are caused by the combination of errors. > (e.g. another error in an error path) > > I suggest the following interface. > > - /sys/kernel/debug/fail_function/inject: > > specifies the target function of error injection by name. > /sys/kernel/debug/fail_function// directory will be created. > > - /sys/kernel/debug/fail_function/uninject: > > specifies the target function of error injection by name that is > currently being injected. /sys/kernel/debug/fail_function// > directory will be removed. > > - /sys/kernel/debug/fail_function//retval: > > specifies the "error" return value to inject to the given function. OK, it is easy to make it. But also we might need to consider using bpf if we do such complex error injection. BTW, would we need "uninject" file? or just make inject file accept "!function" syntax to remove function as ftrace does? Thank you, -- Masami Hiramatsu <mhira...@kernel.org> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH bpf-next v4 5/5] error-injection: Support fault injection framework
Support in-kernel fault-injection framework via debugfs. This allows you to inject a conditional error to specified function using debugfs interfaces. Here is the result of test script described in Documentation/fault-injection/fault-injection.txt === # ./test_fail_function.sh 1+0 records in 1+0 records out 1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.0227404 s, 46.1 MB/s btrfs-progs v4.4 See http://btrfs.wiki.kernel.org for more information. Label: (null) UUID: bfa96010-12e9-4360-aed0-42eec7af5798 Node size: 16384 Sector size:4096 Filesystem size:1001.00MiB Block group profiles: Data: single8.00MiB Metadata: DUP 58.00MiB System: DUP 12.00MiB SSD detected: no Incompat features: extref, skinny-metadata Number of devices: 1 Devices: IDSIZE PATH 1 1001.00MiB /dev/loop2 mount: mount /dev/loop2 on /opt/tmpmnt failed: Cannot allocate memory SUCCESS! === Signed-off-by: Masami Hiramatsu <mhira...@kernel.org> Reviewed-by: Josef Bacik <jba...@fb.com> --- Changes in v3: - Check and adjust error value for each target function - Clear kporbe flag for reuse - Add more documents and example --- Documentation/fault-injection/fault-injection.txt | 62 ++ kernel/Makefile |1 kernel/fail_function.c| 217 + lib/Kconfig.debug | 10 + 4 files changed, 290 insertions(+) create mode 100644 kernel/fail_function.c diff --git a/Documentation/fault-injection/fault-injection.txt b/Documentation/fault-injection/fault-injection.txt index 918972babcd8..4aecbceef9d2 100644 --- a/Documentation/fault-injection/fault-injection.txt +++ b/Documentation/fault-injection/fault-injection.txt @@ -30,6 +30,12 @@ o fail_mmc_request injects MMC data errors on devices permitted by setting debugfs entries under /sys/kernel/debug/mmc0/fail_mmc_request +o fail_function + + injects error return on specific functions, which are marked by + ALLOW_ERROR_INJECTION() macro, by setting debugfs entries + under /sys/kernel/debug/fail_function. No boot option supported. + Configure fault-injection capabilities behavior --- @@ -123,6 +129,24 @@ configuration of fault-injection capabilities. default is 'N', setting it to 'Y' will disable failure injections when dealing with private (address space) futexes. +- /sys/kernel/debug/fail_function/inject: + + specifies the target function of error injection by name. + +- /sys/kernel/debug/fail_function/retval: + + specifies the "error" return value to inject to the given + function. + +- /sys/kernel/debug/fail_function/injectable: + + (read only) shows error injectable functions and what type of + error values can be specified. The error type will be one of + below; + - NULL: retval must be 0. + - ERRNO: retval must be -1 to -MAX_ERRNO (-4096). + - ERR_NULL: retval must be 0 or -1 to -MAX_ERRNO (-4096). + o Boot option In order to inject faults while debugfs is not available (early boot time), @@ -268,6 +292,44 @@ trap "echo 0 > /sys/kernel/debug/$FAILTYPE/probability" SIGINT SIGTERM EXIT echo "Injecting errors into the module $module... (interrupt to stop)" sleep 100 +-- + +o Inject open_ctree error while btrfs mount + +#!/bin/bash + +rm -f testfile.img +dd if=/dev/zero of=testfile.img bs=1M seek=1000 count=1 +DEVICE=$(losetup --show -f testfile.img) +mkfs.btrfs -f $DEVICE +mkdir -p tmpmnt + +FAILTYPE=fail_function +echo open_ctree > /sys/kernel/debug/$FAILTYPE/inject +echo -12 > /sys/kernel/debug/$FAILTYPE/retval +echo N > /sys/kernel/debug/$FAILTYPE/task-filter +echo 100 > /sys/kernel/debug/$FAILTYPE/probability +echo 0 > /sys/kernel/debug/$FAILTYPE/interval +echo -1 > /sys/kernel/debug/$FAILTYPE/times +echo 0 > /sys/kernel/debug/$FAILTYPE/space +echo 1 > /sys/kernel/debug/$FAILTYPE/verbose + +mount -t btrfs $DEVICE tmpmnt +if [ $? -ne 0 ] +then + echo "SUCCESS!" +else + echo "FAILED!" + umount tmpmnt +fi + +echo > /sys/kernel/debug/$FAILTYPE/inject + +rmdir tmpmnt +losetup -d $DEVICE +rm testfile.img + + Tool to run command with failslab or fail_page_alloc In order to make it easier to accomplish the tasks mentioned above, we can use diff --git a/kernel/Makefile b/kernel/Makefile index 172d151d429c..f85ae5dfa474 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -81,6 +81,7 @@ obj-$(CONFIG_AUDIT_TREE) += audit_tree.o obj-$(CONFIG_GCOV_KERNEL) += gcov/ obj-$(CONFIG_KCOV) += kco
[PATCH bpf-next v4 4/5] error-injection: Add injectable error types
Add injectable error types for each error-injectable function.
One motivation of error injection test is to find software flaws,
mistakes or mis-handlings of expectable errors. If we find such
flaws by the test, that is a program bug, so we need to fix it.
But if the tester miss input the error (e.g. just return success
code without processing anything), it causes unexpected behavior
even if the caller is correctly programmed to handle any errors.
That is not what we want to test by error injection.
To clarify what type of errors the caller must expect for each
injectable function, this introduces injectable error types:
- EI_ETYPE_NULL : means the function will return NULL if it
fails. No ERR_PTR, just a NULL.
- EI_ETYPE_ERRNO : means the function will return -ERRNO
if it fails.
- EI_ETYPE_ERRNO_NULL : means the function will return -ERRNO
(ERR_PTR) or NULL.
ALLOW_ERROR_INJECTION() macro is expanded to get one of
NULL, ERRNO, ERRNO_NULL to record the error type for
each function. e.g.
ALLOW_ERROR_INJECTION(open_ctree, ERRNO)
This error types are shown in debugfs as below.
/ # cat /sys/kernel/debug/error_injection/list
open_ctree [btrfs]ERRNO
io_ctl_init [btrfs] ERRNO
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
Reviewed-by: Josef Bacik <jba...@fb.com>
---
fs/btrfs/disk-io.c|2 +-
fs/btrfs/free-space-cache.c |2 +-
include/asm-generic/error-injection.h | 23 +++---
include/asm-generic/vmlinux.lds.h |2 +-
include/linux/error-injection.h |6 +
include/linux/module.h|3 ++
lib/error-inject.c| 43 -
7 files changed, 66 insertions(+), 15 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 9798e21ebe9d..83e2349e1362 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3124,7 +3124,7 @@ int open_ctree(struct super_block *sb,
goto fail_block_groups;
goto retry_root_backup;
}
-ALLOW_ERROR_INJECTION(open_ctree);
+ALLOW_ERROR_INJECTION(open_ctree, ERRNO);
static void btrfs_end_buffer_write_sync(struct buffer_head *bh, int uptodate)
{
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index ef847699031a..586bb06472bb 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -333,7 +333,7 @@ static int io_ctl_init(struct btrfs_io_ctl *io_ctl, struct
inode *inode,
return 0;
}
-ALLOW_ERROR_INJECTION(io_ctl_init);
+ALLOW_ERROR_INJECTION(io_ctl_init, ERRNO);
static void io_ctl_free(struct btrfs_io_ctl *io_ctl)
{
diff --git a/include/asm-generic/error-injection.h
b/include/asm-generic/error-injection.h
index 08352c9d9f97..296c65442f00 100644
--- a/include/asm-generic/error-injection.h
+++ b/include/asm-generic/error-injection.h
@@ -3,17 +3,32 @@
#define _ASM_GENERIC_ERROR_INJECTION_H
#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
+enum {
+ EI_ETYPE_NONE, /* Dummy value for undefined case */
+ EI_ETYPE_NULL, /* Return NULL if failure */
+ EI_ETYPE_ERRNO, /* Return -ERRNO if failure */
+ EI_ETYPE_ERRNO_NULL,/* Return -ERRNO or NULL if failure */
+};
+
+struct error_injection_entry {
+ unsigned long addr;
+ int etype;
+};
+
#ifdef CONFIG_FUNCTION_ERROR_INJECTION
/*
* Whitelist ganerating macro. Specify functions which can be
* error-injectable using this macro.
*/
-#define ALLOW_ERROR_INJECTION(fname) \
-static unsigned long __used\
+#define ALLOW_ERROR_INJECTION(fname, _etype) \
+static struct error_injection_entry __used \
__attribute__((__section__("_error_injection_whitelist"))) \
- _eil_addr_##fname = (unsigned long)fname;
+ _eil_addr_##fname = { \
+ .addr = (unsigned long)fname, \
+ .etype = EI_ETYPE_##_etype, \
+ };
#else
-#define ALLOW_ERROR_INJECTION(fname)
+#define ALLOW_ERROR_INJECTION(fname, _etype)
#endif
#endif
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index f2068cca5206..ebe544e048cd 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -137,7 +137,7 @@
#endif
#ifdef CONFIG_FUNCTION_ERROR_INJECTION
-#define ERROR_INJECT_WHITELIST() . = ALIGN(8); \
+#define ERROR_INJECT_WHITELIST() STRUCT_ALIGN(); \
VMLINUX_SYMBOL(__start_error_injection_whitelist) = .;\
KEEP(*(_error_injection_whitelist)) \
VMLINUX_SYMBOL(__
[PATCH bpf-next v4 3/5] error-injection: Separate error-injection from kprobe
Since error-injection framework is not limited to be used
by kprobes, nor bpf. Other kernel subsystems can use it
freely for checking safeness of error-injection, e.g.
livepatch, ftrace etc.
So this separate error-injection framework from kprobes.
Some differences has been made:
- "kprobe" word is removed from any APIs/structures.
- BPF_ALLOW_ERROR_INJECTION() is renamed to
ALLOW_ERROR_INJECTION() since it is not limited for BPF too.
- CONFIG_FUNCTION_ERROR_INJECTION is the config item of this
feature. It is automatically enabled if the arch supports
error injection feature for kprobe or ftrace etc.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
---
Changes in v3:
- Fix a build error for asmlinkage on i386 by including compiler.h
- Fix "CONFIG_FUNCTION_ERROR_INJECT" typo.
- Separate CONFIG_MODULES dependent code
- Add CONFIG_KPROBES dependency for arch_deref_entry_point()
- Call error-injection init function in late_initcall stage.
- Fix read-side mutex lock
- Some cosmetic cleanups
Changes in v4:
- Include error-injection.h for each ALLOW_ERROR_INJECTION
macro user, instead of bpf.h
---
arch/Kconfig |2
arch/x86/Kconfig |2
arch/x86/include/asm/error-injection.h | 13 ++
arch/x86/kernel/kprobes/core.c | 14 --
arch/x86/lib/Makefile |1
arch/x86/lib/error-inject.c| 19 +++
fs/btrfs/disk-io.c |4 -
fs/btrfs/free-space-cache.c|4 -
include/asm-generic/error-injection.h | 20 +++
include/asm-generic/vmlinux.lds.h | 14 +-
include/linux/bpf.h| 11 --
include/linux/error-injection.h| 21 +++
include/linux/kprobes.h|1
include/linux/module.h |6 -
kernel/kprobes.c | 163
kernel/module.c|8 +
kernel/trace/Kconfig |2
kernel/trace/bpf_trace.c |4 -
kernel/trace/trace_kprobe.c|3
lib/Kconfig.debug |4 +
lib/Makefile |1
lib/error-inject.c | 213
22 files changed, 317 insertions(+), 213 deletions(-)
create mode 100644 arch/x86/include/asm/error-injection.h
create mode 100644 arch/x86/lib/error-inject.c
create mode 100644 include/asm-generic/error-injection.h
create mode 100644 include/linux/error-injection.h
create mode 100644 lib/error-inject.c
diff --git a/arch/Kconfig b/arch/Kconfig
index d3f4aaf9cb7a..97376accfb14 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -196,7 +196,7 @@ config HAVE_OPTPROBES
config HAVE_KPROBES_ON_FTRACE
bool
-config HAVE_KPROBE_OVERRIDE
+config HAVE_FUNCTION_ERROR_INJECTION
bool
config HAVE_NMI
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 45dc6233f2b9..366b19cb79b7 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -154,7 +154,7 @@ config X86
select HAVE_KERNEL_XZ
select HAVE_KPROBES
select HAVE_KPROBES_ON_FTRACE
- select HAVE_KPROBE_OVERRIDE
+ select HAVE_FUNCTION_ERROR_INJECTION
select HAVE_KRETPROBES
select HAVE_KVM
select HAVE_LIVEPATCH if X86_64
diff --git a/arch/x86/include/asm/error-injection.h
b/arch/x86/include/asm/error-injection.h
new file mode 100644
index ..47b7a1296245
--- /dev/null
+++ b/arch/x86/include/asm/error-injection.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ERROR_INJECTION_H
+#define _ASM_ERROR_INJECTION_H
+
+#include
+#include
+#include
+#include
+
+asmlinkage void just_return_func(void);
+void override_function_with_return(struct pt_regs *regs);
+
+#endif /* _ASM_ERROR_INJECTION_H */
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index b02a377d5905..bd36f3c33cd0 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -1183,17 +1183,3 @@ int arch_trampoline_kprobe(struct kprobe *p)
{
return 0;
}
-
-asmlinkage void override_func(void);
-asm(
- ".type override_func, @function\n"
- "override_func:\n"
- " ret\n"
- ".size override_func, .-override_func\n"
-);
-
-void arch_kprobe_override_function(struct pt_regs *regs)
-{
- regs->ip = (unsigned long)_func;
-}
-NOKPROBE_SYMBOL(arch_kprobe_override_function);
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index 7b181b61170e..171377b83be1 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -26,6 +26,7 @@ lib-y += memcpy_$(BITS).o
lib-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o
lib-$(CONFIG_INSTRUCTION_DECODER) += insn.o inat.o insn-eval.o
lib-$(CONFIG_RANDOMIZE_BASE) += kaslr.o
+lib-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
ob
[PATCH bpf-next v4 2/5] tracing/kprobe: bpf: Compare instruction pointer with original one
Compare instruction pointer with original one on the
stack instead using per-cpu bpf_kprobe_override flag.
This patch also consolidates reset_current_kprobe() and
preempt_enable_no_resched() blocks. Those can be done
in one place.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
Reviewed-by: Josef Bacik <jba...@fb.com>
---
kernel/trace/bpf_trace.c|1 -
kernel/trace/trace_kprobe.c | 21 +++--
2 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 1966ad3bf3e0..24ed6363e00f 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -83,7 +83,6 @@ EXPORT_SYMBOL_GPL(trace_call_bpf);
#ifdef CONFIG_BPF_KPROBE_OVERRIDE
BPF_CALL_2(bpf_override_return, struct pt_regs *, regs, unsigned long, rc)
{
- __this_cpu_write(bpf_kprobe_override, 1);
regs_set_return_value(regs, rc);
arch_kprobe_override_function(regs);
return 0;
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 3c8deb977a8b..b8c90441bc87 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -42,8 +42,6 @@ struct trace_kprobe {
(offsetof(struct trace_kprobe, tp.args) + \
(sizeof(struct probe_arg) * (n)))
-DEFINE_PER_CPU(int, bpf_kprobe_override);
-
static nokprobe_inline bool trace_kprobe_is_return(struct trace_kprobe *tk)
{
return tk->rp.handler != NULL;
@@ -1205,6 +1203,7 @@ kprobe_perf_func(struct trace_kprobe *tk, struct pt_regs
*regs)
int rctx;
if (bpf_prog_array_valid(call)) {
+ unsigned long orig_ip = instruction_pointer(regs);
int ret;
ret = trace_call_bpf(call, regs);
@@ -1212,12 +1211,13 @@ kprobe_perf_func(struct trace_kprobe *tk, struct
pt_regs *regs)
/*
* We need to check and see if we modified the pc of the
* pt_regs, and if so clear the kprobe and return 1 so that we
-* don't do the instruction skipping. Also reset our state so
-* we are clean the next pass through.
+* don't do the single stepping.
+* The ftrace kprobe handler leaves it up to us to re-enable
+* preemption here before returning if we've modified the ip.
*/
- if (__this_cpu_read(bpf_kprobe_override)) {
- __this_cpu_write(bpf_kprobe_override, 0);
+ if (orig_ip != instruction_pointer(regs)) {
reset_current_kprobe();
+ preempt_enable_no_resched();
return 1;
}
if (!ret)
@@ -1325,15 +1325,8 @@ static int kprobe_dispatcher(struct kprobe *kp, struct
pt_regs *regs)
if (tk->tp.flags & TP_FLAG_TRACE)
kprobe_trace_func(tk, regs);
#ifdef CONFIG_PERF_EVENTS
- if (tk->tp.flags & TP_FLAG_PROFILE) {
+ if (tk->tp.flags & TP_FLAG_PROFILE)
ret = kprobe_perf_func(tk, regs);
- /*
-* The ftrace kprobe handler leaves it up to us to re-enable
-* preemption here before returning if we've modified the ip.
-*/
- if (ret)
- preempt_enable_no_resched();
- }
#endif
return ret;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH bpf-next v4 1/5] tracing/kprobe: bpf: Check error injectable event is on function entry
Check whether error injectable event is on function entry or not.
Currently it checks the event is ftrace-based kprobes or not,
but that is wrong. It should check if the event is on the entry
of target function. Since error injection will override a function
to just return with modified return value, that operation must
be done before the target function starts making stackframe.
As a side effect, bpf error injection is no need to depend on
function-tracer. It can work with sw-breakpoint based kprobe
events too.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
Reviewed-by: Josef Bacik <jba...@fb.com>
---
Changes in v3:
- Move arch_ftrace_kprobe_override_function() to
core.c because it is no longer depending on ftrace.
- Fix a bug to skip passing kprobes target name to
kprobe_on_func_entry(). Passing both @addr and @symbol
to that function will result in failure.
---
arch/x86/include/asm/kprobes.h |4 +---
arch/x86/kernel/kprobes/core.c | 14 ++
arch/x86/kernel/kprobes/ftrace.c | 14 --
kernel/trace/Kconfig |2 --
kernel/trace/bpf_trace.c |8
kernel/trace/trace_kprobe.c |9 ++---
kernel/trace/trace_probe.h | 12 ++--
7 files changed, 31 insertions(+), 32 deletions(-)
diff --git a/arch/x86/include/asm/kprobes.h b/arch/x86/include/asm/kprobes.h
index 36abb23a7a35..367d99cff426 100644
--- a/arch/x86/include/asm/kprobes.h
+++ b/arch/x86/include/asm/kprobes.h
@@ -67,9 +67,7 @@ extern const int kretprobe_blacklist_size;
void arch_remove_kprobe(struct kprobe *p);
asmlinkage void kretprobe_trampoline(void);
-#ifdef CONFIG_KPROBES_ON_FTRACE
-extern void arch_ftrace_kprobe_override_function(struct pt_regs *regs);
-#endif
+extern void arch_kprobe_override_function(struct pt_regs *regs);
/* Architecture specific copy of original instruction*/
struct arch_specific_insn {
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index bd36f3c33cd0..b02a377d5905 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -1183,3 +1183,17 @@ int arch_trampoline_kprobe(struct kprobe *p)
{
return 0;
}
+
+asmlinkage void override_func(void);
+asm(
+ ".type override_func, @function\n"
+ "override_func:\n"
+ " ret\n"
+ ".size override_func, .-override_func\n"
+);
+
+void arch_kprobe_override_function(struct pt_regs *regs)
+{
+ regs->ip = (unsigned long)_func;
+}
+NOKPROBE_SYMBOL(arch_kprobe_override_function);
diff --git a/arch/x86/kernel/kprobes/ftrace.c b/arch/x86/kernel/kprobes/ftrace.c
index 1ea748d682fd..8dc0161cec8f 100644
--- a/arch/x86/kernel/kprobes/ftrace.c
+++ b/arch/x86/kernel/kprobes/ftrace.c
@@ -97,17 +97,3 @@ int arch_prepare_kprobe_ftrace(struct kprobe *p)
p->ainsn.boostable = false;
return 0;
}
-
-asmlinkage void override_func(void);
-asm(
- ".type override_func, @function\n"
- "override_func:\n"
- " ret\n"
- ".size override_func, .-override_func\n"
-);
-
-void arch_ftrace_kprobe_override_function(struct pt_regs *regs)
-{
- regs->ip = (unsigned long)_func;
-}
-NOKPROBE_SYMBOL(arch_ftrace_kprobe_override_function);
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index ae3a2d519e50..6400e1bf97c5 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -533,9 +533,7 @@ config FUNCTION_PROFILER
config BPF_KPROBE_OVERRIDE
bool "Enable BPF programs to override a kprobed function"
depends on BPF_EVENTS
- depends on KPROBES_ON_FTRACE
depends on HAVE_KPROBE_OVERRIDE
- depends on DYNAMIC_FTRACE_WITH_REGS
default n
help
Allows BPF to override the execution of a probed function and
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index f6d2327ecb59..1966ad3bf3e0 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -85,7 +85,7 @@ BPF_CALL_2(bpf_override_return, struct pt_regs *, regs,
unsigned long, rc)
{
__this_cpu_write(bpf_kprobe_override, 1);
regs_set_return_value(regs, rc);
- arch_ftrace_kprobe_override_function(regs);
+ arch_kprobe_override_function(regs);
return 0;
}
@@ -800,11 +800,11 @@ int perf_event_attach_bpf_prog(struct perf_event *event,
int ret = -EEXIST;
/*
-* Kprobe override only works for ftrace based kprobes, and only if they
-* are on the opt-in list.
+* Kprobe override only works if they are on the function entry,
+* and only if they are on the opt-in list.
*/
if (prog->kprobe_override &&
- (!trace_kprobe_ftrace(event->tp_event) ||
+ (!trace_kprobe_on_func_entry(event->tp_event) ||
!trace_kprobe_error_injectable(event->tp_event)))
[PATCH bpf-next v4 0/5] Separate error injection table from kprobes
Hi, Here are the 4th version of patches to moving error injection table from kprobes. This series changes error-injection.h including points for ALLOW_ERROR_INJECTION macro and add Reviewed-by from Josef Bacik (except [3/5]). Here is the previous version: https://patchwork.ozlabs.org/cover/858176/ Changes in v3: - [3/5] Change error-injection.h including points to each file which uses ALLOW_ERROR_INJECTION instead of bpf.h - Add Reviewed-by from Josef Bacik except [3/5] Thank you, --- Masami Hiramatsu (5): tracing/kprobe: bpf: Check error injectable event is on function entry tracing/kprobe: bpf: Compare instruction pointer with original one error-injection: Separate error-injection from kprobe error-injection: Add injectable error types error-injection: Support fault injection framework Documentation/fault-injection/fault-injection.txt | 62 + arch/Kconfig |2 arch/x86/Kconfig |2 arch/x86/include/asm/error-injection.h| 13 + arch/x86/include/asm/kprobes.h|4 arch/x86/kernel/kprobes/ftrace.c | 14 - arch/x86/lib/Makefile |1 arch/x86/lib/error-inject.c | 19 ++ fs/btrfs/disk-io.c|4 fs/btrfs/free-space-cache.c |4 include/asm-generic/error-injection.h | 35 +++ include/asm-generic/vmlinux.lds.h | 14 + include/linux/bpf.h | 11 - include/linux/error-injection.h | 27 ++ include/linux/kprobes.h |1 include/linux/module.h|7 - kernel/Makefile |1 kernel/fail_function.c| 217 +++ kernel/kprobes.c | 163 -- kernel/module.c |8 - kernel/trace/Kconfig |4 kernel/trace/bpf_trace.c | 11 - kernel/trace/trace_kprobe.c | 33 +-- kernel/trace/trace_probe.h| 12 + lib/Kconfig.debug | 14 + lib/Makefile |1 lib/error-inject.c| 242 + 27 files changed, 681 insertions(+), 245 deletions(-) create mode 100644 arch/x86/include/asm/error-injection.h create mode 100644 arch/x86/lib/error-inject.c create mode 100644 include/asm-generic/error-injection.h create mode 100644 include/linux/error-injection.h create mode 100644 kernel/fail_function.c create mode 100644 lib/error-inject.c -- Masami Hiramatsu (Linaro) -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH bpf-next v3 3/5] error-injection: Separate error-injection from kprobe
On Wed, 10 Jan 2018 10:36:15 -0500 Josef Bacik <jo...@toxicpanda.com> wrote: > On Wed, Jan 10, 2018 at 07:18:05PM +0900, Masami Hiramatsu wrote: > > Since error-injection framework is not limited to be used > > by kprobes, nor bpf. Other kernel subsystems can use it > > freely for checking safeness of error-injection, e.g. > > livepatch, ftrace etc. > > So this separate error-injection framework from kprobes. > > > > Some differences has been made: > > > > - "kprobe" word is removed from any APIs/structures. > > - BPF_ALLOW_ERROR_INJECTION() is renamed to > > ALLOW_ERROR_INJECTION() since it is not limited for BPF too. > > - CONFIG_FUNCTION_ERROR_INJECTION is the config item of this > > feature. It is automatically enabled if the arch supports > > error injection feature for kprobe or ftrace etc. > > > > Signed-off-by: Masami Hiramatsu <mhira...@kernel.org> > > --- > > Changes in v3: > >- Fix a build error for asmlinkage on i386 by including compiler.h > >- Fix "CONFIG_FUNCTION_ERROR_INJECT" typo. > >- Separate CONFIG_MODULES dependent code > >- Add CONFIG_KPROBES dependency for arch_deref_entry_point() > >- Call error-injection init function in late_initcall stage. > >- Fix read-side mutex lock > >- Some cosmetic cleanups > > --- > > arch/Kconfig |2 > > arch/x86/Kconfig |2 > > arch/x86/include/asm/error-injection.h | 13 ++ > > arch/x86/kernel/kprobes/core.c | 14 -- > > arch/x86/lib/Makefile |1 > > arch/x86/lib/error-inject.c| 19 +++ > > fs/btrfs/disk-io.c |2 > > fs/btrfs/free-space-cache.c|2 > > include/asm-generic/error-injection.h | 20 +++ > > include/asm-generic/vmlinux.lds.h | 14 +- > > include/linux/bpf.h| 12 -- > > include/linux/error-injection.h| 21 +++ > > include/linux/kprobes.h|1 > > include/linux/module.h |6 - > > kernel/kprobes.c | 163 > > kernel/module.c|8 + > > kernel/trace/Kconfig |2 > > kernel/trace/bpf_trace.c |2 > > kernel/trace/trace_kprobe.c|3 > > lib/Kconfig.debug |4 + > > lib/Makefile |1 > > lib/error-inject.c | 213 > > > > 22 files changed, 315 insertions(+), 210 deletions(-) > > create mode 100644 arch/x86/include/asm/error-injection.h > > create mode 100644 arch/x86/lib/error-inject.c > > create mode 100644 include/asm-generic/error-injection.h > > create mode 100644 include/linux/error-injection.h > > create mode 100644 lib/error-inject.c > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > index d3f4aaf9cb7a..97376accfb14 100644 > > --- a/arch/Kconfig > > +++ b/arch/Kconfig > > @@ -196,7 +196,7 @@ config HAVE_OPTPROBES > > config HAVE_KPROBES_ON_FTRACE > > bool > > > > -config HAVE_KPROBE_OVERRIDE > > +config HAVE_FUNCTION_ERROR_INJECTION > > bool > > > > config HAVE_NMI > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > > index 45dc6233f2b9..366b19cb79b7 100644 > > --- a/arch/x86/Kconfig > > +++ b/arch/x86/Kconfig > > @@ -154,7 +154,7 @@ config X86 > > select HAVE_KERNEL_XZ > > select HAVE_KPROBES > > select HAVE_KPROBES_ON_FTRACE > > - select HAVE_KPROBE_OVERRIDE > > + select HAVE_FUNCTION_ERROR_INJECTION > > select HAVE_KRETPROBES > > select HAVE_KVM > > select HAVE_LIVEPATCH if X86_64 > > diff --git a/arch/x86/include/asm/error-injection.h > > b/arch/x86/include/asm/error-injection.h > > new file mode 100644 > > index ..47b7a1296245 > > --- /dev/null > > +++ b/arch/x86/include/asm/error-injection.h > > @@ -0,0 +1,13 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef _ASM_ERROR_INJECTION_H > > +#define _ASM_ERROR_INJECTION_H > > + > > +#include > > +#include > > +#include > > +#include > > + > > +asmlinkage void just_return_func(void); > > +void override_function_with_return(struct pt_regs *regs); > > + > > +#endif /* _ASM_ERROR_INJECTION_H */ > > diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/k
[PATCH bpf-next v3 3/5] error-injection: Separate error-injection from kprobe
Since error-injection framework is not limited to be used
by kprobes, nor bpf. Other kernel subsystems can use it
freely for checking safeness of error-injection, e.g.
livepatch, ftrace etc.
So this separate error-injection framework from kprobes.
Some differences has been made:
- "kprobe" word is removed from any APIs/structures.
- BPF_ALLOW_ERROR_INJECTION() is renamed to
ALLOW_ERROR_INJECTION() since it is not limited for BPF too.
- CONFIG_FUNCTION_ERROR_INJECTION is the config item of this
feature. It is automatically enabled if the arch supports
error injection feature for kprobe or ftrace etc.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
---
Changes in v3:
- Fix a build error for asmlinkage on i386 by including compiler.h
- Fix "CONFIG_FUNCTION_ERROR_INJECT" typo.
- Separate CONFIG_MODULES dependent code
- Add CONFIG_KPROBES dependency for arch_deref_entry_point()
- Call error-injection init function in late_initcall stage.
- Fix read-side mutex lock
- Some cosmetic cleanups
---
arch/Kconfig |2
arch/x86/Kconfig |2
arch/x86/include/asm/error-injection.h | 13 ++
arch/x86/kernel/kprobes/core.c | 14 --
arch/x86/lib/Makefile |1
arch/x86/lib/error-inject.c| 19 +++
fs/btrfs/disk-io.c |2
fs/btrfs/free-space-cache.c|2
include/asm-generic/error-injection.h | 20 +++
include/asm-generic/vmlinux.lds.h | 14 +-
include/linux/bpf.h| 12 --
include/linux/error-injection.h| 21 +++
include/linux/kprobes.h|1
include/linux/module.h |6 -
kernel/kprobes.c | 163
kernel/module.c|8 +
kernel/trace/Kconfig |2
kernel/trace/bpf_trace.c |2
kernel/trace/trace_kprobe.c|3
lib/Kconfig.debug |4 +
lib/Makefile |1
lib/error-inject.c | 213
22 files changed, 315 insertions(+), 210 deletions(-)
create mode 100644 arch/x86/include/asm/error-injection.h
create mode 100644 arch/x86/lib/error-inject.c
create mode 100644 include/asm-generic/error-injection.h
create mode 100644 include/linux/error-injection.h
create mode 100644 lib/error-inject.c
diff --git a/arch/Kconfig b/arch/Kconfig
index d3f4aaf9cb7a..97376accfb14 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -196,7 +196,7 @@ config HAVE_OPTPROBES
config HAVE_KPROBES_ON_FTRACE
bool
-config HAVE_KPROBE_OVERRIDE
+config HAVE_FUNCTION_ERROR_INJECTION
bool
config HAVE_NMI
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 45dc6233f2b9..366b19cb79b7 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -154,7 +154,7 @@ config X86
select HAVE_KERNEL_XZ
select HAVE_KPROBES
select HAVE_KPROBES_ON_FTRACE
- select HAVE_KPROBE_OVERRIDE
+ select HAVE_FUNCTION_ERROR_INJECTION
select HAVE_KRETPROBES
select HAVE_KVM
select HAVE_LIVEPATCH if X86_64
diff --git a/arch/x86/include/asm/error-injection.h
b/arch/x86/include/asm/error-injection.h
new file mode 100644
index ..47b7a1296245
--- /dev/null
+++ b/arch/x86/include/asm/error-injection.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ERROR_INJECTION_H
+#define _ASM_ERROR_INJECTION_H
+
+#include
+#include
+#include
+#include
+
+asmlinkage void just_return_func(void);
+void override_function_with_return(struct pt_regs *regs);
+
+#endif /* _ASM_ERROR_INJECTION_H */
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index b02a377d5905..bd36f3c33cd0 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -1183,17 +1183,3 @@ int arch_trampoline_kprobe(struct kprobe *p)
{
return 0;
}
-
-asmlinkage void override_func(void);
-asm(
- ".type override_func, @function\n"
- "override_func:\n"
- " ret\n"
- ".size override_func, .-override_func\n"
-);
-
-void arch_kprobe_override_function(struct pt_regs *regs)
-{
- regs->ip = (unsigned long)_func;
-}
-NOKPROBE_SYMBOL(arch_kprobe_override_function);
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index 7b181b61170e..171377b83be1 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -26,6 +26,7 @@ lib-y += memcpy_$(BITS).o
lib-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o
lib-$(CONFIG_INSTRUCTION_DECODER) += insn.o inat.o insn-eval.o
lib-$(CONFIG_RANDOMIZE_BASE) += kaslr.o
+lib-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
obj-y += msr.o msr-reg.o msr-reg-export.o hweight.o
diff --git a/arch/x86/lib/error-inject.c b/arch/x86/lib/error-in
[PATCH bpf-next v3 4/5] error-injection: Add injectable error types
Add injectable error types for each error-injectable function.
One motivation of error injection test is to find software flaws,
mistakes or mis-handlings of expectable errors. If we find such
flaws by the test, that is a program bug, so we need to fix it.
But if the tester miss input the error (e.g. just return success
code without processing anything), it causes unexpected behavior
even if the caller is correctly programmed to handle any errors.
That is not what we want to test by error injection.
To clarify what type of errors the caller must expect for each
injectable function, this introduces injectable error types:
- EI_ETYPE_NULL : means the function will return NULL if it
fails. No ERR_PTR, just a NULL.
- EI_ETYPE_ERRNO : means the function will return -ERRNO
if it fails.
- EI_ETYPE_ERRNO_NULL : means the function will return -ERRNO
(ERR_PTR) or NULL.
ALLOW_ERROR_INJECTION() macro is expanded to get one of
NULL, ERRNO, ERRNO_NULL to record the error type for
each function. e.g.
ALLOW_ERROR_INJECTION(open_ctree, ERRNO)
This error types are shown in debugfs as below.
/ # cat /sys/kernel/debug/error_injection/list
open_ctree [btrfs]ERRNO
io_ctl_init [btrfs] ERRNO
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
---
fs/btrfs/disk-io.c|2 +-
fs/btrfs/free-space-cache.c |2 +-
include/asm-generic/error-injection.h | 23 +++---
include/asm-generic/vmlinux.lds.h |2 +-
include/linux/error-injection.h |6 +
include/linux/module.h|3 ++
lib/error-inject.c| 43 -
7 files changed, 66 insertions(+), 15 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 5c540129ad81..9269fc23f490 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3124,7 +3124,7 @@ int open_ctree(struct super_block *sb,
goto fail_block_groups;
goto retry_root_backup;
}
-ALLOW_ERROR_INJECTION(open_ctree);
+ALLOW_ERROR_INJECTION(open_ctree, ERRNO);
static void btrfs_end_buffer_write_sync(struct buffer_head *bh, int uptodate)
{
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 2a75e088b215..9eb45f77e381 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -333,7 +333,7 @@ static int io_ctl_init(struct btrfs_io_ctl *io_ctl, struct
inode *inode,
return 0;
}
-ALLOW_ERROR_INJECTION(io_ctl_init);
+ALLOW_ERROR_INJECTION(io_ctl_init, ERRNO);
static void io_ctl_free(struct btrfs_io_ctl *io_ctl)
{
diff --git a/include/asm-generic/error-injection.h
b/include/asm-generic/error-injection.h
index 08352c9d9f97..296c65442f00 100644
--- a/include/asm-generic/error-injection.h
+++ b/include/asm-generic/error-injection.h
@@ -3,17 +3,32 @@
#define _ASM_GENERIC_ERROR_INJECTION_H
#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
+enum {
+ EI_ETYPE_NONE, /* Dummy value for undefined case */
+ EI_ETYPE_NULL, /* Return NULL if failure */
+ EI_ETYPE_ERRNO, /* Return -ERRNO if failure */
+ EI_ETYPE_ERRNO_NULL,/* Return -ERRNO or NULL if failure */
+};
+
+struct error_injection_entry {
+ unsigned long addr;
+ int etype;
+};
+
#ifdef CONFIG_FUNCTION_ERROR_INJECTION
/*
* Whitelist ganerating macro. Specify functions which can be
* error-injectable using this macro.
*/
-#define ALLOW_ERROR_INJECTION(fname) \
-static unsigned long __used\
+#define ALLOW_ERROR_INJECTION(fname, _etype) \
+static struct error_injection_entry __used \
__attribute__((__section__("_error_injection_whitelist"))) \
- _eil_addr_##fname = (unsigned long)fname;
+ _eil_addr_##fname = { \
+ .addr = (unsigned long)fname, \
+ .etype = EI_ETYPE_##_etype, \
+ };
#else
-#define ALLOW_ERROR_INJECTION(fname)
+#define ALLOW_ERROR_INJECTION(fname, _etype)
#endif
#endif
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index f2068cca5206..ebe544e048cd 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -137,7 +137,7 @@
#endif
#ifdef CONFIG_FUNCTION_ERROR_INJECTION
-#define ERROR_INJECT_WHITELIST() . = ALIGN(8); \
+#define ERROR_INJECT_WHITELIST() STRUCT_ALIGN(); \
VMLINUX_SYMBOL(__start_error_injection_whitelist) = .;\
KEEP(*(_error_injection_whitelist)) \
VMLINUX_SYMBOL(__stop_error_injection_whitelist) = .;
diff --g
[PATCH bpf-next v3 5/5] error-injection: Support fault injection framework
Support in-kernel fault-injection framework via debugfs. This allows you to inject a conditional error to specified function using debugfs interfaces. Here is the result of test script described in Documentation/fault-injection/fault-injection.txt === # ./test_fail_function.sh 1+0 records in 1+0 records out 1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.0227404 s, 46.1 MB/s btrfs-progs v4.4 See http://btrfs.wiki.kernel.org for more information. Label: (null) UUID: bfa96010-12e9-4360-aed0-42eec7af5798 Node size: 16384 Sector size:4096 Filesystem size:1001.00MiB Block group profiles: Data: single8.00MiB Metadata: DUP 58.00MiB System: DUP 12.00MiB SSD detected: no Incompat features: extref, skinny-metadata Number of devices: 1 Devices: IDSIZE PATH 1 1001.00MiB /dev/loop2 mount: mount /dev/loop2 on /opt/tmpmnt failed: Cannot allocate memory SUCCESS! === Signed-off-by: Masami Hiramatsu <mhira...@kernel.org> --- Changes in v3: - Check and adjust error value for each target function - Clear kporbe flag for reuse - Add more documents and example --- Documentation/fault-injection/fault-injection.txt | 62 ++ kernel/Makefile |1 kernel/fail_function.c| 217 + lib/Kconfig.debug | 10 + 4 files changed, 290 insertions(+) create mode 100644 kernel/fail_function.c diff --git a/Documentation/fault-injection/fault-injection.txt b/Documentation/fault-injection/fault-injection.txt index 918972babcd8..4aecbceef9d2 100644 --- a/Documentation/fault-injection/fault-injection.txt +++ b/Documentation/fault-injection/fault-injection.txt @@ -30,6 +30,12 @@ o fail_mmc_request injects MMC data errors on devices permitted by setting debugfs entries under /sys/kernel/debug/mmc0/fail_mmc_request +o fail_function + + injects error return on specific functions, which are marked by + ALLOW_ERROR_INJECTION() macro, by setting debugfs entries + under /sys/kernel/debug/fail_function. No boot option supported. + Configure fault-injection capabilities behavior --- @@ -123,6 +129,24 @@ configuration of fault-injection capabilities. default is 'N', setting it to 'Y' will disable failure injections when dealing with private (address space) futexes. +- /sys/kernel/debug/fail_function/inject: + + specifies the target function of error injection by name. + +- /sys/kernel/debug/fail_function/retval: + + specifies the "error" return value to inject to the given + function. + +- /sys/kernel/debug/fail_function/injectable: + + (read only) shows error injectable functions and what type of + error values can be specified. The error type will be one of + below; + - NULL: retval must be 0. + - ERRNO: retval must be -1 to -MAX_ERRNO (-4096). + - ERR_NULL: retval must be 0 or -1 to -MAX_ERRNO (-4096). + o Boot option In order to inject faults while debugfs is not available (early boot time), @@ -268,6 +292,44 @@ trap "echo 0 > /sys/kernel/debug/$FAILTYPE/probability" SIGINT SIGTERM EXIT echo "Injecting errors into the module $module... (interrupt to stop)" sleep 100 +-- + +o Inject open_ctree error while btrfs mount + +#!/bin/bash + +rm -f testfile.img +dd if=/dev/zero of=testfile.img bs=1M seek=1000 count=1 +DEVICE=$(losetup --show -f testfile.img) +mkfs.btrfs -f $DEVICE +mkdir -p tmpmnt + +FAILTYPE=fail_function +echo open_ctree > /sys/kernel/debug/$FAILTYPE/inject +echo -12 > /sys/kernel/debug/$FAILTYPE/retval +echo N > /sys/kernel/debug/$FAILTYPE/task-filter +echo 100 > /sys/kernel/debug/$FAILTYPE/probability +echo 0 > /sys/kernel/debug/$FAILTYPE/interval +echo -1 > /sys/kernel/debug/$FAILTYPE/times +echo 0 > /sys/kernel/debug/$FAILTYPE/space +echo 1 > /sys/kernel/debug/$FAILTYPE/verbose + +mount -t btrfs $DEVICE tmpmnt +if [ $? -ne 0 ] +then + echo "SUCCESS!" +else + echo "FAILED!" + umount tmpmnt +fi + +echo > /sys/kernel/debug/$FAILTYPE/inject + +rmdir tmpmnt +losetup -d $DEVICE +rm testfile.img + + Tool to run command with failslab or fail_page_alloc In order to make it easier to accomplish the tasks mentioned above, we can use diff --git a/kernel/Makefile b/kernel/Makefile index 172d151d429c..f85ae5dfa474 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -81,6 +81,7 @@ obj-$(CONFIG_AUDIT_TREE) += audit_tree.o obj-$(CONFIG_GCOV_KERNEL) += gcov/ obj-$(CONFIG_KCOV) += kcov.o obj-$(CONFIG_KPROBES) += kprobe
[PATCH bpf-next v3 1/5] tracing/kprobe: bpf: Check error injectable event is on function entry
Check whether error injectable event is on function entry or not.
Currently it checks the event is ftrace-based kprobes or not,
but that is wrong. It should check if the event is on the entry
of target function. Since error injection will override a function
to just return with modified return value, that operation must
be done before the target function starts making stackframe.
As a side effect, bpf error injection is no need to depend on
function-tracer. It can work with sw-breakpoint based kprobe
events too.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
---
Changes in v3:
- Move arch_ftrace_kprobe_override_function() to
core.c because it is no longer depending on ftrace.
- Fix a bug to skip passing kprobes target name to
kprobe_on_func_entry(). Passing both @addr and @symbol
to that function will result in failure.
---
arch/x86/include/asm/kprobes.h |4 +---
arch/x86/kernel/kprobes/core.c | 14 ++
arch/x86/kernel/kprobes/ftrace.c | 14 --
kernel/trace/Kconfig |2 --
kernel/trace/bpf_trace.c |8
kernel/trace/trace_kprobe.c |9 ++---
kernel/trace/trace_probe.h | 12 ++--
7 files changed, 31 insertions(+), 32 deletions(-)
diff --git a/arch/x86/include/asm/kprobes.h b/arch/x86/include/asm/kprobes.h
index 36abb23a7a35..367d99cff426 100644
--- a/arch/x86/include/asm/kprobes.h
+++ b/arch/x86/include/asm/kprobes.h
@@ -67,9 +67,7 @@ extern const int kretprobe_blacklist_size;
void arch_remove_kprobe(struct kprobe *p);
asmlinkage void kretprobe_trampoline(void);
-#ifdef CONFIG_KPROBES_ON_FTRACE
-extern void arch_ftrace_kprobe_override_function(struct pt_regs *regs);
-#endif
+extern void arch_kprobe_override_function(struct pt_regs *regs);
/* Architecture specific copy of original instruction*/
struct arch_specific_insn {
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index bd36f3c33cd0..b02a377d5905 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -1183,3 +1183,17 @@ int arch_trampoline_kprobe(struct kprobe *p)
{
return 0;
}
+
+asmlinkage void override_func(void);
+asm(
+ ".type override_func, @function\n"
+ "override_func:\n"
+ " ret\n"
+ ".size override_func, .-override_func\n"
+);
+
+void arch_kprobe_override_function(struct pt_regs *regs)
+{
+ regs->ip = (unsigned long)_func;
+}
+NOKPROBE_SYMBOL(arch_kprobe_override_function);
diff --git a/arch/x86/kernel/kprobes/ftrace.c b/arch/x86/kernel/kprobes/ftrace.c
index 1ea748d682fd..8dc0161cec8f 100644
--- a/arch/x86/kernel/kprobes/ftrace.c
+++ b/arch/x86/kernel/kprobes/ftrace.c
@@ -97,17 +97,3 @@ int arch_prepare_kprobe_ftrace(struct kprobe *p)
p->ainsn.boostable = false;
return 0;
}
-
-asmlinkage void override_func(void);
-asm(
- ".type override_func, @function\n"
- "override_func:\n"
- " ret\n"
- ".size override_func, .-override_func\n"
-);
-
-void arch_ftrace_kprobe_override_function(struct pt_regs *regs)
-{
- regs->ip = (unsigned long)_func;
-}
-NOKPROBE_SYMBOL(arch_ftrace_kprobe_override_function);
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index ae3a2d519e50..6400e1bf97c5 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -533,9 +533,7 @@ config FUNCTION_PROFILER
config BPF_KPROBE_OVERRIDE
bool "Enable BPF programs to override a kprobed function"
depends on BPF_EVENTS
- depends on KPROBES_ON_FTRACE
depends on HAVE_KPROBE_OVERRIDE
- depends on DYNAMIC_FTRACE_WITH_REGS
default n
help
Allows BPF to override the execution of a probed function and
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index f6d2327ecb59..1966ad3bf3e0 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -85,7 +85,7 @@ BPF_CALL_2(bpf_override_return, struct pt_regs *, regs,
unsigned long, rc)
{
__this_cpu_write(bpf_kprobe_override, 1);
regs_set_return_value(regs, rc);
- arch_ftrace_kprobe_override_function(regs);
+ arch_kprobe_override_function(regs);
return 0;
}
@@ -800,11 +800,11 @@ int perf_event_attach_bpf_prog(struct perf_event *event,
int ret = -EEXIST;
/*
-* Kprobe override only works for ftrace based kprobes, and only if they
-* are on the opt-in list.
+* Kprobe override only works if they are on the function entry,
+* and only if they are on the opt-in list.
*/
if (prog->kprobe_override &&
- (!trace_kprobe_ftrace(event->tp_event) ||
+ (!trace_kprobe_on_func_entry(event->tp_event) ||
!trace_kprobe_error_injectable(event->tp_event)))
return -EINVAL;
diff --git a/kernel/trace
[PATCH bpf-next v3 0/5] Separate error injection table from kprobes
Hi, Here are the 3rd version of patches to moving error injection table from kprobes. This series includes some fixes and error type descriptions which we discussed in the previous thread. Here is the previous version: https://lkml.org/lkml/2017/12/26/26 There are 2 main reasons why I separate it from kprobes. - kprobes users can modify execution path not only at error-injection whitelist functions but also other functions. - This error injection information is also useful for ftrace (function-hook) and livepatch. It should not be limited by CONFIG_KPROBES. So I introduced CONFIG_FUNCTION_ERROR_INJECTION for this feature. Unfortunately currently CONFIG_FUNCTION_ERROR_INJECTION depends on CONFIG_KPROBES because of arch_deref_entry_point(), which should be provided by asm/types.h as ppc64 and ia64 do or kallsyms subsystem. Anyway, since that is another story, I will make another series to fix it. For [1/5], I tested it by test_override_return.sh on the kernel with CONFIG_DYNAMIC_FTRACE=n, and succeeded as below. (actually I'v found some bugs and fixed, thanks Alexei!) == # LANG=C ./test_override_return.sh 1+0 records in 1+0 records out 1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.00200037 s, 524 MB/s btrfs-progs v4.9.1 See http://btrfs.wiki.kernel.org for more information. Performing full device TRIM /dev/loop1 (1001.00MiB) ... Label: (null) UUID: 02635ea7-f6d1-4ee8-a677-4354a38f6930 Node size: 16384 Sector size:4096 Filesystem size:1001.00MiB Block group profiles: Data: single8.00MiB Metadata: DUP 50.00MiB System: DUP 8.00MiB SSD detected: no Incompat features: extref, skinny-metadata Number of devices: 1 Devices: IDSIZE PATH 1 1001.00MiB /dev/loop1 mount: /opt/samples/bpf/tmpmnt: mount(2) system call failed: Cannot allocate memory. SUCCESS! == I've removed RFC from this series, but actually, [4/5] and [5/5] will be need more discussion. So please feel free to merge [1/5] to [3/5] separately. [4/5] introduces error return types for describing what error values the function callers must expect. For the purpose of error injection test is to find out hidden problems which caller doesn't handle errors correctly. So if the caller is programmed correctly, even if we inject an error, it continues to work as expected. However, if we inject a succeed return value even though the callee doesn't do anything, the caller does wrong processing (and end up with kernel panic, or even worse data corruption). That is not what we want to do with error injection. [5/5] introduces function-based error injection interface via debugfs, which can cause "success injection", So I coupled it with [4/5] to ensure the expected error types for the target function. Changes in v3: - [1/5] Move arch_ftrace_kprobe_override_function() to core.c to remove CONFIG_KPROBE_ON_FTRACE dependency, and fix a bug in trace_kprobe_on_func_entry(). - [3/5] Fix a build error and typos, separate CONFIG_MODULES dependent code, add CONFIG_KPROBES dependency, and call error-injection init function in late_initcall stage. - [4/5] Newly added - [5/5] Check and adjust error value for each target function and add more documents and example. Thank you, --- Masami Hiramatsu (5): tracing/kprobe: bpf: Check error injectable event is on function entry tracing/kprobe: bpf: Compare instruction pointer with original one error-injection: Separate error-injection from kprobe error-injection: Add injectable error types error-injection: Support fault injection framework Documentation/fault-injection/fault-injection.txt | 62 + arch/Kconfig |2 arch/x86/Kconfig |2 arch/x86/include/asm/error-injection.h| 13 + arch/x86/include/asm/kprobes.h|4 arch/x86/kernel/kprobes/ftrace.c | 14 - arch/x86/lib/Makefile |1 arch/x86/lib/error-inject.c | 19 ++ fs/btrfs/disk-io.c|2 fs/btrfs/free-space-cache.c |2 include/asm-generic/error-injection.h | 35 +++ include/asm-generic/vmlinux.lds.h | 14 + include/linux/bpf.h | 12 - include/linux/error-injection.h | 27 ++ include/linux/kprobes.h |1 include/linux/module.h|7 - kernel/Makefile |1 kernel/fail_function.c| 217 +++ kernel/kprobes.c | 163 -- kerne
Re: [RFC PATCH bpf-next v2 0/4] Separate error injection table from kprobes
On Thu, 4 Jan 2018 11:07:16 -0500
Josef Bacik <jo...@toxicpanda.com> wrote:
> On Tue, Dec 26, 2017 at 04:46:28PM +0900, Masami Hiramatsu wrote:
> > Hi Josef and Alexei,
> >
> > Here are the 2nd version of patches to moving error injection
> > table from kprobes. In this series I did a small fixes and
> > add function-based fault injection.
> >
> > Here is the previous version:
> >
> > https://lkml.org/lkml/2017/12/22/554
> >
> > There are 2 main reasons why I separate it from kprobes.
> >
> > - kprobes users can modify execution path not only at
> >error-injection whitelist functions but also other
> >functions. I don't like to suggest user that such
> >limitation is from kprobes itself.
> >
> > - This error injection information is also useful for
> >ftrace (function-hook) and livepatch. It should not
> >be limited by CONFIG_KPROBES.
> >
> > So I introduced CONFIG_FUNCTION_ERROR_INJECTION for this feature.
> > Also CONFIG_FAIL_FUNCTION is added, which provides function-based
> > error injection interface via debugfs following fault-injection
> > framework. See [4/4].
> >
> > Any thoughts?
>
> Sorry Masami, I've been on vacation for the last two weeks. This approach is
> fine by me, if we want to allow other mechanisms other than bpf to use this
> functionality then hooray. I'll do a proper review when you post v3, just
> wanted to let you know I wasn't ignoring you. Thanks,
Yeah, thank you for the kindful notice ;)
BTW, could you tell me how I can run your test case?
When I tried to build the tests (samples/bpf) I got below error and stopped.
[mhiramat@devbox bpf]$ LANG=C make
make -C ../../ /home/mhiramat/ksrc/linux/samples/bpf/
make[1]: Entering directory '/home/mhiramat/ksrc/linux'
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
CHK include/generated/bounds.h
CHK include/generated/timeconst.h
CHK include/generated/asm-offsets.h
CALLscripts/checksyscalls.sh
DESCEND objtool
CHK scripts/mod/devicetable-offsets.h
HOSTCC /home/mhiramat/ksrc/linux/samples/bpf/test_lru_dist.o
/home/mhiramat/ksrc/linux/samples/bpf/test_lru_dist.c:39:8: error: redefinition
of 'struct list_head'
struct list_head {
^
In file included from /home/mhiramat/ksrc/linux/samples/bpf/test_lru_dist.c:9:0:
./tools/include/linux/types.h:69:8: note: originally defined here
struct list_head {
^
make[2]: *** [scripts/Makefile.host:107:
/home/mhiramat/ksrc/linux/samples/bpf/test_lru_dist.o] Error 1
make[1]: *** [Makefile:1675: /home/mhiramat/ksrc/linux/samples/bpf/] Error 2
make[1]: Leaving directory '/home/mhiramat/ksrc/linux'
make: *** [Makefile:204: all] Error 2
Thank you,
--
Masami Hiramatsu <mhira...@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH bpf-next v2 1/4] tracing/kprobe: bpf: Check error injectable event is on function entry
On Sun, 7 Jan 2018 19:01:57 -0800 Alexei Starovoitov <a...@fb.com> wrote: > On 12/29/17 12:20 AM, Masami Hiramatsu wrote: > >> Please run Josef's test in the !ftrace setup. > > Yes, I'll add the result of the test case. > > if Josef's test is passing in !ftrace config, > please resend your patches. > I think 2 and 3 were nice simplifications. > and patch 1 is good too if it's passes the test. > Would be great to get them in for this merge window. OK, I'll try to do that. Thanks! > > Thanks! > -- Masami Hiramatsu <mhira...@kernel.org> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH bpf-next v2 1/4] tracing/kprobe: bpf: Check error injectable event is on function entry
On Thu, 28 Dec 2017 17:03:24 -0800 Alexei Starovoitov <a...@fb.com> wrote: > On 12/28/17 12:20 AM, Masami Hiramatsu wrote: > > On Wed, 27 Dec 2017 20:32:07 -0800 > > Alexei Starovoitov <a...@fb.com> wrote: > > > >> On 12/27/17 8:16 PM, Steven Rostedt wrote: > >>> On Wed, 27 Dec 2017 19:45:42 -0800 > >>> Alexei Starovoitov <a...@fb.com> wrote: > >>> > >>>> I don't think that's the case. My reading of current > >>>> trace_kprobe_ftrace() -> arch_check_ftrace_location() > >>>> is that it will not be true for old mcount case. > >>> > >>> In the old mcount case, you can't use ftrace to return without calling > >>> the function. That is, no modification of the return ip, unless you > >>> created a trampoline that could handle arbitrary stack frames, and > >>> remove them from the stack before returning back to the function. > >> > >> correct. I was saying that trace_kprobe_ftrace() won't let us do > >> bpf_override_return with old mcount. > > > > No, trace_kprobe_ftrace() just checks the given address will be > > managed by ftrace. you can see arch_check_ftrace_location() in > > kernel/kprobes.c. > > > > FYI, CONFIG_KPROBES_ON_FTRACE depends on DYNAMIC_FTRACE_WITH_REGS, and > > DYNAMIC_FTRACE_WITH_REGS doesn't depend on CC_USING_FENTRY. > > This means if you compile kernel with old gcc and enable DYNAMIC_FTRACE, > > kprobes uses ftrace on mcount address which is NOT the entry point > > of target function. > > ok. fair enough. I think we can gate the feature to !mcount only. > > > On the other hand, changing IP feature has been implemented originaly > > by kprobes with int3 (sw breakpoint). This means you can use kprobes > > at correct address (the entry address of the function) you can hijack > > the function, as jprobe did. > > > >>>> As far as the rest of your arguments it very much puzzles me that > >>>> you claim that this patch suppose to work based on historical > >>>> reasoning whereas you did NOT test it. > >>> > >>> I believe that Masami is saying that the modification of the IP from > >>> kprobes has been very well tested. But I'm guessing that you still want > >>> a test case for using kprobes in this particular instance. It's not the > >>> implementation of modifying the IP that you are worried about, but the > >>> implementation of BPF using it in this case. Right? > >> > >> exactly. No doubt that old code works. > >> But it doesn't mean that bpf_override_return() will continue to > >> work in kprobes that are not ftrace based. > >> I suspect Josef's existing test case will cover this situation. > >> Probably only special .config is needed to disable ftrace, so > >> "kprobe on entry but not ftrace" check will kick in. > > > > Right. If you need to test it, you can run Josef's test case without > > CONFIG_DYNAMIC_FTRACE. > > It should be obvious that the person who submits the patch > must run the tests. > > >> But I didn't get an impression that this situation was tested. > >> Instead I see only logical reasoning that it's _supposed_ to work. > >> That's not enough. > > > > OK, so would you just ask me to run samples/bpf ? > > Please run Josef's test in the !ftrace setup. Yes, I'll add the result of the test case. Thank you, -- Masami Hiramatsu <mhira...@kernel.org> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH bpf-next v2 4/4] error-injection: Support fault injection framework
On Thu, 28 Dec 2017 17:11:31 -0800
Alexei Starovoitov <a...@fb.com> wrote:
> On 12/27/17 11:51 PM, Masami Hiramatsu wrote:
> >
> > Then what happen if the user set invalid retval to those functions?
> > even if we limit the injectable functions, it can cause a problem,
> >
> > for example,
> >
> > obj = func_return_object();
> > if (!obj) {
> > handling_error...;
> > }
> > obj->field = x;
> >
> > In this case, obviously func_return_object() must return NULL if there is
> > an error, not -ENOMEM. But without the correct retval information, how would
> > you check the BPF code doesn't cause a trouble?
> > Currently it seems you are expecting only the functions which return error
> > code.
> >
> > ret = func_return_state();
> > if (ret < 0) {
> > handling_error...;
> > }
> >
> > But how we can distinguish those?
> >
> > If we have the error range for each function, we can ensure what is
> > *correct* error code, NULL or errno, or any other error numbers. :)
>
> messing up return values may cause problems and range check is
> not going to magically help.
> The caller may handle only a certain set of errors or interpret
> some of them like EBUSY as a signal to retry.
> It's plain impossible to make sure that kernel will be functional
> after error injection has been made.
Hmm, if so, why we need this injectable table?
If we can not make sure the safeness of the error injection (of course, yes)
why we need to limit the error injection on such limited functions?
I think we don't need it anymore. Any function can be injectable, and no
need to make sure the safeness.
Thank you,
> Like kmalloc() unconditionally returning NULL will be deadly
> for the kernel, hence this patch 4/4 has very limited practical
> use. The bpf program need to make intelligent decisions when
> to return an error and what kind of error to return.
> Doing blank range check adds a false sense of additional safety.
> More so it wastes kilobytes of memory to do this check, hence nack.
>
--
Masami Hiramatsu <mhira...@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH bpf-next v2 1/4] tracing/kprobe: bpf: Check error injectable event is on function entry
On Wed, 27 Dec 2017 20:32:07 -0800 Alexei Starovoitov <a...@fb.com> wrote: > On 12/27/17 8:16 PM, Steven Rostedt wrote: > > On Wed, 27 Dec 2017 19:45:42 -0800 > > Alexei Starovoitov <a...@fb.com> wrote: > > > >> I don't think that's the case. My reading of current > >> trace_kprobe_ftrace() -> arch_check_ftrace_location() > >> is that it will not be true for old mcount case. > > > > In the old mcount case, you can't use ftrace to return without calling > > the function. That is, no modification of the return ip, unless you > > created a trampoline that could handle arbitrary stack frames, and > > remove them from the stack before returning back to the function. > > correct. I was saying that trace_kprobe_ftrace() won't let us do > bpf_override_return with old mcount. No, trace_kprobe_ftrace() just checks the given address will be managed by ftrace. you can see arch_check_ftrace_location() in kernel/kprobes.c. FYI, CONFIG_KPROBES_ON_FTRACE depends on DYNAMIC_FTRACE_WITH_REGS, and DYNAMIC_FTRACE_WITH_REGS doesn't depend on CC_USING_FENTRY. This means if you compile kernel with old gcc and enable DYNAMIC_FTRACE, kprobes uses ftrace on mcount address which is NOT the entry point of target function. On the other hand, changing IP feature has been implemented originaly by kprobes with int3 (sw breakpoint). This means you can use kprobes at correct address (the entry address of the function) you can hijack the function, as jprobe did. > >> As far as the rest of your arguments it very much puzzles me that > >> you claim that this patch suppose to work based on historical > >> reasoning whereas you did NOT test it. > > > > I believe that Masami is saying that the modification of the IP from > > kprobes has been very well tested. But I'm guessing that you still want > > a test case for using kprobes in this particular instance. It's not the > > implementation of modifying the IP that you are worried about, but the > > implementation of BPF using it in this case. Right? > > exactly. No doubt that old code works. > But it doesn't mean that bpf_override_return() will continue to > work in kprobes that are not ftrace based. > I suspect Josef's existing test case will cover this situation. > Probably only special .config is needed to disable ftrace, so > "kprobe on entry but not ftrace" check will kick in. Right. If you need to test it, you can run Josef's test case without CONFIG_DYNAMIC_FTRACE. > But I didn't get an impression that this situation was tested. > Instead I see only logical reasoning that it's _supposed_ to work. > That's not enough. OK, so would you just ask me to run samples/bpf ? Thanks, -- Masami Hiramatsu <mhira...@kernel.org> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH bpf-next v2 1/4] tracing/kprobe: bpf: Check error injectable event is on function entry
On Wed, 27 Dec 2017 19:45:42 -0800
Alexei Starovoitov <a...@fb.com> wrote:
> On 12/27/17 6:34 PM, Masami Hiramatsu wrote:
> > On Wed, 27 Dec 2017 14:46:24 -0800
> > Alexei Starovoitov <a...@fb.com> wrote:
> >
> >> On 12/26/17 9:56 PM, Masami Hiramatsu wrote:
> >>> On Tue, 26 Dec 2017 17:57:32 -0800
> >>> Alexei Starovoitov <alexei.starovoi...@gmail.com> wrote:
> >>>
> >>>> On Tue, Dec 26, 2017 at 04:46:59PM +0900, Masami Hiramatsu wrote:
> >>>>> Check whether error injectable event is on function entry or not.
> >>>>> Currently it checks the event is ftrace-based kprobes or not,
> >>>>> but that is wrong. It should check if the event is on the entry
> >>>>> of target function. Since error injection will override a function
> >>>>> to just return with modified return value, that operation must
> >>>>> be done before the target function starts making stackframe.
> >>>>>
> >>>>> As a side effect, bpf error injection is no need to depend on
> >>>>> function-tracer. It can work with sw-breakpoint based kprobe
> >>>>> events too.
> >>>>>
> >>>>> Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
> >>>>> ---
> >>>>> kernel/trace/Kconfig|2 --
> >>>>> kernel/trace/bpf_trace.c|6 +++---
> >>>>> kernel/trace/trace_kprobe.c |8 +---
> >>>>> kernel/trace/trace_probe.h | 12 ++--
> >>>>> 4 files changed, 14 insertions(+), 14 deletions(-)
> >>>>>
> >>>>> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> >>>>> index ae3a2d519e50..6400e1bf97c5 100644
> >>>>> --- a/kernel/trace/Kconfig
> >>>>> +++ b/kernel/trace/Kconfig
> >>>>> @@ -533,9 +533,7 @@ config FUNCTION_PROFILER
> >>>>> config BPF_KPROBE_OVERRIDE
> >>>>> bool "Enable BPF programs to override a kprobed function"
> >>>>> depends on BPF_EVENTS
> >>>>> - depends on KPROBES_ON_FTRACE
> >>>>> depends on HAVE_KPROBE_OVERRIDE
> >>>>> - depends on DYNAMIC_FTRACE_WITH_REGS
> >>>>> default n
> >>>>> help
> >>>>> Allows BPF to override the execution of a probed function and
> >>>>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> >>>>> index f6d2327ecb59..d663660f8392 100644
> >>>>> --- a/kernel/trace/bpf_trace.c
> >>>>> +++ b/kernel/trace/bpf_trace.c
> >>>>> @@ -800,11 +800,11 @@ int perf_event_attach_bpf_prog(struct perf_event
> >>>>> *event,
> >>>>> int ret = -EEXIST;
> >>>>>
> >>>>> /*
> >>>>> -* Kprobe override only works for ftrace based kprobes, and
> >>>>> only if they
> >>>>> -* are on the opt-in list.
> >>>>> +* Kprobe override only works if they are on the function entry,
> >>>>> +* and only if they are on the opt-in list.
> >>>>> */
> >>>>> if (prog->kprobe_override &&
> >>>>> - (!trace_kprobe_ftrace(event->tp_event) ||
> >>>>> + (!trace_kprobe_on_func_entry(event->tp_event) ||
> >>>>> !trace_kprobe_error_injectable(event->tp_event)))
> >>>>> return -EINVAL;
> >>>>>
> >>>>> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> >>>>> index 91f4b57dab82..265e3e27e8dc 100644
> >>>>> --- a/kernel/trace/trace_kprobe.c
> >>>>> +++ b/kernel/trace/trace_kprobe.c
> >>>>> @@ -88,13 +88,15 @@ static nokprobe_inline unsigned long
> >>>>> trace_kprobe_nhit(struct trace_kprobe *tk)
> >>>>> return nhit;
> >>>>> }
> >>>>>
> >>>>> -int trace_kprobe_ftrace(struct trace_event_call *call)
> >>>>> +bool trace_kprobe_on_func_entry(struct trace_event_call *call)
> >>>>> {
> >>>>> struct trace_kprobe *tk = (struct trace_kprobe *)call->data;
> >>>>> - re
Re: [RFC PATCH bpf-next v2 4/4] error-injection: Support fault injection framework
On Wed, 27 Dec 2017 19:49:28 -0800
Alexei Starovoitov <a...@fb.com> wrote:
> On 12/27/17 5:38 PM, Masami Hiramatsu wrote:
> > On Wed, 27 Dec 2017 14:49:46 -0800
> > Alexei Starovoitov <a...@fb.com> wrote:
> >
> >> On 12/27/17 12:09 AM, Masami Hiramatsu wrote:
> >>> On Tue, 26 Dec 2017 18:12:56 -0800
> >>> Alexei Starovoitov <alexei.starovoi...@gmail.com> wrote:
> >>>
> >>>> On Tue, Dec 26, 2017 at 04:48:25PM +0900, Masami Hiramatsu wrote:
> >>>>> Support in-kernel fault-injection framework via debugfs.
> >>>>> This allows you to inject a conditional error to specified
> >>>>> function using debugfs interfaces.
> >>>>>
> >>>>> Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
> >>>>> ---
> >>>>> Documentation/fault-injection/fault-injection.txt |5 +
> >>>>> kernel/Makefile |1
> >>>>> kernel/fail_function.c| 169
> >>>>> +
> >>>>> lib/Kconfig.debug | 10 +
> >>>>> 4 files changed, 185 insertions(+)
> >>>>> create mode 100644 kernel/fail_function.c
> >>>>>
> >>>>> diff --git a/Documentation/fault-injection/fault-injection.txt
> >>>>> b/Documentation/fault-injection/fault-injection.txt
> >>>>> index 918972babcd8..6243a588dd71 100644
> >>>>> --- a/Documentation/fault-injection/fault-injection.txt
> >>>>> +++ b/Documentation/fault-injection/fault-injection.txt
> >>>>> @@ -30,6 +30,11 @@ o fail_mmc_request
> >>>>>injects MMC data errors on devices permitted by setting
> >>>>>debugfs entries under /sys/kernel/debug/mmc0/fail_mmc_request
> >>>>>
> >>>>> +o fail_function
> >>>>> +
> >>>>> + injects error return on specific functions by setting debugfs entries
> >>>>> + under /sys/kernel/debug/fail_function. No boot option supported.
> >>>>
> >>>> I like it.
> >>>> Could you document it a bit better?
> >>>
> >>> Yes, I will do in next series.
> >>>
> >>>> In particular retval is configurable, but without an example no one
> >>>> will be able to figure out how to use it.
> >>>
> >>> Ah, right. BTW, as I pointed in the covermail, should we store the
> >>> expected error value range into the injectable list? e.g.
> >>>
> >>> ALLOW_ERROR_INJECTION(open_ctree, -1, -MAX_ERRNO)
> >>>
> >>> And provide APIs to check/get it.
> >>
> >> I'm afraid such check would be too costly.
> >> Right now we have only two functions marked but I expect hundreds more
> >> will be added in the near future as soon as developers realize the
> >> potential of such error injection.
> >> All of ALLOW_ERROR_INJECTION marks add 8 byte overhead each to .data.
> >> Multiple by 1k and we have 8k of data spent on marks.
> >> If we add max/min range marks that doubles it for very little use.
> >> I think marking function only is enough.
> >
> > Sorry, I don't think so.
> > Even if it takes 16 bytes more for each points, I don't think it is
> > any overhead for machines in these days. Even if so, we can provide
> > a kconfig to reduce it.
> > I mean, we are living in GB-order memory are, and it will be bigger
> > in the future. Why we have to worry about hundreds of 16bytes memory
> > pieces? It will take a few KB, and even if we mark thousands of
> > functions, it never reaches 1MB, in GB memory pool. :)
> >
> > Of course, for many small-footprint embedded devices (like having
> > less than 128MB memory), this feature can be a overhead. But they
> > can cut off the table by kconfig.
>
> I still disagree on wasting 16-byte * num_of_funcs of .data here.
> The trade-off of usability vs memory just not worth it. Sorry.
> Please focus on testing your changes instead.
Then what happen if the user set invalid retval to those functions?
even if we limit the injectable functions, it can cause a problem,
for example,
obj = func_return_object();
if (!obj) {
handling_error...;
}
obj->field = x;
In this case, obviously func_return_object() must return NULL if there is
an error, not -ENOMEM. But without the correct retval information, how would
you check the BPF code doesn't cause a trouble?
Currently it seems you are expecting only the functions which return error code.
ret = func_return_state();
if (ret < 0) {
handling_error...;
}
But how we can distinguish those?
If we have the error range for each function, we can ensure what is
*correct* error code, NULL or errno, or any other error numbers. :)
At least fail_function needs this feature because it can check
return value when setting it up.
Thank you,
--
Masami Hiramatsu <mhira...@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH bpf-next v2 1/4] tracing/kprobe: bpf: Check error injectable event is on function entry
On Wed, 27 Dec 2017 14:46:24 -0800
Alexei Starovoitov <a...@fb.com> wrote:
> On 12/26/17 9:56 PM, Masami Hiramatsu wrote:
> > On Tue, 26 Dec 2017 17:57:32 -0800
> > Alexei Starovoitov <alexei.starovoi...@gmail.com> wrote:
> >
> >> On Tue, Dec 26, 2017 at 04:46:59PM +0900, Masami Hiramatsu wrote:
> >>> Check whether error injectable event is on function entry or not.
> >>> Currently it checks the event is ftrace-based kprobes or not,
> >>> but that is wrong. It should check if the event is on the entry
> >>> of target function. Since error injection will override a function
> >>> to just return with modified return value, that operation must
> >>> be done before the target function starts making stackframe.
> >>>
> >>> As a side effect, bpf error injection is no need to depend on
> >>> function-tracer. It can work with sw-breakpoint based kprobe
> >>> events too.
> >>>
> >>> Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
> >>> ---
> >>> kernel/trace/Kconfig|2 --
> >>> kernel/trace/bpf_trace.c|6 +++---
> >>> kernel/trace/trace_kprobe.c |8 +---
> >>> kernel/trace/trace_probe.h | 12 ++--
> >>> 4 files changed, 14 insertions(+), 14 deletions(-)
> >>>
> >>> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> >>> index ae3a2d519e50..6400e1bf97c5 100644
> >>> --- a/kernel/trace/Kconfig
> >>> +++ b/kernel/trace/Kconfig
> >>> @@ -533,9 +533,7 @@ config FUNCTION_PROFILER
> >>> config BPF_KPROBE_OVERRIDE
> >>> bool "Enable BPF programs to override a kprobed function"
> >>> depends on BPF_EVENTS
> >>> - depends on KPROBES_ON_FTRACE
> >>> depends on HAVE_KPROBE_OVERRIDE
> >>> - depends on DYNAMIC_FTRACE_WITH_REGS
> >>> default n
> >>> help
> >>>Allows BPF to override the execution of a probed function and
> >>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> >>> index f6d2327ecb59..d663660f8392 100644
> >>> --- a/kernel/trace/bpf_trace.c
> >>> +++ b/kernel/trace/bpf_trace.c
> >>> @@ -800,11 +800,11 @@ int perf_event_attach_bpf_prog(struct perf_event
> >>> *event,
> >>> int ret = -EEXIST;
> >>>
> >>> /*
> >>> - * Kprobe override only works for ftrace based kprobes, and only if they
> >>> - * are on the opt-in list.
> >>> + * Kprobe override only works if they are on the function entry,
> >>> + * and only if they are on the opt-in list.
> >>>*/
> >>> if (prog->kprobe_override &&
> >>> - (!trace_kprobe_ftrace(event->tp_event) ||
> >>> + (!trace_kprobe_on_func_entry(event->tp_event) ||
> >>>!trace_kprobe_error_injectable(event->tp_event)))
> >>> return -EINVAL;
> >>>
> >>> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> >>> index 91f4b57dab82..265e3e27e8dc 100644
> >>> --- a/kernel/trace/trace_kprobe.c
> >>> +++ b/kernel/trace/trace_kprobe.c
> >>> @@ -88,13 +88,15 @@ static nokprobe_inline unsigned long
> >>> trace_kprobe_nhit(struct trace_kprobe *tk)
> >>> return nhit;
> >>> }
> >>>
> >>> -int trace_kprobe_ftrace(struct trace_event_call *call)
> >>> +bool trace_kprobe_on_func_entry(struct trace_event_call *call)
> >>> {
> >>> struct trace_kprobe *tk = (struct trace_kprobe *)call->data;
> >>> - return kprobe_ftrace(>rp.kp);
> >>> +
> >>> + return kprobe_on_func_entry(tk->rp.kp.addr, tk->rp.kp.symbol_name,
> >>> + tk->rp.kp.offset);
> >>
> >> That would be nice, but did you test this?
> >
> > Yes, because the jprobe, which was only official user of modifying execution
> > path using kprobe, did same way to check. (and kretprobe also does it)
> >
> >> My understanding that kprobe will restore all regs and
> >> here we need to override return ip _and_ value.
> >
> > yes, no problem. kprobe restore all regs from pt_regs, including regs->ip.
> >
> >> Could you add a patch with the test the way Josef did
> >> or describe the steps to test this new mode?
> >
> > Would you mean below patch? If so, it should w
Re: [RFC PATCH bpf-next v2 4/4] error-injection: Support fault injection framework
On Wed, 27 Dec 2017 14:49:46 -0800 Alexei Starovoitov <a...@fb.com> wrote: > On 12/27/17 12:09 AM, Masami Hiramatsu wrote: > > On Tue, 26 Dec 2017 18:12:56 -0800 > > Alexei Starovoitov <alexei.starovoi...@gmail.com> wrote: > > > >> On Tue, Dec 26, 2017 at 04:48:25PM +0900, Masami Hiramatsu wrote: > >>> Support in-kernel fault-injection framework via debugfs. > >>> This allows you to inject a conditional error to specified > >>> function using debugfs interfaces. > >>> > >>> Signed-off-by: Masami Hiramatsu <mhira...@kernel.org> > >>> --- > >>> Documentation/fault-injection/fault-injection.txt |5 + > >>> kernel/Makefile |1 > >>> kernel/fail_function.c| 169 > >>> + > >>> lib/Kconfig.debug | 10 + > >>> 4 files changed, 185 insertions(+) > >>> create mode 100644 kernel/fail_function.c > >>> > >>> diff --git a/Documentation/fault-injection/fault-injection.txt > >>> b/Documentation/fault-injection/fault-injection.txt > >>> index 918972babcd8..6243a588dd71 100644 > >>> --- a/Documentation/fault-injection/fault-injection.txt > >>> +++ b/Documentation/fault-injection/fault-injection.txt > >>> @@ -30,6 +30,11 @@ o fail_mmc_request > >>>injects MMC data errors on devices permitted by setting > >>>debugfs entries under /sys/kernel/debug/mmc0/fail_mmc_request > >>> > >>> +o fail_function > >>> + > >>> + injects error return on specific functions by setting debugfs entries > >>> + under /sys/kernel/debug/fail_function. No boot option supported. > >> > >> I like it. > >> Could you document it a bit better? > > > > Yes, I will do in next series. > > > >> In particular retval is configurable, but without an example no one > >> will be able to figure out how to use it. > > > > Ah, right. BTW, as I pointed in the covermail, should we store the > > expected error value range into the injectable list? e.g. > > > > ALLOW_ERROR_INJECTION(open_ctree, -1, -MAX_ERRNO) > > > > And provide APIs to check/get it. > > I'm afraid such check would be too costly. > Right now we have only two functions marked but I expect hundreds more > will be added in the near future as soon as developers realize the > potential of such error injection. > All of ALLOW_ERROR_INJECTION marks add 8 byte overhead each to .data. > Multiple by 1k and we have 8k of data spent on marks. > If we add max/min range marks that doubles it for very little use. > I think marking function only is enough. Sorry, I don't think so. Even if it takes 16 bytes more for each points, I don't think it is any overhead for machines in these days. Even if so, we can provide a kconfig to reduce it. I mean, we are living in GB-order memory are, and it will be bigger in the future. Why we have to worry about hundreds of 16bytes memory pieces? It will take a few KB, and even if we mark thousands of functions, it never reaches 1MB, in GB memory pool. :) Of course, for many small-footprint embedded devices (like having less than 128MB memory), this feature can be a overhead. But they can cut off the table by kconfig. Thank you, -- Masami Hiramatsu <mhira...@kernel.org> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH bpf-next v2 4/4] error-injection: Support fault injection framework
On Tue, 26 Dec 2017 18:12:56 -0800 Alexei Starovoitov <alexei.starovoi...@gmail.com> wrote: > On Tue, Dec 26, 2017 at 04:48:25PM +0900, Masami Hiramatsu wrote: > > Support in-kernel fault-injection framework via debugfs. > > This allows you to inject a conditional error to specified > > function using debugfs interfaces. > > > > Signed-off-by: Masami Hiramatsu <mhira...@kernel.org> > > --- > > Documentation/fault-injection/fault-injection.txt |5 + > > kernel/Makefile |1 > > kernel/fail_function.c| 169 > > + > > lib/Kconfig.debug | 10 + > > 4 files changed, 185 insertions(+) > > create mode 100644 kernel/fail_function.c > > > > diff --git a/Documentation/fault-injection/fault-injection.txt > > b/Documentation/fault-injection/fault-injection.txt > > index 918972babcd8..6243a588dd71 100644 > > --- a/Documentation/fault-injection/fault-injection.txt > > +++ b/Documentation/fault-injection/fault-injection.txt > > @@ -30,6 +30,11 @@ o fail_mmc_request > >injects MMC data errors on devices permitted by setting > >debugfs entries under /sys/kernel/debug/mmc0/fail_mmc_request > > > > +o fail_function > > + > > + injects error return on specific functions by setting debugfs entries > > + under /sys/kernel/debug/fail_function. No boot option supported. > > I like it. > Could you document it a bit better? Yes, I will do in next series. > In particular retval is configurable, but without an example no one > will be able to figure out how to use it. Ah, right. BTW, as I pointed in the covermail, should we store the expected error value range into the injectable list? e.g. ALLOW_ERROR_INJECTION(open_ctree, -1, -MAX_ERRNO) And provide APIs to check/get it. const struct error_range *ei_get_error_range(unsigned long addr); > > I think you can drop RFC tag from the next version of these patches. > Thanks! Thank you, I'll fix some errors came from configurations, and resend it. Thanks! -- Masami Hiramatsu <mhira...@kernel.org> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH bpf-next v2 1/4] tracing/kprobe: bpf: Check error injectable event is on function entry
On Tue, 26 Dec 2017 17:57:32 -0800
Alexei Starovoitov <alexei.starovoi...@gmail.com> wrote:
> On Tue, Dec 26, 2017 at 04:46:59PM +0900, Masami Hiramatsu wrote:
> > Check whether error injectable event is on function entry or not.
> > Currently it checks the event is ftrace-based kprobes or not,
> > but that is wrong. It should check if the event is on the entry
> > of target function. Since error injection will override a function
> > to just return with modified return value, that operation must
> > be done before the target function starts making stackframe.
> >
> > As a side effect, bpf error injection is no need to depend on
> > function-tracer. It can work with sw-breakpoint based kprobe
> > events too.
> >
> > Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
> > ---
> > kernel/trace/Kconfig|2 --
> > kernel/trace/bpf_trace.c|6 +++---
> > kernel/trace/trace_kprobe.c |8 +---
> > kernel/trace/trace_probe.h | 12 ++--
> > 4 files changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> > index ae3a2d519e50..6400e1bf97c5 100644
> > --- a/kernel/trace/Kconfig
> > +++ b/kernel/trace/Kconfig
> > @@ -533,9 +533,7 @@ config FUNCTION_PROFILER
> > config BPF_KPROBE_OVERRIDE
> > bool "Enable BPF programs to override a kprobed function"
> > depends on BPF_EVENTS
> > - depends on KPROBES_ON_FTRACE
> > depends on HAVE_KPROBE_OVERRIDE
> > - depends on DYNAMIC_FTRACE_WITH_REGS
> > default n
> > help
> > Allows BPF to override the execution of a probed function and
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index f6d2327ecb59..d663660f8392 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -800,11 +800,11 @@ int perf_event_attach_bpf_prog(struct perf_event
> > *event,
> > int ret = -EEXIST;
> >
> > /*
> > -* Kprobe override only works for ftrace based kprobes, and only if they
> > -* are on the opt-in list.
> > +* Kprobe override only works if they are on the function entry,
> > +* and only if they are on the opt-in list.
> > */
> > if (prog->kprobe_override &&
> > - (!trace_kprobe_ftrace(event->tp_event) ||
> > + (!trace_kprobe_on_func_entry(event->tp_event) ||
> > !trace_kprobe_error_injectable(event->tp_event)))
> > return -EINVAL;
> >
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index 91f4b57dab82..265e3e27e8dc 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -88,13 +88,15 @@ static nokprobe_inline unsigned long
> > trace_kprobe_nhit(struct trace_kprobe *tk)
> > return nhit;
> > }
> >
> > -int trace_kprobe_ftrace(struct trace_event_call *call)
> > +bool trace_kprobe_on_func_entry(struct trace_event_call *call)
> > {
> > struct trace_kprobe *tk = (struct trace_kprobe *)call->data;
> > - return kprobe_ftrace(>rp.kp);
> > +
> > + return kprobe_on_func_entry(tk->rp.kp.addr, tk->rp.kp.symbol_name,
> > + tk->rp.kp.offset);
>
> That would be nice, but did you test this?
Yes, because the jprobe, which was only official user of modifying execution
path using kprobe, did same way to check. (and kretprobe also does it)
> My understanding that kprobe will restore all regs and
> here we need to override return ip _and_ value.
yes, no problem. kprobe restore all regs from pt_regs, including regs->ip.
> Could you add a patch with the test the way Josef did
> or describe the steps to test this new mode?
Would you mean below patch? If so, it should work without any change.
[PATCH v10 4/5] samples/bpf: add a test for bpf_override_return
Thank you,
--
Masami Hiramatsu <mhira...@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH bpf-next v2 4/4] error-injection: Support fault injection framework
Support in-kernel fault-injection framework via debugfs.
This allows you to inject a conditional error to specified
function using debugfs interfaces.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
---
Documentation/fault-injection/fault-injection.txt |5 +
kernel/Makefile |1
kernel/fail_function.c| 169 +
lib/Kconfig.debug | 10 +
4 files changed, 185 insertions(+)
create mode 100644 kernel/fail_function.c
diff --git a/Documentation/fault-injection/fault-injection.txt
b/Documentation/fault-injection/fault-injection.txt
index 918972babcd8..6243a588dd71 100644
--- a/Documentation/fault-injection/fault-injection.txt
+++ b/Documentation/fault-injection/fault-injection.txt
@@ -30,6 +30,11 @@ o fail_mmc_request
injects MMC data errors on devices permitted by setting
debugfs entries under /sys/kernel/debug/mmc0/fail_mmc_request
+o fail_function
+
+ injects error return on specific functions by setting debugfs entries
+ under /sys/kernel/debug/fail_function. No boot option supported.
+
Configure fault-injection capabilities behavior
---
diff --git a/kernel/Makefile b/kernel/Makefile
index 172d151d429c..f85ae5dfa474 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_AUDIT_TREE) += audit_tree.o
obj-$(CONFIG_GCOV_KERNEL) += gcov/
obj-$(CONFIG_KCOV) += kcov.o
obj-$(CONFIG_KPROBES) += kprobes.o
+obj-$(CONFIG_FAIL_FUNCTION) += fail_function.o
obj-$(CONFIG_KGDB) += debug/
obj-$(CONFIG_DETECT_HUNG_TASK) += hung_task.o
obj-$(CONFIG_LOCKUP_DETECTOR) += watchdog.o
diff --git a/kernel/fail_function.c b/kernel/fail_function.c
new file mode 100644
index ..203d3582487a
--- /dev/null
+++ b/kernel/fail_function.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * fail_function.c: Function-based error injection
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static int fei_kprobe_handler(struct kprobe *kp, struct pt_regs *regs);
+
+static DEFINE_MUTEX(fei_lock);
+static struct {
+ struct kprobe kp;
+ unsigned long retval;
+ struct fault_attr attr;
+} fei_attr = {
+ .kp = { .pre_handler = fei_kprobe_handler, },
+ .retval = ~0UL, /* This indicates -1 in long/int return value */
+ .attr = FAULT_ATTR_INITIALIZER,
+};
+
+static int fei_kprobe_handler(struct kprobe *kp, struct pt_regs *regs)
+{
+ if (should_fail(_attr.attr, 1)) {
+ regs_set_return_value(regs, fei_attr.retval);
+ override_function_with_return(regs);
+ /* Kprobe specific fixup */
+ reset_current_kprobe();
+ preempt_enable_no_resched();
+ return 1;
+ }
+
+ return 0;
+}
+NOKPROBE_SYMBOL(fei_kprobe_handler)
+
+static void *fei_seq_start(struct seq_file *m, loff_t *pos)
+{
+ mutex_lock(_lock);
+ return *pos == 0 ? (void *)1 : NULL;
+}
+
+static void fei_seq_stop(struct seq_file *m, void *v)
+{
+ mutex_unlock(_lock);
+}
+
+static void *fei_seq_next(struct seq_file *m, void *v, loff_t *pos)
+{
+ return NULL;
+}
+
+static int fei_seq_show(struct seq_file *m, void *v)
+{
+ if (fei_attr.kp.addr)
+ seq_printf(m, "%pf\n", fei_attr.kp.addr);
+ else
+ seq_puts(m, "# not specified\n");
+ return 0;
+}
+
+static const struct seq_operations fei_seq_ops = {
+ .start = fei_seq_start,
+ .next = fei_seq_next,
+ .stop = fei_seq_stop,
+ .show = fei_seq_show,
+};
+
+static int fei_open(struct inode *inode, struct file *file)
+{
+ return seq_open(file, _seq_ops);
+}
+
+static ssize_t fei_write(struct file *file, const char __user *buffer,
+size_t count, loff_t *ppos)
+{
+ unsigned long addr;
+ char *buf, *sym;
+ int ret;
+
+ /* cut off if it is too long */
+ if (count > KSYM_NAME_LEN)
+ count = KSYM_NAME_LEN;
+ buf = kmalloc(sizeof(char) * (count + 1), GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ if (copy_from_user(buf, buffer, count)) {
+ ret = -EFAULT;
+ goto out;
+ }
+ buf[count] = '\0';
+ sym = strstrip(buf);
+
+ if (strlen(sym) == 0 || sym[0] == '0') {
+ if (fei_attr.kp.addr) {
+ unregister_kprobe(_attr.kp);
+ fei_attr.kp.addr = NULL;
+ }
+ ret = count;
+ goto out;
+ }
+
+ addr = kallsyms_lookup_name(sym);
+ if (!addr) {
+ ret = -EINVAL;
+ goto out;
+ }
+ if (!within_error_injection_list(addr)) {
+ ret = -ERANGE;
+ goto out;
+ }
+
+ if (fei_attr.kp.addr) {
+
[RFC PATCH bpf-next v2 3/4] error-injection: Separate error-injection from kprobe
Since error-injection framework is not limited to be used
by kprobes, nor bpf. Other kernel subsystems can use it
freely for checking safeness of error-injection, e.g.
livepatch, ftrace etc.
So this separate error-injection framework from kprobes.
Some differences has been made:
- "kprobe" word is removed from any APIs/structures.
- BPF_ALLOW_ERROR_INJECTION() is renamed to
ALLOW_ERROR_INJECTION() since it is not limited for BPF too.
- CONFIG_FUNCTION_ERROR_INJECTION is the config item of this
feature. It is automatically enabled if the arch supports
error injection feature for kprobe or ftrace etc.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
---
Changes in v2:
- Fix the override function name to override_function_with_return()
- Show only function name in the list, user don't have to care about
it's size, since function override only happens at the entry.
---
arch/Kconfig |2
arch/x86/Kconfig |2
arch/x86/include/asm/error-injection.h | 12 ++
arch/x86/kernel/kprobes/ftrace.c | 14 --
arch/x86/lib/Makefile |2
arch/x86/lib/error-inject.c| 19 +++
fs/btrfs/disk-io.c |2
fs/btrfs/free-space-cache.c|2
include/asm-generic/error-injection.h | 20 +++
include/asm-generic/vmlinux.lds.h | 14 +-
include/linux/bpf.h| 12 --
include/linux/error-injection.h| 21 +++
include/linux/kprobes.h|1
include/linux/module.h |6 -
kernel/kprobes.c | 163 --
kernel/module.c|8 +
kernel/trace/Kconfig |2
kernel/trace/bpf_trace.c |2
kernel/trace/trace_kprobe.c|3
lib/Kconfig.debug |4 +
lib/Makefile |1
lib/error-inject.c | 198
22 files changed, 300 insertions(+), 210 deletions(-)
create mode 100644 arch/x86/include/asm/error-injection.h
create mode 100644 arch/x86/lib/error-inject.c
create mode 100644 include/asm-generic/error-injection.h
create mode 100644 include/linux/error-injection.h
create mode 100644 lib/error-inject.c
diff --git a/arch/Kconfig b/arch/Kconfig
index d3f4aaf9cb7a..97376accfb14 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -196,7 +196,7 @@ config HAVE_OPTPROBES
config HAVE_KPROBES_ON_FTRACE
bool
-config HAVE_KPROBE_OVERRIDE
+config HAVE_FUNCTION_ERROR_INJECTION
bool
config HAVE_NMI
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 04d66e6fa447..fc519e3ae754 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -154,7 +154,7 @@ config X86
select HAVE_KERNEL_XZ
select HAVE_KPROBES
select HAVE_KPROBES_ON_FTRACE
- select HAVE_KPROBE_OVERRIDE
+ select HAVE_FUNCTION_ERROR_INJECTION
select HAVE_KRETPROBES
select HAVE_KVM
select HAVE_LIVEPATCH if X86_64
diff --git a/arch/x86/include/asm/error-injection.h
b/arch/x86/include/asm/error-injection.h
new file mode 100644
index ..6c2a133622f4
--- /dev/null
+++ b/arch/x86/include/asm/error-injection.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ERROR_INJECTION_H
+#define _ASM_ERROR_INJECTION_H
+
+#include
+#include
+#include
+
+asmlinkage void just_return_func(void);
+void override_function_with_return(struct pt_regs *regs);
+
+#endif /* _ASM_ERROR_INJECTION_H */
diff --git a/arch/x86/kernel/kprobes/ftrace.c b/arch/x86/kernel/kprobes/ftrace.c
index 1ea748d682fd..8dc0161cec8f 100644
--- a/arch/x86/kernel/kprobes/ftrace.c
+++ b/arch/x86/kernel/kprobes/ftrace.c
@@ -97,17 +97,3 @@ int arch_prepare_kprobe_ftrace(struct kprobe *p)
p->ainsn.boostable = false;
return 0;
}
-
-asmlinkage void override_func(void);
-asm(
- ".type override_func, @function\n"
- "override_func:\n"
- " ret\n"
- ".size override_func, .-override_func\n"
-);
-
-void arch_ftrace_kprobe_override_function(struct pt_regs *regs)
-{
- regs->ip = (unsigned long)_func;
-}
-NOKPROBE_SYMBOL(arch_ftrace_kprobe_override_function);
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index 7b181b61170e..081f09435d28 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -26,6 +26,8 @@ lib-y += memcpy_$(BITS).o
lib-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o
lib-$(CONFIG_INSTRUCTION_DECODER) += insn.o inat.o insn-eval.o
lib-$(CONFIG_RANDOMIZE_BASE) += kaslr.o
+lib-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
+
obj-y += msr.o msr-reg.o msr-reg-export.o hweight.o
diff --git a/arch/x86/lib/error-inject.c b/arch/x86/lib/error-inject.c
new file mode 100644
index ..7b881d03d0dd
--- /dev/null
+++ b/arch/x86/lib/er
[RFC PATCH bpf-next v2 2/4] tracing/kprobe: bpf: Compare instruction pointer with original one
Compare instruction pointer with original one on the
stack instead using per-cpu bpf_kprobe_override flag.
This patch also consolidates reset_current_kprobe() and
preempt_enable_no_resched() blocks. Those can be done
in one place.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
---
kernel/trace/bpf_trace.c|1 -
kernel/trace/trace_kprobe.c | 21 +++--
2 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d663660f8392..cefa9b0e396c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -83,7 +83,6 @@ EXPORT_SYMBOL_GPL(trace_call_bpf);
#ifdef CONFIG_BPF_KPROBE_OVERRIDE
BPF_CALL_2(bpf_override_return, struct pt_regs *, regs, unsigned long, rc)
{
- __this_cpu_write(bpf_kprobe_override, 1);
regs_set_return_value(regs, rc);
arch_ftrace_kprobe_override_function(regs);
return 0;
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 265e3e27e8dc..a7c7035963f2 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -42,8 +42,6 @@ struct trace_kprobe {
(offsetof(struct trace_kprobe, tp.args) + \
(sizeof(struct probe_arg) * (n)))
-DEFINE_PER_CPU(int, bpf_kprobe_override);
-
static nokprobe_inline bool trace_kprobe_is_return(struct trace_kprobe *tk)
{
return tk->rp.handler != NULL;
@@ -1204,6 +1202,7 @@ kprobe_perf_func(struct trace_kprobe *tk, struct pt_regs
*regs)
int rctx;
if (bpf_prog_array_valid(call)) {
+ unsigned long orig_ip = instruction_pointer(regs);
int ret;
ret = trace_call_bpf(call, regs);
@@ -1211,12 +1210,13 @@ kprobe_perf_func(struct trace_kprobe *tk, struct
pt_regs *regs)
/*
* We need to check and see if we modified the pc of the
* pt_regs, and if so clear the kprobe and return 1 so that we
-* don't do the instruction skipping. Also reset our state so
-* we are clean the next pass through.
+* don't do the single stepping.
+* The ftrace kprobe handler leaves it up to us to re-enable
+* preemption here before returning if we've modified the ip.
*/
- if (__this_cpu_read(bpf_kprobe_override)) {
- __this_cpu_write(bpf_kprobe_override, 0);
+ if (orig_ip != instruction_pointer(regs)) {
reset_current_kprobe();
+ preempt_enable_no_resched();
return 1;
}
if (!ret)
@@ -1324,15 +1324,8 @@ static int kprobe_dispatcher(struct kprobe *kp, struct
pt_regs *regs)
if (tk->tp.flags & TP_FLAG_TRACE)
kprobe_trace_func(tk, regs);
#ifdef CONFIG_PERF_EVENTS
- if (tk->tp.flags & TP_FLAG_PROFILE) {
+ if (tk->tp.flags & TP_FLAG_PROFILE)
ret = kprobe_perf_func(tk, regs);
- /*
-* The ftrace kprobe handler leaves it up to us to re-enable
-* preemption here before returning if we've modified the ip.
-*/
- if (ret)
- preempt_enable_no_resched();
- }
#endif
return ret;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH bpf-next v2 1/4] tracing/kprobe: bpf: Check error injectable event is on function entry
Check whether error injectable event is on function entry or not.
Currently it checks the event is ftrace-based kprobes or not,
but that is wrong. It should check if the event is on the entry
of target function. Since error injection will override a function
to just return with modified return value, that operation must
be done before the target function starts making stackframe.
As a side effect, bpf error injection is no need to depend on
function-tracer. It can work with sw-breakpoint based kprobe
events too.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
---
kernel/trace/Kconfig|2 --
kernel/trace/bpf_trace.c|6 +++---
kernel/trace/trace_kprobe.c |8 +---
kernel/trace/trace_probe.h | 12 ++--
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index ae3a2d519e50..6400e1bf97c5 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -533,9 +533,7 @@ config FUNCTION_PROFILER
config BPF_KPROBE_OVERRIDE
bool "Enable BPF programs to override a kprobed function"
depends on BPF_EVENTS
- depends on KPROBES_ON_FTRACE
depends on HAVE_KPROBE_OVERRIDE
- depends on DYNAMIC_FTRACE_WITH_REGS
default n
help
Allows BPF to override the execution of a probed function and
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index f6d2327ecb59..d663660f8392 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -800,11 +800,11 @@ int perf_event_attach_bpf_prog(struct perf_event *event,
int ret = -EEXIST;
/*
-* Kprobe override only works for ftrace based kprobes, and only if they
-* are on the opt-in list.
+* Kprobe override only works if they are on the function entry,
+* and only if they are on the opt-in list.
*/
if (prog->kprobe_override &&
- (!trace_kprobe_ftrace(event->tp_event) ||
+ (!trace_kprobe_on_func_entry(event->tp_event) ||
!trace_kprobe_error_injectable(event->tp_event)))
return -EINVAL;
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 91f4b57dab82..265e3e27e8dc 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,13 +88,15 @@ static nokprobe_inline unsigned long
trace_kprobe_nhit(struct trace_kprobe *tk)
return nhit;
}
-int trace_kprobe_ftrace(struct trace_event_call *call)
+bool trace_kprobe_on_func_entry(struct trace_event_call *call)
{
struct trace_kprobe *tk = (struct trace_kprobe *)call->data;
- return kprobe_ftrace(>rp.kp);
+
+ return kprobe_on_func_entry(tk->rp.kp.addr, tk->rp.kp.symbol_name,
+ tk->rp.kp.offset);
}
-int trace_kprobe_error_injectable(struct trace_event_call *call)
+bool trace_kprobe_error_injectable(struct trace_event_call *call)
{
struct trace_kprobe *tk = (struct trace_kprobe *)call->data;
unsigned long addr;
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 5e54d748c84c..e101c5bb9eda 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -252,8 +252,8 @@ struct symbol_cache;
unsigned long update_symbol_cache(struct symbol_cache *sc);
void free_symbol_cache(struct symbol_cache *sc);
struct symbol_cache *alloc_symbol_cache(const char *sym, long offset);
-int trace_kprobe_ftrace(struct trace_event_call *call);
-int trace_kprobe_error_injectable(struct trace_event_call *call);
+bool trace_kprobe_on_func_entry(struct trace_event_call *call);
+bool trace_kprobe_error_injectable(struct trace_event_call *call);
#else
/* uprobes do not support symbol fetch methods */
#define fetch_symbol_u8NULL
@@ -280,14 +280,14 @@ alloc_symbol_cache(const char *sym, long offset)
return NULL;
}
-static inline int trace_kprobe_ftrace(struct trace_event_call *call)
+static inline bool trace_kprobe_on_func_entry(struct trace_event_call *call)
{
- return 0;
+ return false;
}
-static inline int trace_kprobe_error_injectable(struct trace_event_call *call)
+static inline bool trace_kprobe_error_injectable(struct trace_event_call *call)
{
- return 0;
+ return false;
}
#endif /* CONFIG_KPROBE_EVENTS */
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH bpf-next v2 0/4] Separate error injection table from kprobes
Hi Josef and Alexei, Here are the 2nd version of patches to moving error injection table from kprobes. In this series I did a small fixes and add function-based fault injection. Here is the previous version: https://lkml.org/lkml/2017/12/22/554 There are 2 main reasons why I separate it from kprobes. - kprobes users can modify execution path not only at error-injection whitelist functions but also other functions. I don't like to suggest user that such limitation is from kprobes itself. - This error injection information is also useful for ftrace (function-hook) and livepatch. It should not be limited by CONFIG_KPROBES. So I introduced CONFIG_FUNCTION_ERROR_INJECTION for this feature. Also CONFIG_FAIL_FUNCTION is added, which provides function-based error injection interface via debugfs following fault-injection framework. See [4/4]. Any thoughts? BTW, I think we should add an error-range description in ALLOW_ERROR_INJECTION() macro. If user sets a success return value and override it by mistake, caller must break data or cause kernel panic. Thank you, --- Masami Hiramatsu (4): tracing/kprobe: bpf: Check error injectable event is on function entry tracing/kprobe: bpf: Compare instruction pointer with original one error-injection: Separate error-injection from kprobe error-injection: Support fault injection framework Documentation/fault-injection/fault-injection.txt |5 + arch/Kconfig |2 arch/x86/Kconfig |2 arch/x86/include/asm/error-injection.h| 12 + arch/x86/kernel/kprobes/ftrace.c | 14 - arch/x86/lib/Makefile |2 arch/x86/lib/error-inject.c | 19 ++ fs/btrfs/disk-io.c|2 fs/btrfs/free-space-cache.c |2 include/asm-generic/error-injection.h | 20 ++ include/asm-generic/vmlinux.lds.h | 14 + include/linux/bpf.h | 12 - include/linux/error-injection.h | 21 ++ include/linux/kprobes.h |1 include/linux/module.h|6 - kernel/Makefile |1 kernel/fail_function.c| 169 ++ kernel/kprobes.c | 163 - kernel/module.c |8 - kernel/trace/Kconfig |4 kernel/trace/bpf_trace.c |9 - kernel/trace/trace_kprobe.c | 32 +-- kernel/trace/trace_probe.h| 12 + lib/Kconfig.debug | 14 + lib/Makefile |1 lib/error-inject.c| 198 + 26 files changed, 506 insertions(+), 239 deletions(-) create mode 100644 arch/x86/include/asm/error-injection.h create mode 100644 arch/x86/lib/error-inject.c create mode 100644 include/asm-generic/error-injection.h create mode 100644 include/linux/error-injection.h create mode 100644 kernel/fail_function.c create mode 100644 lib/error-inject.c -- Masami Hiramatsu (Linaro) -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH bpf-next 3/3] error-injection: Separate error-injection from kprobe
Since error-injection framework is not limited to be used
by kprobes, nor bpf. Other kernel subsystems can use it
freely for checking safeness of error-injection, e.g.
livepatch, ftrace etc.
So this separate error-injection framework from kprobes.
Some differences has been made:
- "kprobe" word is removed from any APIs/structures.
- BPF_ALLOW_ERROR_INJECTION() is renamed to
ALLOW_ERROR_INJECTION() since it is not limited for BPF too.
- CONFIG_FUNCTION_ERROR_INJECTION is the config item of this
feature. It is automatically enabled if the arch supports
error injection feature for kprobe or ftrace etc.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
---
arch/Kconfig |2
arch/x86/Kconfig |2
arch/x86/include/asm/error-injection.h | 12 ++
arch/x86/kernel/kprobes/ftrace.c | 14 --
arch/x86/lib/Makefile |2
arch/x86/lib/error-inject.c| 19 +++
fs/btrfs/disk-io.c |2
fs/btrfs/free-space-cache.c|2
include/asm-generic/error-injection.h | 20 +++
include/asm-generic/vmlinux.lds.h | 14 +-
include/linux/bpf.h| 12 --
include/linux/error-injection.h| 21 +++
include/linux/kprobes.h|1
include/linux/module.h |6 -
kernel/kprobes.c | 163 --
kernel/module.c|8 +
kernel/trace/Kconfig |2
kernel/trace/bpf_trace.c |2
kernel/trace/trace_kprobe.c|3
lib/Kconfig.debug |4 +
lib/Makefile |1
lib/error-inject.c | 200
22 files changed, 302 insertions(+), 210 deletions(-)
create mode 100644 arch/x86/include/asm/error-injection.h
create mode 100644 arch/x86/lib/error-inject.c
create mode 100644 include/asm-generic/error-injection.h
create mode 100644 include/linux/error-injection.h
create mode 100644 lib/error-inject.c
diff --git a/arch/Kconfig b/arch/Kconfig
index d3f4aaf9cb7a..97376accfb14 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -196,7 +196,7 @@ config HAVE_OPTPROBES
config HAVE_KPROBES_ON_FTRACE
bool
-config HAVE_KPROBE_OVERRIDE
+config HAVE_FUNCTION_ERROR_INJECTION
bool
config HAVE_NMI
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 04d66e6fa447..fc519e3ae754 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -154,7 +154,7 @@ config X86
select HAVE_KERNEL_XZ
select HAVE_KPROBES
select HAVE_KPROBES_ON_FTRACE
- select HAVE_KPROBE_OVERRIDE
+ select HAVE_FUNCTION_ERROR_INJECTION
select HAVE_KRETPROBES
select HAVE_KVM
select HAVE_LIVEPATCH if X86_64
diff --git a/arch/x86/include/asm/error-injection.h
b/arch/x86/include/asm/error-injection.h
new file mode 100644
index ..d89759a0354c
--- /dev/null
+++ b/arch/x86/include/asm/error-injection.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ERROR_INJECTION_H
+#define _ASM_ERROR_INJECTION_H
+
+#include
+#include
+#include
+
+asmlinkage void just_return_func(void);
+void override_function_to_return(struct pt_regs *regs);
+
+#endif /* _ASM_ERROR_INJECTION_H */
diff --git a/arch/x86/kernel/kprobes/ftrace.c b/arch/x86/kernel/kprobes/ftrace.c
index 1ea748d682fd..8dc0161cec8f 100644
--- a/arch/x86/kernel/kprobes/ftrace.c
+++ b/arch/x86/kernel/kprobes/ftrace.c
@@ -97,17 +97,3 @@ int arch_prepare_kprobe_ftrace(struct kprobe *p)
p->ainsn.boostable = false;
return 0;
}
-
-asmlinkage void override_func(void);
-asm(
- ".type override_func, @function\n"
- "override_func:\n"
- " ret\n"
- ".size override_func, .-override_func\n"
-);
-
-void arch_ftrace_kprobe_override_function(struct pt_regs *regs)
-{
- regs->ip = (unsigned long)_func;
-}
-NOKPROBE_SYMBOL(arch_ftrace_kprobe_override_function);
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index 7b181b61170e..081f09435d28 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -26,6 +26,8 @@ lib-y += memcpy_$(BITS).o
lib-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o
lib-$(CONFIG_INSTRUCTION_DECODER) += insn.o inat.o insn-eval.o
lib-$(CONFIG_RANDOMIZE_BASE) += kaslr.o
+lib-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
+
obj-y += msr.o msr-reg.o msr-reg-export.o hweight.o
diff --git a/arch/x86/lib/error-inject.c b/arch/x86/lib/error-inject.c
new file mode 100644
index ..1998d4ae161e
--- /dev/null
+++ b/arch/x86/lib/error-inject.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include
+#include
+
+asmlinkage void just_return_func(void);
+
+asm(
+ ".type just_return_func, @function\n"
+ "just_return_func:\n&
[RFC PATCH bpf-next 2/3] tracing/kprobe: bpf: Compare instruction pointer with original one
Compare instruction pointer with original one on the
stack instead using per-cpu bpf_kprobe_override flag.
This patch also consolidates reset_current_kprobe() and
preempt_enable_no_resched() blocks. Those can be done
in one place.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
---
kernel/trace/bpf_trace.c|1 -
kernel/trace/trace_kprobe.c | 21 +++--
2 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d663660f8392..cefa9b0e396c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -83,7 +83,6 @@ EXPORT_SYMBOL_GPL(trace_call_bpf);
#ifdef CONFIG_BPF_KPROBE_OVERRIDE
BPF_CALL_2(bpf_override_return, struct pt_regs *, regs, unsigned long, rc)
{
- __this_cpu_write(bpf_kprobe_override, 1);
regs_set_return_value(regs, rc);
arch_ftrace_kprobe_override_function(regs);
return 0;
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 265e3e27e8dc..a7c7035963f2 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -42,8 +42,6 @@ struct trace_kprobe {
(offsetof(struct trace_kprobe, tp.args) + \
(sizeof(struct probe_arg) * (n)))
-DEFINE_PER_CPU(int, bpf_kprobe_override);
-
static nokprobe_inline bool trace_kprobe_is_return(struct trace_kprobe *tk)
{
return tk->rp.handler != NULL;
@@ -1204,6 +1202,7 @@ kprobe_perf_func(struct trace_kprobe *tk, struct pt_regs
*regs)
int rctx;
if (bpf_prog_array_valid(call)) {
+ unsigned long orig_ip = instruction_pointer(regs);
int ret;
ret = trace_call_bpf(call, regs);
@@ -1211,12 +1210,13 @@ kprobe_perf_func(struct trace_kprobe *tk, struct
pt_regs *regs)
/*
* We need to check and see if we modified the pc of the
* pt_regs, and if so clear the kprobe and return 1 so that we
-* don't do the instruction skipping. Also reset our state so
-* we are clean the next pass through.
+* don't do the single stepping.
+* The ftrace kprobe handler leaves it up to us to re-enable
+* preemption here before returning if we've modified the ip.
*/
- if (__this_cpu_read(bpf_kprobe_override)) {
- __this_cpu_write(bpf_kprobe_override, 0);
+ if (orig_ip != instruction_pointer(regs)) {
reset_current_kprobe();
+ preempt_enable_no_resched();
return 1;
}
if (!ret)
@@ -1324,15 +1324,8 @@ static int kprobe_dispatcher(struct kprobe *kp, struct
pt_regs *regs)
if (tk->tp.flags & TP_FLAG_TRACE)
kprobe_trace_func(tk, regs);
#ifdef CONFIG_PERF_EVENTS
- if (tk->tp.flags & TP_FLAG_PROFILE) {
+ if (tk->tp.flags & TP_FLAG_PROFILE)
ret = kprobe_perf_func(tk, regs);
- /*
-* The ftrace kprobe handler leaves it up to us to re-enable
-* preemption here before returning if we've modified the ip.
-*/
- if (ret)
- preempt_enable_no_resched();
- }
#endif
return ret;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH bpf-next 1/3] tracing/kprobe: bpf: Check error injectable event is on function entry
Check whether error injectable event is on function entry or not.
Currently it checks the event is ftrace-based kprobes or not,
but that is wrong. It should check if the event is on the entry
of target function. Since error injection will override a function
to just return with modified return value, that operation must
be done before the target function starts making stackframe.
As a side effect, bpf error injection is no need to depend on
function-tracer. It can work with sw-breakpoint based kprobe
events too.
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
---
kernel/trace/Kconfig|2 --
kernel/trace/bpf_trace.c|6 +++---
kernel/trace/trace_kprobe.c |8 +---
kernel/trace/trace_probe.h | 12 ++--
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index ae3a2d519e50..6400e1bf97c5 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -533,9 +533,7 @@ config FUNCTION_PROFILER
config BPF_KPROBE_OVERRIDE
bool "Enable BPF programs to override a kprobed function"
depends on BPF_EVENTS
- depends on KPROBES_ON_FTRACE
depends on HAVE_KPROBE_OVERRIDE
- depends on DYNAMIC_FTRACE_WITH_REGS
default n
help
Allows BPF to override the execution of a probed function and
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index f6d2327ecb59..d663660f8392 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -800,11 +800,11 @@ int perf_event_attach_bpf_prog(struct perf_event *event,
int ret = -EEXIST;
/*
-* Kprobe override only works for ftrace based kprobes, and only if they
-* are on the opt-in list.
+* Kprobe override only works if they are on the function entry,
+* and only if they are on the opt-in list.
*/
if (prog->kprobe_override &&
- (!trace_kprobe_ftrace(event->tp_event) ||
+ (!trace_kprobe_on_func_entry(event->tp_event) ||
!trace_kprobe_error_injectable(event->tp_event)))
return -EINVAL;
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 91f4b57dab82..265e3e27e8dc 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,13 +88,15 @@ static nokprobe_inline unsigned long
trace_kprobe_nhit(struct trace_kprobe *tk)
return nhit;
}
-int trace_kprobe_ftrace(struct trace_event_call *call)
+bool trace_kprobe_on_func_entry(struct trace_event_call *call)
{
struct trace_kprobe *tk = (struct trace_kprobe *)call->data;
- return kprobe_ftrace(>rp.kp);
+
+ return kprobe_on_func_entry(tk->rp.kp.addr, tk->rp.kp.symbol_name,
+ tk->rp.kp.offset);
}
-int trace_kprobe_error_injectable(struct trace_event_call *call)
+bool trace_kprobe_error_injectable(struct trace_event_call *call)
{
struct trace_kprobe *tk = (struct trace_kprobe *)call->data;
unsigned long addr;
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 5e54d748c84c..e101c5bb9eda 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -252,8 +252,8 @@ struct symbol_cache;
unsigned long update_symbol_cache(struct symbol_cache *sc);
void free_symbol_cache(struct symbol_cache *sc);
struct symbol_cache *alloc_symbol_cache(const char *sym, long offset);
-int trace_kprobe_ftrace(struct trace_event_call *call);
-int trace_kprobe_error_injectable(struct trace_event_call *call);
+bool trace_kprobe_on_func_entry(struct trace_event_call *call);
+bool trace_kprobe_error_injectable(struct trace_event_call *call);
#else
/* uprobes do not support symbol fetch methods */
#define fetch_symbol_u8NULL
@@ -280,14 +280,14 @@ alloc_symbol_cache(const char *sym, long offset)
return NULL;
}
-static inline int trace_kprobe_ftrace(struct trace_event_call *call)
+static inline bool trace_kprobe_on_func_entry(struct trace_event_call *call)
{
- return 0;
+ return false;
}
-static inline int trace_kprobe_error_injectable(struct trace_event_call *call)
+static inline bool trace_kprobe_error_injectable(struct trace_event_call *call)
{
- return 0;
+ return false;
}
#endif /* CONFIG_KPROBE_EVENTS */
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH bpf-next 0/3] Separate error injection framework from kprobes
Hi Josef and Alexei, Here are the patches which describe what I think more "natural" introduction of error injection APIs. Basically what I did on this series is to separate error injection from kprobes and put it on new error-injection small subsystem which is currently provide whitelists and just-return function stub. There are 2 main reasons why I separate it from kprobes. - kprobes users can modify execution path not only at error-injection whitelist functions but also other functions. I don't like to suggest user that such limitation is from kprobes itself. - This error injection information is also useful for ftrace (function-hook) and livepatch. It should not be limited by CONFIG_KPROBES. So I introduced CONFIG_FUNCTION_ERROR_INJECTION for this feature. This series also have some improvement suggestions. - [1/3] "kprobe override function" feature is not limited by ftrace-based kprobe, but also you can use it on sw-breakpoint based kprobe too. Also, you must check the kprobe is on the entry of function right before setting up the stackframe. - [2/3] If we store original instruction pointer and compare it with regs->ip, we don't need per-cpu bpf_kprobe_override. Also, reset_current_kprobe() and preempt_enable_no_resched() are no need to separate. Any thoughts? If it is good, I also add MAINTAINERS entry for this feature and add some testcases using kprobes and ftrace to inject error. (And maybe we also need a document how to use) BTW, it seems there are many error injection frameworks in lib/. We may also consider these distinctions. Thank you, --- Masami Hiramatsu (3): tracing/kprobe: bpf: Check error injectable event is on function entry tracing/kprobe: bpf: Compare instruction pointer with original one error-injection: Separate error-injection from kprobe arch/Kconfig |2 arch/x86/Kconfig |2 arch/x86/include/asm/error-injection.h | 12 ++ arch/x86/kernel/kprobes/ftrace.c | 14 -- arch/x86/lib/Makefile |2 arch/x86/lib/error-inject.c| 19 +++ fs/btrfs/disk-io.c |2 fs/btrfs/free-space-cache.c|2 include/asm-generic/error-injection.h | 20 +++ include/asm-generic/vmlinux.lds.h | 14 +- include/linux/bpf.h| 12 -- include/linux/error-injection.h| 21 +++ include/linux/kprobes.h|1 include/linux/module.h |6 - kernel/kprobes.c | 163 -- kernel/module.c|8 + kernel/trace/Kconfig |4 - kernel/trace/bpf_trace.c |9 + kernel/trace/trace_kprobe.c| 32 ++--- kernel/trace/trace_probe.h | 12 +- lib/Kconfig.debug |4 + lib/Makefile |1 lib/error-inject.c | 200 23 files changed, 323 insertions(+), 239 deletions(-) create mode 100644 arch/x86/include/asm/error-injection.h create mode 100644 arch/x86/lib/error-inject.c create mode 100644 include/asm-generic/error-injection.h create mode 100644 include/linux/error-injection.h create mode 100644 lib/error-inject.c -- Signature -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 1/5] add infrastructure for tagging functions as error injectable
_kprobe_ei_list(void) {}
> +static inline void module_load_kprobe_ei_list(struct module *m) {}
> +static inline void module_unload_kprobe_ei_list(struct module *m) {}
> +#endif
> +
> /* Module notifier call back, checking kprobes on the module */
> static int kprobes_module_callback(struct notifier_block *nb,
> unsigned long val, void *data)
> @@ -2178,6 +2279,11 @@ static int kprobes_module_callback(struct
> notifier_block *nb,
> unsigned int i;
> int checkcore = (val == MODULE_STATE_GOING);
>
> + if (val == MODULE_STATE_COMING)
> + module_load_kprobe_ei_list(mod);
> + else if (val == MODULE_STATE_GOING)
> + module_unload_kprobe_ei_list(mod);
> +
> if (val != MODULE_STATE_GOING && val != MODULE_STATE_LIVE)
> return NOTIFY_DONE;
>
> @@ -2240,6 +2346,8 @@ static int __init init_kprobes(void)
> pr_err("Please take care of using kprobes.\n");
> }
>
> + populate_kernel_kprobe_ei_list();
> +
> if (kretprobe_blacklist_size) {
> /* lookup the function address from its name */
> for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
> @@ -2407,6 +2515,56 @@ static const struct file_operations
> debugfs_kprobe_blacklist_ops = {
> .release= seq_release,
> };
>
> +/*
> + * kprobes/error_injection_list -- shows which functions can be overriden for
> + * error injection.
> + * */
> +static void *kprobe_ei_seq_start(struct seq_file *m, loff_t *pos)
> +{
> + mutex_lock(_ei_mutex);
> + return seq_list_start(_error_injection_list, *pos);
> +}
> +
> +static void kprobe_ei_seq_stop(struct seq_file *m, void *v)
> +{
> + mutex_unlock(_ei_mutex);
> +}
> +
> +static void *kprobe_ei_seq_next(struct seq_file *m, void *v, loff_t *pos)
> +{
> + return seq_list_next(v, _error_injection_list, pos);
> +}
> +
> +static int kprobe_ei_seq_show(struct seq_file *m, void *v)
> +{
> + char buffer[KSYM_SYMBOL_LEN];
> + struct kprobe_ei_entry *ent =
> + list_entry(v, struct kprobe_ei_entry, list);
> +
> + sprint_symbol(buffer, ent->start_addr);
> + seq_printf(m, "%s\n", buffer);
> + return 0;
> +}
> +
> +static const struct seq_operations kprobe_ei_seq_ops = {
> + .start = kprobe_ei_seq_start,
> + .next = kprobe_ei_seq_next,
> + .stop = kprobe_ei_seq_stop,
> + .show = kprobe_ei_seq_show,
> +};
> +
> +static int kprobe_ei_open(struct inode *inode, struct file *filp)
> +{
> + return seq_open(filp, _ei_seq_ops);
> +}
> +
> +static const struct file_operations debugfs_kprobe_ei_ops = {
> + .open = kprobe_ei_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release= seq_release,
> +};
> +
> static void arm_all_kprobes(void)
> {
> struct hlist_head *head;
> @@ -2548,6 +2706,11 @@ static int __init debugfs_kprobe_init(void)
> if (!file)
> goto error;
>
> + file = debugfs_create_file("error_injection_list", 0444, dir, NULL,
> + _kprobe_ei_ops);
> + if (!file)
> + goto error;
> +
> return 0;
>
> error:
> diff --git a/kernel/module.c b/kernel/module.c
> index dea01ac9cb74..bd695bfdc5c4 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3118,7 +3118,11 @@ static int find_module_sections(struct module *mod,
> struct load_info *info)
>sizeof(*mod->ftrace_callsites),
>>num_ftrace_callsites);
> #endif
> -
> +#ifdef CONFIG_BPF_KPROBE_OVERRIDE
> + mod->kprobe_ei_funcs = section_objs(info, "_kprobe_error_inject_list",
> + sizeof(*mod->kprobe_ei_funcs),
> + >num_kprobe_ei_funcs);
> +#endif
> mod->extable = section_objs(info, "__ex_table",
> sizeof(*mod->extable), >num_exentries);
>
> --
> 2.7.5
>
--
Masami Hiramatsu <mhira...@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 1/5] add infrastructure for tagging functions as error injectable
On Tue, 19 Dec 2017 18:14:17 -0800 Alexei Starovoitov <a...@fb.com> wrote: > On 12/18/17 10:29 PM, Masami Hiramatsu wrote: > >> > >> +#if defined(__KERNEL__) && !defined(__ASSEMBLY__) > >> +#ifdef CONFIG_BPF_KPROBE_OVERRIDE > > > > BTW, CONFIG_BPF_KPROBE_OVERRIDE is also confusable name. > > Since this feature override a function to just return with > > some return value (as far as I understand, or would you > > also plan to modify execution path inside a function?), > > I think it should be better CONFIG_BPF_FUNCTION_OVERRIDE or > > CONFIG_BPF_EXECUTION_OVERRIDE. > > I don't think such renaming makes sense. > The feature is overriding kprobe by changing how kprobe returns. > It doesn't override BPF_FUNCTION or BPF_EXECUTION. No, I meant this is BPF's feature which override FUNCTION, so BPF is a kind of namespace. (Is that only for a function entry because it can not tweak stackframe at this morment?) > The kernel enters and exists bpf program as normal. Yeah, but that bpf program modifies instruction pointer, am I correct? > > > Indeed, BPF is based on kprobes, but it seems you are limiting it > > with ftrace (function-call trace) (I'm not sure the reason why), > > so using "kprobes" for this feature seems strange for me. > > do you have an idea how kprobe override can happen when kprobe > placed in the middle of the function? For example, if you know a basic block in the function, maybe you can skip a block or something like that. But nowadays it is somewhat hard because optimizer mixed it up. > > Please make your suggestion as patches based on top of bpf-next. bpf-next seems already pick this series. Would you mean I revert it and write new patch? Thank you, > > Thanks > -- Masami Hiramatsu <mhira...@kernel.org> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 3/5] bpf: add a bpf_override_function helper
On Mon, 18 Dec 2017 16:09:30 +0100
Daniel Borkmann <dan...@iogearbox.net> wrote:
> On 12/18/2017 10:51 AM, Masami Hiramatsu wrote:
> > On Fri, 15 Dec 2017 14:12:54 -0500
> > Josef Bacik <jo...@toxicpanda.com> wrote:
> >> From: Josef Bacik <jba...@fb.com>
> >>
> >> Error injection is sloppy and very ad-hoc. BPF could fill this niche
> >> perfectly with it's kprobe functionality. We could make sure errors are
> >> only triggered in specific call chains that we care about with very
> >> specific situations. Accomplish this with the bpf_override_funciton
> >> helper. This will modify the probe'd callers return value to the
> >> specified value and set the PC to an override function that simply
> >> returns, bypassing the originally probed function. This gives us a nice
> >> clean way to implement systematic error injection for all of our code
> >> paths.
> >
> > OK, got it. I think the error_injectable function list should be defined
> > in kernel/trace/bpf_trace.c because only bpf calls it and needs to care
> > the "safeness".
> >
> > [...]
> >> diff --git a/arch/x86/kernel/kprobes/ftrace.c
> >> b/arch/x86/kernel/kprobes/ftrace.c
> >> index 8dc0161cec8f..1ea748d682fd 100644
> >> --- a/arch/x86/kernel/kprobes/ftrace.c
> >> +++ b/arch/x86/kernel/kprobes/ftrace.c
> >> @@ -97,3 +97,17 @@ int arch_prepare_kprobe_ftrace(struct kprobe *p)
> >>p->ainsn.boostable = false;
> >>return 0;
> >> }
> >> +
> >> +asmlinkage void override_func(void);
> >> +asm(
> >> + ".type override_func, @function\n"
> >> + "override_func:\n"
> >> + " ret\n"
> >> + ".size override_func, .-override_func\n"
> >> +);
> >> +
> >> +void arch_ftrace_kprobe_override_function(struct pt_regs *regs)
> >> +{
> >> + regs->ip = (unsigned long)_func;
> >> +}
> >> +NOKPROBE_SYMBOL(arch_ftrace_kprobe_override_function);
> >
> > Calling this as "override_function" is meaningless. This is a function
> > which just return. So I think combination of just_return_func() and
> > arch_bpf_override_func_just_return() will be better.
> >
> > Moreover, this arch/x86/kernel/kprobes/ftrace.c is an archtecture
> > dependent implementation of kprobes, not bpf.
>
> Josef, please work out any necessary cleanups that would still need
> to be addressed based on Masami's feedback and send them as follow-up
> patches, thanks.
>
> > Hmm, arch/x86/net/bpf_jit_comp.c will be better place?
>
> (No, it's JIT only and I'd really prefer to keep it that way, mixing
> this would result in a huge mess.)
OK, that is same to kprobes. kernel/kprobes.c and arch/x86/kernel/kprobe/*
are for instrumentation code. And kernel/trace/trace_kprobe.c is ftrace's
kprobe user interface, just one implementation of kprobe usage. So please
do not mix it up. It will result in a huge mess to me.
Thank you,
--
Masami Hiramatsu <mhira...@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 1/5] add infrastructure for tagging functions as error injectable
dule_unload_kprobe_ei_list(struct module *m) {}
> +#endif
> +
> /* Module notifier call back, checking kprobes on the module */
> static int kprobes_module_callback(struct notifier_block *nb,
> unsigned long val, void *data)
> @@ -2178,6 +2279,11 @@ static int kprobes_module_callback(struct
> notifier_block *nb,
> unsigned int i;
> int checkcore = (val == MODULE_STATE_GOING);
>
> + if (val == MODULE_STATE_COMING)
> + module_load_kprobe_ei_list(mod);
> + else if (val == MODULE_STATE_GOING)
> + module_unload_kprobe_ei_list(mod);
> +
> if (val != MODULE_STATE_GOING && val != MODULE_STATE_LIVE)
> return NOTIFY_DONE;
>
> @@ -2240,6 +2346,8 @@ static int __init init_kprobes(void)
> pr_err("Please take care of using kprobes.\n");
> }
>
> + populate_kernel_kprobe_ei_list();
> +
> if (kretprobe_blacklist_size) {
> /* lookup the function address from its name */
> for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
> @@ -2407,6 +2515,56 @@ static const struct file_operations
> debugfs_kprobe_blacklist_ops = {
> .release= seq_release,
> };
>
> +/*
> + * kprobes/error_injection_list -- shows which functions can be overriden for
> + * error injection.
> + * */
> +static void *kprobe_ei_seq_start(struct seq_file *m, loff_t *pos)
> +{
> + mutex_lock(_ei_mutex);
> + return seq_list_start(_error_injection_list, *pos);
> +}
> +
> +static void kprobe_ei_seq_stop(struct seq_file *m, void *v)
> +{
> + mutex_unlock(_ei_mutex);
> +}
> +
> +static void *kprobe_ei_seq_next(struct seq_file *m, void *v, loff_t *pos)
> +{
> + return seq_list_next(v, _error_injection_list, pos);
> +}
> +
> +static int kprobe_ei_seq_show(struct seq_file *m, void *v)
> +{
> + char buffer[KSYM_SYMBOL_LEN];
> + struct kprobe_ei_entry *ent =
> + list_entry(v, struct kprobe_ei_entry, list);
> +
> + sprint_symbol(buffer, ent->start_addr);
> + seq_printf(m, "%s\n", buffer);
> + return 0;
> +}
> +
> +static const struct seq_operations kprobe_ei_seq_ops = {
> + .start = kprobe_ei_seq_start,
> + .next = kprobe_ei_seq_next,
> + .stop = kprobe_ei_seq_stop,
> + .show = kprobe_ei_seq_show,
> +};
> +
> +static int kprobe_ei_open(struct inode *inode, struct file *filp)
> +{
> + return seq_open(filp, _ei_seq_ops);
> +}
> +
> +static const struct file_operations debugfs_kprobe_ei_ops = {
> + .open = kprobe_ei_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release= seq_release,
> +};
> +
> static void arm_all_kprobes(void)
> {
> struct hlist_head *head;
> @@ -2548,6 +2706,11 @@ static int __init debugfs_kprobe_init(void)
> if (!file)
> goto error;
>
> + file = debugfs_create_file("error_injection_list", 0444, dir, NULL,
> + _kprobe_ei_ops);
> + if (!file)
> + goto error;
> +
> return 0;
>
> error:
> diff --git a/kernel/module.c b/kernel/module.c
> index dea01ac9cb74..bd695bfdc5c4 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3118,7 +3118,11 @@ static int find_module_sections(struct module *mod,
> struct load_info *info)
>sizeof(*mod->ftrace_callsites),
>>num_ftrace_callsites);
> #endif
> -
> +#ifdef CONFIG_BPF_KPROBE_OVERRIDE
> + mod->kprobe_ei_funcs = section_objs(info, "_kprobe_error_inject_list",
> + sizeof(*mod->kprobe_ei_funcs),
> + >num_kprobe_ei_funcs);
> +#endif
> mod->extable = section_objs(info, "__ex_table",
> sizeof(*mod->extable), >num_exentries);
>
> --
> 2.7.5
>
--
Masami Hiramatsu <mhira...@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 3/5] bpf: add a bpf_override_function helper
*/
> + if (__this_cpu_read(bpf_kprobe_override)) {
> + __this_cpu_write(bpf_kprobe_override, 0);
> + reset_current_kprobe();
OK, I will fix this issue(reset kprobe and preempt-enable) by removing
jprobe soon.
(currently waiting for removing {tcp,sctp,dccp}_probe code, which are
only users of jprobe in the kernel)
Thank you,
> + return 1;
> + }
> + if (!ret)
> + return 0;
> + }
>
> head = this_cpu_ptr(call->perf_events);
> if (hlist_empty(head))
> - return;
> + return 0;
>
> dsize = __get_data_size(>tp, regs);
> __size = sizeof(*entry) + tk->tp.size + dsize;
> @@ -1193,13 +1232,14 @@ kprobe_perf_func(struct trace_kprobe *tk, struct
> pt_regs *regs)
>
> entry = perf_trace_buf_alloc(size, NULL, );
> if (!entry)
> - return;
> + return 0;
>
> entry->ip = (unsigned long)tk->rp.kp.addr;
> memset([1], 0, dsize);
> store_trace_args(sizeof(*entry), >tp, regs, (u8 *)[1], dsize);
> perf_trace_buf_submit(entry, size, rctx, call->event.type, 1, regs,
> head, NULL);
> + return 0;
> }
> NOKPROBE_SYMBOL(kprobe_perf_func);
>
> @@ -1275,16 +1315,24 @@ static int kprobe_register(struct trace_event_call
> *event,
> static int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs)
> {
> struct trace_kprobe *tk = container_of(kp, struct trace_kprobe, rp.kp);
> + int ret = 0;
>
> raw_cpu_inc(*tk->nhit);
>
> if (tk->tp.flags & TP_FLAG_TRACE)
> kprobe_trace_func(tk, regs);
> #ifdef CONFIG_PERF_EVENTS
> - if (tk->tp.flags & TP_FLAG_PROFILE)
> - kprobe_perf_func(tk, regs);
> + if (tk->tp.flags & TP_FLAG_PROFILE) {
> + ret = kprobe_perf_func(tk, regs);
> + /*
> + * The ftrace kprobe handler leaves it up to us to re-enable
> + * preemption here before returning if we've modified the ip.
> + */
> + if (ret)
> + preempt_enable_no_resched();
> + }
> #endif
> - return 0; /* We don't tweek kernel, so just return 0 */
> + return ret;
> }
> NOKPROBE_SYMBOL(kprobe_dispatcher);
>
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index fb66e3eaa192..5e54d748c84c 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -252,6 +252,8 @@ struct symbol_cache;
> unsigned long update_symbol_cache(struct symbol_cache *sc);
> void free_symbol_cache(struct symbol_cache *sc);
> struct symbol_cache *alloc_symbol_cache(const char *sym, long offset);
> +int trace_kprobe_ftrace(struct trace_event_call *call);
> +int trace_kprobe_error_injectable(struct trace_event_call *call);
> #else
> /* uprobes do not support symbol fetch methods */
> #define fetch_symbol_u8 NULL
> @@ -277,6 +279,16 @@ alloc_symbol_cache(const char *sym, long offset)
> {
> return NULL;
> }
> +
> +static inline int trace_kprobe_ftrace(struct trace_event_call *call)
> +{
> + return 0;
> +}
> +
> +static inline int trace_kprobe_error_injectable(struct trace_event_call
> *call)
> +{
> + return 0;
> +}
> #endif /* CONFIG_KPROBE_EVENTS */
>
> struct probe_arg {
> --
> 2.7.5
>
--
Masami Hiramatsu <mhira...@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 1/5] add infrastructure for tagging functions as error injectable
ror_injection_list, list) {
> + if (ent->priv == mod) {
> + list_del_init(>list);
> + kfree(ent);
> + }
> + }
> + mutex_unlock(_ei_mutex);
> +}
> +#else
> +static inline void __init populate_kernel_kprobe_ei_list(void) {}
> +static inline void module_load_kprobe_ei_list(struct module *m) {}
> +static inline void module_unload_kprobe_ei_list(struct module *m) {}
> +#endif
> +
> /* Module notifier call back, checking kprobes on the module */
> static int kprobes_module_callback(struct notifier_block *nb,
> unsigned long val, void *data)
> @@ -2178,6 +2279,11 @@ static int kprobes_module_callback(struct
> notifier_block *nb,
> unsigned int i;
> int checkcore = (val == MODULE_STATE_GOING);
>
> + if (val == MODULE_STATE_COMING)
> + module_load_kprobe_ei_list(mod);
> + else if (val == MODULE_STATE_GOING)
> + module_unload_kprobe_ei_list(mod);
> +
> if (val != MODULE_STATE_GOING && val != MODULE_STATE_LIVE)
> return NOTIFY_DONE;
>
> @@ -2240,6 +2346,8 @@ static int __init init_kprobes(void)
> pr_err("Please take care of using kprobes.\n");
> }
>
> + populate_kernel_kprobe_ei_list();
> +
> if (kretprobe_blacklist_size) {
> /* lookup the function address from its name */
> for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
> @@ -2407,6 +2515,56 @@ static const struct file_operations
> debugfs_kprobe_blacklist_ops = {
> .release= seq_release,
> };
>
> +/*
> + * kprobes/error_injection_list -- shows which functions can be overriden for
> + * error injection.
> + * */
> +static void *kprobe_ei_seq_start(struct seq_file *m, loff_t *pos)
> +{
> + mutex_lock(_ei_mutex);
> + return seq_list_start(_error_injection_list, *pos);
> +}
> +
> +static void kprobe_ei_seq_stop(struct seq_file *m, void *v)
> +{
> + mutex_unlock(_ei_mutex);
> +}
> +
> +static void *kprobe_ei_seq_next(struct seq_file *m, void *v, loff_t *pos)
> +{
> + return seq_list_next(v, _error_injection_list, pos);
> +}
> +
> +static int kprobe_ei_seq_show(struct seq_file *m, void *v)
> +{
> + char buffer[KSYM_SYMBOL_LEN];
> + struct kprobe_ei_entry *ent =
> + list_entry(v, struct kprobe_ei_entry, list);
> +
> + sprint_symbol(buffer, ent->start_addr);
> + seq_printf(m, "%s\n", buffer);
> + return 0;
> +}
> +
> +static const struct seq_operations kprobe_ei_seq_ops = {
> + .start = kprobe_ei_seq_start,
> + .next = kprobe_ei_seq_next,
> + .stop = kprobe_ei_seq_stop,
> + .show = kprobe_ei_seq_show,
> +};
> +
> +static int kprobe_ei_open(struct inode *inode, struct file *filp)
> +{
> + return seq_open(filp, _ei_seq_ops);
> +}
> +
> +static const struct file_operations debugfs_kprobe_ei_ops = {
> + .open = kprobe_ei_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release= seq_release,
> +};
> +
> static void arm_all_kprobes(void)
> {
> struct hlist_head *head;
> @@ -2548,6 +2706,11 @@ static int __init debugfs_kprobe_init(void)
> if (!file)
> goto error;
>
> + file = debugfs_create_file("error_injection_list", 0444, dir, NULL,
> + _kprobe_ei_ops);
> + if (!file)
> + goto error;
> +
> return 0;
>
> error:
> diff --git a/kernel/module.c b/kernel/module.c
> index dea01ac9cb74..bd695bfdc5c4 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3118,7 +3118,11 @@ static int find_module_sections(struct module *mod,
> struct load_info *info)
>sizeof(*mod->ftrace_callsites),
>>num_ftrace_callsites);
> #endif
> -
> +#ifdef CONFIG_BPF_KPROBE_OVERRIDE
> + mod->kprobe_ei_funcs = section_objs(info, "_kprobe_error_inject_list",
> + sizeof(*mod->kprobe_ei_funcs),
> + >num_kprobe_ei_funcs);
> +#endif
> mod->extable = section_objs(info, "__ex_table",
> sizeof(*mod->extable), >num_exentries);
>
> --
> 2.7.5
>
--
Masami Hiramatsu <mhira...@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 0/5] Add the ability to do BPF directed error injection
xa0/0xa0
> > > > [ 1847.801616] ? remove_wait_queue+0x60/0x60
> > > > [ 1847.802040] ? rcu_preempt_qs+0xa0/0xa0
> > > > [ 1847.802433] ? rcu_exp_wait_wake+0x630/0x630
> > > > [ 1847.802872] ? __lock_acquire+0xfb9/0x1120
> > > > [ 1847.803302] ? __lock_acquire+0x534/0x1120
> > > > [ 1847.803725] ? bdi_unregister+0x57/0x1a0
> > > > [ 1847.804135] bdi_unregister+0x5c/0x1a0
> > > > [ 1847.804519] bdi_put+0xcb/0xe0
> > > > [ 1847.804746] generic_shutdown_super+0xe2/0x110
> > > > [ 1847.805066] kill_anon_super+0xe/0x20
> > > > [ 1847.805344] btrfs_kill_super+0x12/0xa0 [btrfs]
> > > > [ 1847.805664] deactivate_locked_super+0x34/0x60
> > > > [ 1847.806111] btrfs_mount+0xbb6/0x1000 [btrfs]
> > > > [ 1847.806476] ? __lockdep_init_map+0x5c/0x1d0
> > > > [ 1847.806824] ? mount_fs+0xf/0x80
> > > > [ 1847.807104] ? alloc_vfsmnt+0x1a1/0x230
> > > > [ 1847.807416] mount_fs+0xf/0x80
> > > > [ 1847.807712] vfs_kern_mount+0x62/0x160
> > > > [ 1847.808112] btrfs_mount+0x3d3/0x1000 [btrfs]
> > > > [ 1847.808565] ? __lockdep_init_map+0x5c/0x1d0
> > > > [ 1847.809005] ? __lockdep_init_map+0x5c/0x1d0
> > > > [ 1847.809425] ? mount_fs+0xf/0x80
> > > > [ 1847.809731] mount_fs+0xf/0x80
> > > > [ 1847.810070] vfs_kern_mount+0x62/0x160
> > > > [ 1847.810469] do_mount+0x1b1/0xd50
> > > > [ 1847.810821] ? _copy_from_user+0x5b/0x90
> > > > [ 1847.811237] ? memdup_user+0x4b/0x70
> > > > [ 1847.811622] SyS_mount+0x85/0xd0
> > > > [ 1847.811996] entry_SYSCALL_64_fastpath+0x1f/0x96
> > > > [ 1847.812465] RIP: 0033:0x7f6ebecc1b5a
> > > > [ 1847.812840] RSP: 002b:7ffc7bd1c958 EFLAGS: 0202 ORIG_RAX:
> > > > 00a5
> > > > [ 1847.813615] RAX: ffda RBX: 7f6ebefba63a RCX:
> > > > 7f6ebecc1b5a
> > > > [ 1847.814302] RDX: 00bfd010 RSI: 00bfa230 RDI:
> > > > 00bfa210
> > > > [ 1847.814770] RBP: 00bfa0f0 R08: R09:
> > > > 0014
> > > > [ 1847.815246] R10: c0ed R11: 0202 R12:
> > > > 7f6ebf1ca83c
> > > > [ 1847.815720] R13: R14: R15:
> > > > 0001
> > > i>
> > >
> > > Looks like this is new, Masami this is happening because of your change
> > > here
> > >
> > > 5bb4fc2d8641 ("kprobes/x86: Disable preemption in ftrace-based jprobes")
> > >
> > > which makes it not do the preempt_enable() if the handler returns 1. Why
> > > is
> > > that?
Yes, because this (return 1) is expected to be done only by jprobe.
> > > Should I be doing preempt_enable_no_resched() from the handler before
> > > returning 1? Or is this just an oversight on your part? Thanks,
Yes, or you have to hook after return path and fixup preempt count as
jprobe did.
(And now jprobe is coming to an end.)
> >
> > FWIW I shut up the preemption imbalance warnings with the attached
> > coarse bandaid. No idea if that's the correct fix...
No, this is not correct way to fix this issue.
I guess your BPF extention is trying to change instrunction pointer to
another address (right?). If so, you have to carefully do followings
before returning to modified address.
- reset current_kprobes
- call preempt_enable_no_resched()
> >
> > --D
> >
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index 5db8498..fd948e3 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -1215,8 +1215,10 @@ kprobe_perf_func(struct trace_kprobe *tk, struct
> > pt_regs *regs)
> > if (__this_cpu_read(bpf_kprobe_override)) {
> > __this_cpu_write(bpf_kprobe_override, 0);
> > reset_current_kprobe();
> > + preempt_enable();
> > return 1;
> > }
> > + preempt_enable();
> > if (!ret)
> > return 0;
> > }
>
> Yeah I'd like to avoid doing this and know why exactly we leave a unpaired
> preempt_disable() in kprobe_ftrace_handler() so we don't do something like
> this
> only to have the handler change again in the future and break us again.
> Thanks,
Ah, I see. kprobe_perf_func invokes BPF and BPF changes instruction address.
In that case, only what you need is adding preempt_enable_no_resched() at
right after the reset_current_kprobe().
Anyway, Could you CC the series to me?
Thank you,
--
Masami Hiramatsu <mhira...@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
