linux-kernel - Re: [PATCH 18/18] tracing/kprobes: Dump the culprit kprobe in case of kprobe recursion

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 27 Aug 2009 17:34:49 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Masami Hiramatsu <mhiramat@...hat.com>
Cc:	Ingo Molnar <mingo@...e.hu>, LKML <linux-kernel@...r.kernel.org>,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>
Subject: Re: [PATCH 18/18] tracing/kprobes: Dump the culprit kprobe in case
	of kprobe recursion

On Thu, Aug 27, 2009 at 11:30:24AM -0400, Masami Hiramatsu wrote:
> Hi Frederic,
> 
> Frederic Weisbecker wrote:
> > Kprobes can enter into a probing recursion, ie: a kprobe that does an
> > endless loop because one of its core mechanism function used during
> > probing is also probed itself.
> > 
> > This patch helps pinpointing the kprobe that raised such recursion
> > by dumping it and raising a BUG instead of a warning (we also disarm
> > the kprobe to try avoiding recursion in BUG itself). Having a BUG
> > instead of a warning stops the stacktrace in the right place and
> > doesn't pollute the logs with hundreds of traces that eventually end
> > up in a stack overflow.
> 
> Thanks, but I also found similar bug cases.
> 
> > 
> > Signed-off-by: Frederic Weisbecker<fweisbec@...il.com>
> > Cc: Masami Hiramatsu<mhiramat@...hat.com>
> > Cc: Ananth N Mavinakayanahalli<ananth@...ibm.com>
> > ---
> >   arch/x86/kernel/kprobes.c |    8 ++++++--
> >   include/linux/kprobes.h   |    2 ++
> >   kernel/kprobes.c          |    7 +++++++
> >   3 files changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
> > index 16ae961..ecee3d2 100644
> > --- a/arch/x86/kernel/kprobes.c
> > +++ b/arch/x86/kernel/kprobes.c
> > @@ -490,9 +490,13 @@ static int __kprobes reenter_kprobe(struct kprobe *p, struct pt_regs *regs,
> 
> Before this, kprobes checks p != kprobe_running(), but it's a
> meaningless branch. Hitting a kprobe while KPROBES_HIT_SS always
> treated as unrecoverable.



Yeah, but that's the place where a probe ends up when bad reentrancy happens
right?


 
> >   			/* A probe has been hit in the codepath leading up
> >   			 * to, or just after, single-stepping of a probed
> >   			 * instruction. This entire codepath should strictly
> > -			 * reside in .kprobes.text section. Raise a warning
> > -			 * to highlight this peculiar case.
> > +			 * reside in .kprobes.text section.
> > +			 * Raise a BUG or we'll continue in an endless
> > +			 * reentering loop and eventually a stack overflow.
> >   			 */
> > +			arch_disarm_kprobe(p);
> > +			dump_kprobe(p);
> > +			BUG();
> >   		}
> >   	default:
> >   		/* impossible cases */
> > diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> > index bcd9c07..87eb79c 100644
> > --- a/include/linux/kprobes.h
> > +++ b/include/linux/kprobes.h
> > @@ -296,6 +296,8 @@ void recycle_rp_inst(struct kretprobe_instance *ri, struct hlist_head *head);
> >   int disable_kprobe(struct kprobe *kp);
> >   int enable_kprobe(struct kprobe *kp);
> > 
> > +void dump_kprobe(struct kprobe *kp);
> > +
> >   #else /* !CONFIG_KPROBES: */
> > 
> >   static inline int kprobes_built_in(void)
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index ef177d6..f72e96c 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -1141,6 +1141,13 @@ static void __kprobes kill_kprobe(struct kprobe *p)
> >   	arch_remove_kprobe(p);
> >   }
> > 
> > +void __kprobes dump_kprobe(struct kprobe *kp)
> > +{
> > +	printk(KERN_WARNING "Dumping kprobe:\n");
> > +	printk(KERN_WARNING "Name: %s\nAddress: %p\nOffset: %x\n",
> > +	       kp->symbol_name, kp->addr, kp->offset);
> > +}
> 
> Since kp->symbol_name + kp->offset = kp->addr, I recommend to show it
> as "Kprobe at %s+%x:<%p>\n", kp->symbol_name, kp->offset, kp->addr.


Ok I'll fix this, thanks.

 
> > +
> >   /* Module notifier call back, checking kprobes on the module */
> >   static int __kprobes kprobes_module_callback(struct notifier_block *nb,
> >   					     unsigned long val, void *data)
> 
> Thank you,
> 
> -- 
> Masami Hiramatsu
> 
> Software Engineer
> Hitachi Computer Products (America), Inc.
> Software Solutions Division
> 
> e-mail: mhiramat@...hat.com
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/