From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752023AbdJYQzH (ORCPT ); Wed, 25 Oct 2017 12:55:07 -0400 Received: from mail.kernel.org ([198.145.29.99]:59692 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751933AbdJYQy5 (ORCPT ); Wed, 25 Oct 2017 12:54:57 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 579FF218AC Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=mhiramat@kernel.org Date: Thu, 26 Oct 2017 01:54:53 +0900 From: Masami Hiramatsu To: JianKang Chen Cc: , , , , Subject: Re: [PATCH] kernel/kprobes: add check to avoid kprobe memory leak Message-Id: <20171026015453.ba08699319ffaa9a18bbd8e3@kernel.org> In-Reply-To: <1508847422-63641-1-git-send-email-chenjiankang1@huawei.com> References: <1508847422-63641-1-git-send-email-chenjiankang1@huawei.com> X-Mailer: Sylpheed 3.5.0 (GTK+ 2.24.30; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 24 Oct 2017 20:17:02 +0800 JianKang Chen wrote: > 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}; What ? this is buggy code. you must not list same kretprobe. But, year, since register_kprobe() already has similar protection against reusing, register_kretprobe() should do so. [..] > raw_spin_lock_init(&rp->lock); > + > + if (!hlist_empty(&rp->free_instances)) > + return -EBUSY; > + Hmm, but can you use check_kprobe_rereg() before raw_spin_lock_init()? If user reuses rp after it starts, rp->lock can already be used. Thank you, > INIT_HLIST_HEAD(&rp->free_instances); > for (i = 0; i < rp->maxactive; i++) { > inst = kmalloc(sizeof(struct kretprobe_instance) + > -- > 1.7.12.4 > -- Masami Hiramatsu