linux-kernel - [PATCH] kernel/kprobes: add check to avoid kprobe memory leak

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <1508847812-64041-1-git-send-email-chenjiankang1@huawei.com>
Date:   Tue, 24 Oct 2017 20:23:32 +0800
From:   JianKang Chen <chenjiankang1@...wei.com>
To:     <ananth@...ux.vnet.ibm.com>, <anil.s.keshavamurthy@...el.com>,
        <mhiramat@...nel.org>, <davem@...emloft.net>,
        <linux-kernel@...r.kernel.org>
CC:     <xieyisheng1@...wei.com>, <wangkefeng.wang@...wei.com>,
        <chenjiankang1@...wei.com>
Subject: [PATCH] kernel/kprobes: add check to avoid kprobe memory leak

The function register_kretprobe is used to initialize a struct
kretprobe and allocate a list table for kprobe instance.
However,in this function, there is a memory leak.

The test case:

static struct kretprobe rp;
struct  kretprobe *rps[10]={&rp ,&rp ,&rp ,
&rp ,&rp ,&rp ,&rp ,&rp ,&rp,&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;
        rp.kp.addr = (kprobe_opcode_t *)kallsyms_lookup_name("do_fork");
        rp.handler=ret_handler;
        rp.entry_handler=entry_handler;
        rp.maxactive = 1;

        ret = register_kretprobes(rps,10);
        if (ret < 0)
        {
                return -1;
        }

Result:

unreferenced object 0xffff8012aea35500 (size 64):
  comm "insmod", pid 31966, jiffies 4309153380 (age 111.356s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 85 37 fc ff 7f ff ff  ..........7.....
    08 55 a3 ae 12 80 ff ff 01 00 00 00 00 00 00 00  .U..............
  backtrace:
    [<ffff8000002cd880>] create_object+0x1e0/0x3f0
    [<ffff800000fa47ac>] kmemleak_alloc+0x6c/0xf0
    [<ffff8000002ac97c>] __kmalloc+0x23c/0x2e0
    [<ffff8000001a6f2c>] register_kretprobe+0x12c/0x350
    [<ffff8000001a7198>] register_kretprobes+0x48/0xa0
    [<ffff7ffffc380058>] 0xffff7ffffc380058
    [<ffff800000084be0>] do_one_initcall+0x120/0x260
    [<ffff800000faa1a4>] do_init_module+0xf0/0x288
    [<ffff8000001736f4>] load_module+0x1824/0x1b50
    [<ffff800000173cc8>] SyS_finit_module+0xc8/0x100
    [<ffff800000084778>] __sys_trace_return+0x0/0x4
    [<ffffffffffffffff>] 0xffffffffffffffff

when there are two same struct kretprobe rp,
the INIT_HLIST_HEAD() will result
in a empty list table rp->free_instances.
The memory leak will happen.
So it needs to check the list table before INIT_HLIST_HEAD().

Reported-and-tested-by: Wen Kang <kangwen1@...wei.com>
Signed-off-by: JianKang Chen <chenjiankang1@...wei.com>
Signed-off-by: xie yisheng <xieyisheng1@...wei.com>
Signed-off-by: Bixuan Cui <cuibixuan@...wei.com>
---
 kernel/kprobes.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index a1606a4..8b3330c 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1948,6 +1948,10 @@ int register_kretprobe(struct kretprobe *rp)
 #endif
 	}
 	raw_spin_lock_init(&rp->lock);
+
+	if (!hlist_empty(&rp->free_instances))
+		return -EBUSY;
+
 	INIT_HLIST_HEAD(&rp->free_instances);
 	for (i = 0; i < rp->maxactive; i++) {
 		inst = kmalloc(sizeof(struct kretprobe_instance) +
-- 
1.7.12.4