linux-kernel - Re: [RFC][Patch 1/4] kprobe fast unregistration

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

Date:	Mon, 26 Mar 2007 12:17:49 +0900
From:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
To:	"Keshavamurthy, Anil S" <anil.s.keshavamurthy@...el.com>,
	hch@...radead.org
Cc:	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	Prasanna S Panchamukhi <prasanna@...ibm.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	SystemTAP <systemtap@...rces.redhat.com>,
	Satoshi Oshima <soshima@...hat.com>,
	Hideo Aoki <haoki@...hat.com>,
	Yumiko Sugita <yumiko.sugita.yf@...achi.com>,
	"Frank Ch. Eigler" <fche@...hat.com>
Subject: Re: [RFC][Patch 1/4] kprobe fast unregistration

Hi Christoph and Anil,

Thank you for your comments.

Christoph Hellwig wrote:
> Speeding up the unregistration is a very good idea, but this interface
> is rather horrible.  It's almost a receipe for users to get it wrong.
Keshavamurthy, Anil S wrote:
> I agree with Christop that the interface is horrible and error prone.

OK, I agree. I had chosen a confusable name.

> However, I see the use case where people want to disable the probes quickly and
> would like to reenable them again. Looking closely at your patch,
> I think this can be acheived.

Thank you.

> Here is my suggestion.
> 
>> Here is an example code.
>> --
>> struct kprobes *p;
>> for_each_probe(p) {
>> 	unregister_kprobe_fast(p);
>         ^^^^^^^^^^^^^^^^^^^^^^
> Change this to disable_kprobe(p), which is essentially the same as
> what you have implemented. And also provide an opposite function
> to reenable_kprobe(p) which enables the disabled probe again.

I'd like to change that to prepare_to_unregister_kprobe(p) instead of
disable_kprobe(p).

I think Josh and other people want interfaces to disable/reenable all
probes at once when the sysrq is pressed.
So, IMHO, these interfaces should use a global (and per-cpu?) flag which
controls whether kprobes calls user-defined handler or not, instead of
self-modifying.
For example,

if (p && p->pre_handler && kprobe_enable) {
                           ^^^^^^^^^^^^^
   ...
}

But, I think this would be another story.
I'd like to discuss this topic in other mails.

>> }
>> commit_kprobes();
>   ^^^^^^^^^^^^^^
> Change this to unregister_disabled_kprobes(), which essentially 
> unregisters all the disabled probes.

And also, I'd like to change it to unregister_prepared_kprobes().
What would you think about this idea?

Thanks,


> 
> Thanks,
> Anil Keshavamurthy
> 
-- 
Masami HIRAMATSU
Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@...achi.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/