From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752367AbdJ0F52 (ORCPT ); Fri, 27 Oct 2017 01:57:28 -0400 Received: from mail.kernel.org ([198.145.29.99]:54148 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751399AbdJ0F5Z (ORCPT ); Fri, 27 Oct 2017 01:57:25 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2078D21923 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: Fri, 27 Oct 2017 14:57:21 +0900 From: Masami Hiramatsu To: Zhou Chengming Cc: , , , Subject: Re: [PATCH v2] kprobes: avoid the kprobe being re-registered Message-Id: <20171027145721.ef28a8eea41210f05c23f30e@kernel.org> In-Reply-To: <1509069400-133557-1-git-send-email-zhouchengming1@huawei.com> References: <1509069400-133557-1-git-send-email-zhouchengming1@huawei.com> X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.31; x86_64-redhat-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 Fri, 27 Oct 2017 09:56:40 +0800 Zhou Chengming wrote: > Changes from v1: > - We should put the modifies of the kprobe after the re-reg check. > - And then the address_safe check. > - When check_kprobe_address_safe() return fail, the *probed_mod > should be set to NULL, and no module refcount held. Could you split this item from this patch (with initializing probe_mod = NULL), since it is another bug? Thank you, > > Old code use check_kprobe_rereg() to check if the kprobe has been > registered already, but check_kprobe_rereg() will release the > kprobe_mutex then, so maybe two paths will pass the check and > register the same kprobe. This patch put the check inside the mutex. > > Signed-off-by: Zhou Chengming > --- > kernel/kprobes.c | 28 +++++++++------------------- > 1 file changed, 9 insertions(+), 19 deletions(-) > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index a1606a4..f622639 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -1443,19 +1443,6 @@ static struct kprobe *__get_valid_kprobe(struct kprobe *p) > return ap; > } > > -/* Return error if the kprobe is being re-registered */ > -static inline int check_kprobe_rereg(struct kprobe *p) > -{ > - int ret = 0; > - > - mutex_lock(&kprobe_mutex); > - if (__get_valid_kprobe(p)) > - ret = -EINVAL; > - mutex_unlock(&kprobe_mutex); > - > - return ret; > -} > - > int __weak arch_check_ftrace_location(struct kprobe *p) > { > unsigned long ftrace_addr; > @@ -1501,6 +1488,7 @@ static int check_kprobe_address_safe(struct kprobe *p, > * its code to prohibit unexpected unloading. > */ > if (unlikely(!try_module_get(*probed_mod))) { > + *probed_mod = NULL; > ret = -ENOENT; > goto out; > } > @@ -1536,9 +1524,13 @@ int register_kprobe(struct kprobe *p) > return PTR_ERR(addr); > p->addr = addr; > > - ret = check_kprobe_rereg(p); > - if (ret) > - return ret; > + mutex_lock(&kprobe_mutex); > + > + /* Return error if the kprobe is being re-registered */ > + if (__get_valid_kprobe(p)) { > + ret = -EINVAL; > + goto out; > + } > > /* User can pass only KPROBE_FLAG_DISABLED to register_kprobe */ > p->flags &= KPROBE_FLAG_DISABLED; > @@ -1547,9 +1539,7 @@ int register_kprobe(struct kprobe *p) > > ret = check_kprobe_address_safe(p, &probed_mod); > if (ret) > - return ret; > - > - mutex_lock(&kprobe_mutex); > + goto out; > > old_p = get_kprobe(p->addr); > if (old_p) { > -- > 1.8.3.1 > -- Masami Hiramatsu