2017-12-13 12:22:11

by Cheng Jian

[permalink] [raw]
Subject: [PATCH] kprobe : fix out-of-bounds in register_kretprobe when parsing negative data_size

When we register kretprobe, data_size used to allocate space
for storing per-instance private data.

If we use a negative values as data_size, It will register
successfully, then cause slab-out-of-bounds which can be
found by KASAN.

The call trace is like :

=============================================================
BUG: KASAN: slab-out-of-bounds in trampoline_probe_handler
+0xb4/0x2f0 at addr ffff8000b732a7a0
Read of size 8 by task sh/1945
=============================================================
BUG kmalloc-64 (Tainted: G B W OE ):
kasan: bad access detected
-------------------------------------------------------------
INFO: Allocated in register_kretprobe+0x12c/0x350
age=157 cpu=4 pid=1947
......
INFO: Freed in do_one_initcall+0x110/0x260
age=169 cpu=4 pid=1947
......
INFO: Slab 0xffff7bffc2dcca80 objects=21 used=10
fp=0xffff8000b732aa80 flags=0x7fff00000004080
INFO: Object 0xffff8000b732a780 @offset=1920 fp=0x (null)

CPU: 7 PID: 1945 Comm: sh Tainted: G B W OE 4.1.46 #8
Hardware name: linux,dummy-virt (DT)
Call trace:
[<0008d2a0>] dump_backtrace+0x0/0x220
[<0008d4e0>] show_stack+0x20/0x30
[<00ff2278>] dump_stack+0xa8/0xcc
[<002dc6c8>] print_trailer+0xf8/0x160
[<002e20d8>] object_err+0x48/0x60
[<002e48dc>] kasan_report+0x26c/0x5a0
[<002e39a0>] __asan_load8+0x60/0x80
[<01000054>] trampoline_probe_handler+0xb4/0x2f0
[<00ffff38>] kretprobe_trampoline+0x54/0xbc
Memory state around the buggy address:
b732a680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
b732a700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>b732a780: 00 00 00 00 07 fc fc fc fc fc fc fc fc fc fc fc
^

If data_size is invalid, then we should not register it.

Signed-off-by: Cheng Jian <[email protected]>
Reported-by: Kong ZhangHuan <[email protected]>
---
kernel/kprobes.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index da2ccf1..8002f28 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1924,6 +1924,9 @@ int register_kretprobe(struct kretprobe *rp)
int i;
void *addr;

+ if ((ssize_t)rp->data_size < 0)
+ return -EINVAL;
+
if (!kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset))
return -EINVAL;

--
1.8.3.1


2017-12-13 12:25:44

by Cheng Jian

[permalink] [raw]
Subject: Re: [PATCH] kprobe : fix out-of-bounds in register_kretprobe when parsing negative data_size

Hi

The demo is like:

#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/kprobes.h>
#include <linux/version.h>

static int data_size=0;
module_param(data_size, int, 0644);

static struct kretprobe rp;

static int ret_handler(struct kretprobe_instance *ri, struct pt_regs *regs)
{
printk(KERN_DEBUG "ret_handler\n");
return 0;
}
static int entry_handler(struct kretprobe_instance *ri, struct pt_regs
*regs)
{
printk(KERN_DEBUG "entry_handler\n");
return 0;
}
static int __init kretprobe_init(void)
{
int ret;
printk(KERN_DEBUG"size = %ld\n", sizeof(struct kretprobe_instance)
- 1);
#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,2,0)
rp.kp.addr = (kprobe_opcode_t *)kallsyms_lookup_name("_do_fork");
#else
rp.kp.addr = (kprobe_opcode_t *)kallsyms_lookup_name("do_fork");
#endif

rp.handler = ret_handler;
rp.entry_handler = entry_handler;
rp.data_size = data_size;
rp.maxactive = 1;

ret = register_kretprobe(&rp);
if (ret < 0)
{
printk(KERN_DEBUG "register_kretprobe failed, returned %d\n", ret);
return -1;
}
printk(KERN_DEBUG "register_kretprobe pass\n");
return 0;
}

static void __exit kretprobe_exit(void)
{
unregister_kretprobe(&rp);
printk(KERN_DEBUG "kretprobe unregistered\n");
}

module_init(kretprobe_init)
module_exit(kretprobe_exit)
MODULE_LICENSE("GPL");





insmod this module

sudo insmod testRegKretprobe.ko data_size=-1



Thanks.
CHENG Jian


2017-12-13 13:14:39

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH] kprobe : fix out-of-bounds in register_kretprobe when parsing negative data_size

On Wed, 13 Dec 2017 20:27:21 +0800
Cheng Jian <[email protected]> wrote:

> When we register kretprobe, data_size used to allocate space
> for storing per-instance private data.
>
> If we use a negative values as data_size, It will register
> successfully, then cause slab-out-of-bounds which can be
> found by KASAN.
>
> The call trace is like :
>
> =============================================================
> BUG: KASAN: slab-out-of-bounds in trampoline_probe_handler
> +0xb4/0x2f0 at addr ffff8000b732a7a0
> Read of size 8 by task sh/1945
> =============================================================
> BUG kmalloc-64 (Tainted: G B W OE ):
> kasan: bad access detected
> -------------------------------------------------------------
> INFO: Allocated in register_kretprobe+0x12c/0x350
> age=157 cpu=4 pid=1947
> ......
> INFO: Freed in do_one_initcall+0x110/0x260
> age=169 cpu=4 pid=1947
> ......
> INFO: Slab 0xffff7bffc2dcca80 objects=21 used=10
> fp=0xffff8000b732aa80 flags=0x7fff00000004080
> INFO: Object 0xffff8000b732a780 @offset=1920 fp=0x (null)
>
> CPU: 7 PID: 1945 Comm: sh Tainted: G B W OE 4.1.46 #8
> Hardware name: linux,dummy-virt (DT)
> Call trace:
> [<0008d2a0>] dump_backtrace+0x0/0x220
> [<0008d4e0>] show_stack+0x20/0x30
> [<00ff2278>] dump_stack+0xa8/0xcc
> [<002dc6c8>] print_trailer+0xf8/0x160
> [<002e20d8>] object_err+0x48/0x60
> [<002e48dc>] kasan_report+0x26c/0x5a0
> [<002e39a0>] __asan_load8+0x60/0x80
> [<01000054>] trampoline_probe_handler+0xb4/0x2f0
> [<00ffff38>] kretprobe_trampoline+0x54/0xbc
> Memory state around the buggy address:
> b732a680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> b732a700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> >b732a780: 00 00 00 00 07 fc fc fc fc fc fc fc fc fc fc fc
> ^
>
> If data_size is invalid, then we should not register it.

Good catch!
Anyway, this influence is limited because this interface is
only exposed to custom kernel modules. So only roots can
shoot in the foot.

Acked-by: Masami Hiramatsu <[email protected]>

Thank you!

>
> Signed-off-by: Cheng Jian <[email protected]>
> Reported-by: Kong ZhangHuan <[email protected]>
> ---
> kernel/kprobes.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index da2ccf1..8002f28 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1924,6 +1924,9 @@ int register_kretprobe(struct kretprobe *rp)
> int i;
> void *addr;
>
> + if ((ssize_t)rp->data_size < 0)
> + return -EINVAL;
> +
> if (!kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset))
> return -EINVAL;
>
> --
> 1.8.3.1
>


--
Masami Hiramatsu <[email protected]>