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

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

[Cheng Jian]

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 <cj.chengjian@huawei.com>
Reported-by: Kong ZhangHuan <kongzhanghuan@huawei.com>
---
 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

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

[chengjian (D)]

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

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

[Masami Hiramatsu]

On Wed, 13 Dec 2017 20:27:21 +0800
Cheng Jian <cj.chengjian@huawei.com> 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 <mhiramat@kernel.org>

Thank you!

> 
> Signed-off-by: Cheng Jian <cj.chengjian@huawei.com>
> Reported-by: Kong ZhangHuan <kongzhanghuan@huawei.com>
> ---
>  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 <mhiramat@kernel.org>