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@xxxxxxxxxx> 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@xxxxxxxxxx>

Thank you!

> 
> Signed-off-by: Cheng Jian <cj.chengjian@xxxxxxxxxx>
> Reported-by: Kong ZhangHuan <kongzhanghuan@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx>