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]

Message-ID: <4A96B821.40408@redhat.com>
Date:	Thu, 27 Aug 2009 12:45:21 -0400
From:	Masami Hiramatsu <mhiramat@...hat.com>
To:	Frederic Weisbecker <fweisbec@...il.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

Frederic Weisbecker wrote:
> On Thu, Aug 27, 2009 at 11:52:09AM -0400, Masami Hiramatsu wrote:
>> Frederic Weisbecker wrote:
>>> 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?
>>
>> No, a place which is shared by kprobes and other subsystems, will cause a
>> problem.
>>
>> for example, I found an irq_return case which will be p == kprobe_running()
>> on x86-64.
>>
>> ->  <some irq occurs>
>>   ->  irq_return
>>     ->  <hit int3>
>>       ->  do_int3
>>       ->  <handling kprobe (set kprobe_running)>
>>     ->  irq_return (from do_int3)
>>       ->  <hit int3>
>>         ->  do_int3
>>         <handling kprobe (kprobe_running == p)>  <- here!
>>
>
>
> Oh right.
>
>
>> Perhaps, the original code assumes that it will be caused by an int3
>> which another subsystem inserted on out-of-line singlestep buffer
>> if the hitting probe is same as current probe.
>>
>> However, in that case, int3 hitting address is on the out-of-line
>> buffer and should be different from first (current) int3 address.
>
>
> I see.
>
>
>> So, I think this part should also be removed.
>>
>>                if (p == kprobe_running()) {
>>                        regs->flags&= ~X86_EFLAGS_TF;
>>                        regs->flags |= kcb->kprobe_saved_flags;
>>                        return 0;
>>                } else {
>>
>> Thank you,
>
>
> So my patch is useless? Or is it also useful to detect real
> recursion? (despite of such corner cases)

Your patch is still useful! I'd like to suggest a bugfix :-).
Anyway, I'll send an update patch.

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/