From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932444AbdJZOjt (ORCPT ); Thu, 26 Oct 2017 10:39:49 -0400 Received: from mail.kernel.org ([198.145.29.99]:45368 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932255AbdJZOjs (ORCPT ); Thu, 26 Oct 2017 10:39:48 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E75C4218AC 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 23:39:44 +0900 From: Masami Hiramatsu To: Zhou Chengming Cc: , , , Subject: Re: [PATCH] kprobes: avoid the kprobe being re-registered Message-Id: <20171026233944.d56f558c01275e70744c2add@kernel.org> In-Reply-To: <1509019885-58481-1-git-send-email-zhouchengming1@huawei.com> References: <1509019885-58481-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 Thu, 26 Oct 2017 20:11:25 +0800 Zhou Chengming wrote: > 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. Still no good, see below comment. > > Signed-off-by: Zhou Chengming > --- > kernel/kprobes.c | 23 ++++++----------------- > 1 file changed, 6 insertions(+), 17 deletions(-) > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index a1606a4..2a4873a 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; > @@ -1536,10 +1523,6 @@ int register_kprobe(struct kprobe *p) > return PTR_ERR(addr); > p->addr = addr; > > - ret = check_kprobe_rereg(p); > - if (ret) > - return ret; > - > /* User can pass only KPROBE_FLAG_DISABLED to register_kprobe */ > p->flags &= KPROBE_FLAG_DISABLED; > p->nmissed = 0; here, we already modifies the kprobe. We need to check and reject before modifying it. Thank you, > @@ -1551,6 +1534,12 @@ int register_kprobe(struct kprobe *p) > > mutex_lock(&kprobe_mutex); > > + /* Return error if the kprobe is being re-registered */ > + if (__get_valid_kprobe(p)) { > + ret = -EINVAL; > + goto out; > + } > + > old_p = get_kprobe(p->addr); > if (old_p) { > /* Since this may unoptimize old_p, locking text_mutex. */ > -- > 1.8.3.1 > -- Masami Hiramatsu