Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
On 02/05/2014 12:57 PM, Masami Hiramatsu wrote: > (2014/02/05 12:08), Chen Gang wrote: > Anyway, I don't think those inlined functions to be changed, because > most of them are internal functions. If CONFIG_KRETPROBES=n, it just > be ignored. > In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(), disable_kretprobe(), and enable_kretprobe() are not ignored. >>> >>> Really? where are they called? I mean, those functions do not have >>> any instance unless your module uses it (but that is not what the kernel >>> itself should help). >>> >> >> If what you said correct (I guess so), for me, we still need let them in >> CONFIG_KRETPROBES area, and without any dummy outside, just like another >> *kprobe* static inline functions have done in "include/linux/kprobes.h". > > kretprobe_assert() is only for the internal check. So we don't need to > care > about, and disable/enable_kretprobe() are anyway returns -EINVAL because > kretprobe can not be registered. And all of them are inlined functions. > In that case, we don't need to care about it. Hmm... it is related with code 'consistency': - these static inline functions are kretprobe generic implementation, and we are trying to let all kretprobe generic implementation within CONFIG_KRETPROBES area. >>> >>> No, actually, kretprobe is just built on the kprobe. >>> enable/disable_kretprobe >>> just wrapped the kprobe methods. And kretprobe_assert() is just for >>> kretprobe >>> internal use. that is not an API. Moving only the kretprobe_assert() into >>> the >>> CONFIG_KRETPROBE area is not bad, but since it is just a static inline >>> function, >>> if there is no caller, it just be ignored, no side effect. >>> >> >> OK, I can understand. >> >> And do you mean enable/disable_kretprobe() are API? if so, we have to >> implement them whether CONFIG_KRETPROBES enabled or disabled. >> >> And when CONFIG_KRETPROBES=n, just like what you originally said: we >> need returns -EINVAL directly (either, I am not quite sure whether the >> input parameter will be NULL, in this case). > > Both are API, and when implementing it I had also considered that, but > I decided to stay it in inline-function wrapper. The reason why is, > that enable/disable_k*probe require the registered k*probes. If the > kernel hacker uses those functions, they must ensure registering his > k*probes, otherwise it does not work correctly. If the CONFIG_KRETPROBES=n, > register_kretprobe() always fails, this means that the code has > no chance to call those functions (it must be). > OK, thank you for your explanations. - And original kprobe static inline functions have done like that, in same header file, if no obvious reasons, we can try to follow. >>> >>> It is no reasons to follow that too. Please keep your patch simple as much >>> as possible. >>> >> >> "keep our patch simple as much as posssible" sounds reasonable to me. >> After skip "include/linux/kprobe.h", our patch's subject (include >> comments) also need be changed (I will/should change it). >> >> For me, "include/linux/kprobe.h" can also be improved, but it can be >> another patch for it (not only for kretprobe, but also for jpobe). > > if that "improvement" means "simplify", it is acceptable. Now I don't like > ifdefs of CONFIG_KPROBES and dummy functions, since if CONFIG_KPROBES=n, > other kernel modules can also check the kconfig and decide what they do > (or don't). > Perhaps, what we've really needed is "just enough able to compile", > not the fully covered dummy APIs. > Hmm... for me, I still try to send a patch for "include/linux/kprobe.h". For API (although it is kernel internal API), I have a hobby to try to let it 'beautiful' as much as possible. > I just concerned that it is a waste of memory if there are useless > kretprobe > related instances are built when CONFIG_KRETPROBES=n. > Yeah, that is also one of reason (3rd reason). And if necessary, please help check what we have done whether already "let all kretprobe generic implementation within CONFIG_KRETPROBES area" (exclude declaration, struct/union definition, and architecture implementation). >>> >>> As I commented, your changes in kernel/kprobes.c are good to me except >>> two functions. That's all what we need to fix :) >>> >> >> I will send a patch for it (since subject changed, we need not mark >> "patch v2"), thanks. :-) > > OK, I'll review that. > Thanks. > Thank you! > > Thanks. -- Chen Gang Open, share and attitude like air, water and life which God blessed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
(2014/02/05 12:08), Chen Gang wrote: Anyway, I don't think those inlined functions to be changed, because most of them are internal functions. If CONFIG_KRETPROBES=n, it just be ignored. >>> >>> In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(), >>> disable_kretprobe(), and enable_kretprobe() are not ignored. >> >> Really? where are they called? I mean, those functions do not have >> any instance unless your module uses it (but that is not what the kernel >> itself should help). >> > > If what you said correct (I guess so), for me, we still need let them in > CONFIG_KRETPROBES area, and without any dummy outside, just like another > *kprobe* static inline functions have done in "include/linux/kprobes.h". kretprobe_assert() is only for the internal check. So we don't need to care about, and disable/enable_kretprobe() are anyway returns -EINVAL because kretprobe can not be registered. And all of them are inlined functions. In that case, we don't need to care about it. >>> >>> Hmm... it is related with code 'consistency': >>> >>> - these static inline functions are kretprobe generic implementation, >>>and we are trying to let all kretprobe generic implementation within >>>CONFIG_KRETPROBES area. >> >> No, actually, kretprobe is just built on the kprobe. enable/disable_kretprobe >> just wrapped the kprobe methods. And kretprobe_assert() is just for kretprobe >> internal use. that is not an API. Moving only the kretprobe_assert() into the >> CONFIG_KRETPROBE area is not bad, but since it is just a static inline >> function, >> if there is no caller, it just be ignored, no side effect. >> > > OK, I can understand. > > And do you mean enable/disable_kretprobe() are API? if so, we have to > implement them whether CONFIG_KRETPROBES enabled or disabled. > > And when CONFIG_KRETPROBES=n, just like what you originally said: we > need returns -EINVAL directly (either, I am not quite sure whether the > input parameter will be NULL, in this case). Both are API, and when implementing it I had also considered that, but I decided to stay it in inline-function wrapper. The reason why is, that enable/disable_k*probe require the registered k*probes. If the kernel hacker uses those functions, they must ensure registering his k*probes, otherwise it does not work correctly. If the CONFIG_KRETPROBES=n, register_kretprobe() always fails, this means that the code has no chance to call those functions (it must be). >>> - And original kprobe static inline functions have done like that, >>>in same header file, if no obvious reasons, we can try to follow. >> >> It is no reasons to follow that too. Please keep your patch simple as much >> as possible. >> > > "keep our patch simple as much as posssible" sounds reasonable to me. > After skip "include/linux/kprobe.h", our patch's subject (include > comments) also need be changed (I will/should change it). > > For me, "include/linux/kprobe.h" can also be improved, but it can be > another patch for it (not only for kretprobe, but also for jpobe). if that "improvement" means "simplify", it is acceptable. Now I don't like ifdefs of CONFIG_KPROBES and dummy functions, since if CONFIG_KPROBES=n, other kernel modules can also check the kconfig and decide what they do (or don't). Perhaps, what we've really needed is "just enough able to compile", not the fully covered dummy APIs. I just concerned that it is a waste of memory if there are useless kretprobe related instances are built when CONFIG_KRETPROBES=n. >>> >>> Yeah, that is also one of reason (3rd reason). >>> >>> >>> And if necessary, please help check what we have done whether already >>> "let all kretprobe generic implementation within CONFIG_KRETPROBES area" >>> (exclude declaration, struct/union definition, and architecture >>> implementation). >> >> As I commented, your changes in kernel/kprobes.c are good to me except >> two functions. That's all what we need to fix :) >> > > I will send a patch for it (since subject changed, we need not mark > "patch v2"), thanks. :-) OK, I'll review that. Thank you! -- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu...@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
On 02/05/2014 09:21 AM, Masami Hiramatsu wrote:
> (2014/02/05 9:18), Chen Gang wrote:
>> On 02/04/2014 11:39 PM, Masami Hiramatsu wrote:
>>> (2014/02/04 22:53), Chen Gang wrote:
On 02/04/2014 09:29 PM, Masami Hiramatsu wrote:
> (2014/02/04 21:07), Chen Gang wrote:
>> On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
>>> (2014/02/04 14:16), Chen Gang wrote:
When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
are useless, so need move them to CONFIG_KPROBES enabled area.
Now, *kretprobe* generic implementation are all implemented in 2 files:
- in "include/linux/kprobes.h":
move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
move some *kprobe() declarations which kretprobe*() call, to
front.
not touch kretprobe_blacklist[] which is architecture's variable.
- in "kernel/kprobes.c":
move all kretprobe* to CONFIG_KPROBES area and dummy outside.
define kretprobe_flush_task() to let kprobe_flush_task() call.
define init_kretprobes() to let init_kprobes() call.
The patch passes compiling (get "kernel/kprobes.o" and "kernel/built-
in.o") under avr32 and x86_64 allmodconfig, and passes building (get
bzImage and Modpost modules) under x86_64 defconfig.
>>>
>>> Thanks for the fix! and I have some comments below.
>>>
Signed-off-by: Chen Gang
---
include/linux/kprobes.h | 58 +
kernel/kprobes.c| 328
+++-
2 files changed, 222 insertions(+), 164 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 925eaf2..c0d1212 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
return 1;
}
+int disable_kprobe(struct kprobe *kp);
+int enable_kprobe(struct kprobe *kp);
+
+void dump_kprobe(struct kprobe *kp);
+
+extern struct kretprobe_blackpoint kretprobe_blacklist[];
+
#ifdef CONFIG_KRETPROBES
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);
extern int arch_trampoline_kprobe(struct kprobe *p);
+static inline void kretprobe_assert(struct kretprobe_instance *ri,
+ unsigned long orig_ret_address, unsigned long trampoline_address)
+{
+ if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
+ printk(KERN_ERR
+ "kretprobe BUG!: Processing kretprobe %p @ %p\n",
+ ri->rp, ri->rp->kp.addr);
+ BUG();
+ }
+}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return disable_kprobe(>kp);
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return enable_kprobe(>kp);
+}
+
#else /* CONFIG_KRETPROBES */
static inline void arch_prepare_kretprobe(struct kretprobe *rp,
struct pt_regs *regs)
@@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct
kprobe *p)
{
return 0;
}
-#endif /* CONFIG_KRETPROBES */
-
-extern struct kretprobe_blackpoint kretprobe_blacklist[];
-
static inline void kretprobe_assert(struct kretprobe_instance *ri,
unsigned long orig_ret_address, unsigned long trampoline_address)
{
- if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
- printk("kretprobe BUG!: Processing kretprobe %p @ %p\n",
- ri->rp, ri->rp->kp.addr);
- BUG();
- }
}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
>>>
>>> No, these should returns -EINVAL or -ENOSYS, since these are user API.
>>
>> OK, thanks, it sounds reasonable to me.
>>
>>> Anyway, I don't think those inlined functions to be changed, because
>>> most of them are internal functions. If CONFIG_KRETPROBES=n, it just
>>> be ignored.
>>>
>>
>> In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
>> disable_kretprobe(), and enable_kretprobe() are not ignored.
>
> Really? where are they called? I mean, those functions do not have
> any instance unless your module uses it (but that is not what the kernel
> itself should help).
>
If what you said correct (I guess so), for me, we
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
(2014/02/05 9:18), Chen Gang wrote:
> On 02/04/2014 11:39 PM, Masami Hiramatsu wrote:
>> (2014/02/04 22:53), Chen Gang wrote:
>>> On 02/04/2014 09:29 PM, Masami Hiramatsu wrote:
(2014/02/04 21:07), Chen Gang wrote:
> On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
>> (2014/02/04 14:16), Chen Gang wrote:
>>> When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
>>> are useless, so need move them to CONFIG_KPROBES enabled area.
>>>
>>> Now, *kretprobe* generic implementation are all implemented in 2 files:
>>>
>>> - in "include/linux/kprobes.h":
>>>
>>> move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
>>> move some *kprobe() declarations which kretprobe*() call, to front.
>>> not touch kretprobe_blacklist[] which is architecture's variable.
>>>
>>> - in "kernel/kprobes.c":
>>>
>>> move all kretprobe* to CONFIG_KPROBES area and dummy outside.
>>> define kretprobe_flush_task() to let kprobe_flush_task() call.
>>> define init_kretprobes() to let init_kprobes() call.
>>>
>>> The patch passes compiling (get "kernel/kprobes.o" and "kernel/built-
>>> in.o") under avr32 and x86_64 allmodconfig, and passes building (get
>>> bzImage and Modpost modules) under x86_64 defconfig.
>>
>> Thanks for the fix! and I have some comments below.
>>
>>> Signed-off-by: Chen Gang
>>> ---
>>> include/linux/kprobes.h | 58 +
>>> kernel/kprobes.c| 328
>>> +++-
>>> 2 files changed, 222 insertions(+), 164 deletions(-)
>>>
>>> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
>>> index 925eaf2..c0d1212 100644
>>> --- a/include/linux/kprobes.h
>>> +++ b/include/linux/kprobes.h
>>> @@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
>>> return 1;
>>> }
>>>
>>> +int disable_kprobe(struct kprobe *kp);
>>> +int enable_kprobe(struct kprobe *kp);
>>> +
>>> +void dump_kprobe(struct kprobe *kp);
>>> +
>>> +extern struct kretprobe_blackpoint kretprobe_blacklist[];
>>> +
>>> #ifdef CONFIG_KRETPROBES
>>> extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
>>> struct pt_regs *regs);
>>> extern int arch_trampoline_kprobe(struct kprobe *p);
>>> +static inline void kretprobe_assert(struct kretprobe_instance *ri,
>>> + unsigned long orig_ret_address, unsigned long trampoline_address)
>>> +{
>>> + if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
>>> + printk(KERN_ERR
>>> + "kretprobe BUG!: Processing kretprobe %p @ %p\n",
>>> + ri->rp, ri->rp->kp.addr);
>>> + BUG();
>>> + }
>>> +}
>>> +static inline int disable_kretprobe(struct kretprobe *rp)
>>> +{
>>> + return disable_kprobe(>kp);
>>> +}
>>> +static inline int enable_kretprobe(struct kretprobe *rp)
>>> +{
>>> + return enable_kprobe(>kp);
>>> +}
>>> +
>>> #else /* CONFIG_KRETPROBES */
>>> static inline void arch_prepare_kretprobe(struct kretprobe *rp,
>>> struct pt_regs *regs)
>>> @@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct
>>> kprobe *p)
>>> {
>>> return 0;
>>> }
>>> -#endif /* CONFIG_KRETPROBES */
>>> -
>>> -extern struct kretprobe_blackpoint kretprobe_blacklist[];
>>> -
>>> static inline void kretprobe_assert(struct kretprobe_instance *ri,
>>> unsigned long orig_ret_address, unsigned long trampoline_address)
>>> {
>>> - if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
>>> - printk("kretprobe BUG!: Processing kretprobe %p @ %p\n",
>>> - ri->rp, ri->rp->kp.addr);
>>> - BUG();
>>> - }
>>> }
>>> +static inline int disable_kretprobe(struct kretprobe *rp)
>>> +{
>>> + return 0;
>>> +}
>>> +static inline int enable_kretprobe(struct kretprobe *rp)
>>> +{
>>> + return 0;
>>> +}
>>
>> No, these should returns -EINVAL or -ENOSYS, since these are user API.
>
> OK, thanks, it sounds reasonable to me.
>
>> Anyway, I don't think those inlined functions to be changed, because
>> most of them are internal functions. If CONFIG_KRETPROBES=n, it just
>> be ignored.
>>
>
> In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
> disable_kretprobe(), and enable_kretprobe() are not ignored.
Really? where are they called? I mean, those functions do not have
any instance unless your module uses it (but that is not what the kernel
itself should help).
>>>
>>> If what you said correct (I guess so), for me, we still need let them in
>>> CONFIG_KRETPROBES area, and without any dummy outside, just like another
>>> *kprobe* static inline functions have done in "include/linux/kprobes.h".
>>
>>
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
On 02/04/2014 11:39 PM, Masami Hiramatsu wrote:
> (2014/02/04 22:53), Chen Gang wrote:
>> On 02/04/2014 09:29 PM, Masami Hiramatsu wrote:
>>> (2014/02/04 21:07), Chen Gang wrote:
On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
> (2014/02/04 14:16), Chen Gang wrote:
>> When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
>> are useless, so need move them to CONFIG_KPROBES enabled area.
>>
>> Now, *kretprobe* generic implementation are all implemented in 2 files:
>>
>> - in "include/linux/kprobes.h":
>>
>> move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
>> move some *kprobe() declarations which kretprobe*() call, to front.
>> not touch kretprobe_blacklist[] which is architecture's variable.
>>
>> - in "kernel/kprobes.c":
>>
>> move all kretprobe* to CONFIG_KPROBES area and dummy outside.
>> define kretprobe_flush_task() to let kprobe_flush_task() call.
>> define init_kretprobes() to let init_kprobes() call.
>>
>> The patch passes compiling (get "kernel/kprobes.o" and "kernel/built-
>> in.o") under avr32 and x86_64 allmodconfig, and passes building (get
>> bzImage and Modpost modules) under x86_64 defconfig.
>
> Thanks for the fix! and I have some comments below.
>
>> Signed-off-by: Chen Gang
>> ---
>> include/linux/kprobes.h | 58 +
>> kernel/kprobes.c| 328
>> +++-
>> 2 files changed, 222 insertions(+), 164 deletions(-)
>>
>> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
>> index 925eaf2..c0d1212 100644
>> --- a/include/linux/kprobes.h
>> +++ b/include/linux/kprobes.h
>> @@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
>> return 1;
>> }
>>
>> +int disable_kprobe(struct kprobe *kp);
>> +int enable_kprobe(struct kprobe *kp);
>> +
>> +void dump_kprobe(struct kprobe *kp);
>> +
>> +extern struct kretprobe_blackpoint kretprobe_blacklist[];
>> +
>> #ifdef CONFIG_KRETPROBES
>> extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
>> struct pt_regs *regs);
>> extern int arch_trampoline_kprobe(struct kprobe *p);
>> +static inline void kretprobe_assert(struct kretprobe_instance *ri,
>> + unsigned long orig_ret_address, unsigned long trampoline_address)
>> +{
>> + if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
>> + printk(KERN_ERR
>> + "kretprobe BUG!: Processing kretprobe %p @ %p\n",
>> + ri->rp, ri->rp->kp.addr);
>> + BUG();
>> + }
>> +}
>> +static inline int disable_kretprobe(struct kretprobe *rp)
>> +{
>> + return disable_kprobe(>kp);
>> +}
>> +static inline int enable_kretprobe(struct kretprobe *rp)
>> +{
>> + return enable_kprobe(>kp);
>> +}
>> +
>> #else /* CONFIG_KRETPROBES */
>> static inline void arch_prepare_kretprobe(struct kretprobe *rp,
>> struct pt_regs *regs)
>> @@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct
>> kprobe *p)
>> {
>> return 0;
>> }
>> -#endif /* CONFIG_KRETPROBES */
>> -
>> -extern struct kretprobe_blackpoint kretprobe_blacklist[];
>> -
>> static inline void kretprobe_assert(struct kretprobe_instance *ri,
>> unsigned long orig_ret_address, unsigned long trampoline_address)
>> {
>> - if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
>> - printk("kretprobe BUG!: Processing kretprobe %p @ %p\n",
>> - ri->rp, ri->rp->kp.addr);
>> - BUG();
>> - }
>> }
>> +static inline int disable_kretprobe(struct kretprobe *rp)
>> +{
>> + return 0;
>> +}
>> +static inline int enable_kretprobe(struct kretprobe *rp)
>> +{
>> + return 0;
>> +}
>
> No, these should returns -EINVAL or -ENOSYS, since these are user API.
OK, thanks, it sounds reasonable to me.
> Anyway, I don't think those inlined functions to be changed, because
> most of them are internal functions. If CONFIG_KRETPROBES=n, it just
> be ignored.
>
In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
disable_kretprobe(), and enable_kretprobe() are not ignored.
>>>
>>> Really? where are they called? I mean, those functions do not have
>>> any instance unless your module uses it (but that is not what the kernel
>>> itself should help).
>>>
>>
>> If what you said correct (I guess so), for me, we still need let them in
>> CONFIG_KRETPROBES area, and without any dummy outside, just like another
>> *kprobe* static inline functions have done in "include/linux/kprobes.h".
>
> kretprobe_assert() is only for the internal check. So we don't need to care
> about, and disable/enable_kretprobe() are anyway returns -EINVAL because
>
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
(2014/02/04 22:53), Chen Gang wrote:
> On 02/04/2014 09:29 PM, Masami Hiramatsu wrote:
>> (2014/02/04 21:07), Chen Gang wrote:
>>> On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
(2014/02/04 14:16), Chen Gang wrote:
> When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
> are useless, so need move them to CONFIG_KPROBES enabled area.
>
> Now, *kretprobe* generic implementation are all implemented in 2 files:
>
> - in "include/linux/kprobes.h":
>
> move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
> move some *kprobe() declarations which kretprobe*() call, to front.
> not touch kretprobe_blacklist[] which is architecture's variable.
>
> - in "kernel/kprobes.c":
>
> move all kretprobe* to CONFIG_KPROBES area and dummy outside.
> define kretprobe_flush_task() to let kprobe_flush_task() call.
> define init_kretprobes() to let init_kprobes() call.
>
> The patch passes compiling (get "kernel/kprobes.o" and "kernel/built-
> in.o") under avr32 and x86_64 allmodconfig, and passes building (get
> bzImage and Modpost modules) under x86_64 defconfig.
Thanks for the fix! and I have some comments below.
> Signed-off-by: Chen Gang
> ---
> include/linux/kprobes.h | 58 +
> kernel/kprobes.c| 328
> +++-
> 2 files changed, 222 insertions(+), 164 deletions(-)
>
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 925eaf2..c0d1212 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
> return 1;
> }
>
> +int disable_kprobe(struct kprobe *kp);
> +int enable_kprobe(struct kprobe *kp);
> +
> +void dump_kprobe(struct kprobe *kp);
> +
> +extern struct kretprobe_blackpoint kretprobe_blacklist[];
> +
> #ifdef CONFIG_KRETPROBES
> extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
> struct pt_regs *regs);
> extern int arch_trampoline_kprobe(struct kprobe *p);
> +static inline void kretprobe_assert(struct kretprobe_instance *ri,
> + unsigned long orig_ret_address, unsigned long trampoline_address)
> +{
> + if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
> + printk(KERN_ERR
> + "kretprobe BUG!: Processing kretprobe %p @ %p\n",
> + ri->rp, ri->rp->kp.addr);
> + BUG();
> + }
> +}
> +static inline int disable_kretprobe(struct kretprobe *rp)
> +{
> + return disable_kprobe(>kp);
> +}
> +static inline int enable_kretprobe(struct kretprobe *rp)
> +{
> + return enable_kprobe(>kp);
> +}
> +
> #else /* CONFIG_KRETPROBES */
> static inline void arch_prepare_kretprobe(struct kretprobe *rp,
> struct pt_regs *regs)
> @@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct
> kprobe *p)
> {
> return 0;
> }
> -#endif /* CONFIG_KRETPROBES */
> -
> -extern struct kretprobe_blackpoint kretprobe_blacklist[];
> -
> static inline void kretprobe_assert(struct kretprobe_instance *ri,
> unsigned long orig_ret_address, unsigned long trampoline_address)
> {
> - if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
> - printk("kretprobe BUG!: Processing kretprobe %p @ %p\n",
> - ri->rp, ri->rp->kp.addr);
> - BUG();
> - }
> }
> +static inline int disable_kretprobe(struct kretprobe *rp)
> +{
> + return 0;
> +}
> +static inline int enable_kretprobe(struct kretprobe *rp)
> +{
> + return 0;
> +}
No, these should returns -EINVAL or -ENOSYS, since these are user API.
>>>
>>> OK, thanks, it sounds reasonable to me.
>>>
Anyway, I don't think those inlined functions to be changed, because
most of them are internal functions. If CONFIG_KRETPROBES=n, it just
be ignored.
>>>
>>> In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
>>> disable_kretprobe(), and enable_kretprobe() are not ignored.
>>
>> Really? where are they called? I mean, those functions do not have
>> any instance unless your module uses it (but that is not what the kernel
>> itself should help).
>>
>
> If what you said correct (I guess so), for me, we still need let them in
> CONFIG_KRETPROBES area, and without any dummy outside, just like another
> *kprobe* static inline functions have done in "include/linux/kprobes.h".
kretprobe_assert() is only for the internal check. So we don't need to care
about, and disable/enable_kretprobe() are anyway returns -EINVAL because
kretprobe can not be registered. And all of them are inlined functions.
In that case, we don't need to care about it.
I just concerned that it is a waste of memory if there are useless
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
On 02/04/2014 09:29 PM, Masami Hiramatsu wrote:
> (2014/02/04 21:07), Chen Gang wrote:
>> On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
>>> (2014/02/04 14:16), Chen Gang wrote:
When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
are useless, so need move them to CONFIG_KPROBES enabled area.
Now, *kretprobe* generic implementation are all implemented in 2 files:
- in "include/linux/kprobes.h":
move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
move some *kprobe() declarations which kretprobe*() call, to front.
not touch kretprobe_blacklist[] which is architecture's variable.
- in "kernel/kprobes.c":
move all kretprobe* to CONFIG_KPROBES area and dummy outside.
define kretprobe_flush_task() to let kprobe_flush_task() call.
define init_kretprobes() to let init_kprobes() call.
The patch passes compiling (get "kernel/kprobes.o" and "kernel/built-
in.o") under avr32 and x86_64 allmodconfig, and passes building (get
bzImage and Modpost modules) under x86_64 defconfig.
>>>
>>> Thanks for the fix! and I have some comments below.
>>>
Signed-off-by: Chen Gang
---
include/linux/kprobes.h | 58 +
kernel/kprobes.c| 328
+++-
2 files changed, 222 insertions(+), 164 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 925eaf2..c0d1212 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
return 1;
}
+int disable_kprobe(struct kprobe *kp);
+int enable_kprobe(struct kprobe *kp);
+
+void dump_kprobe(struct kprobe *kp);
+
+extern struct kretprobe_blackpoint kretprobe_blacklist[];
+
#ifdef CONFIG_KRETPROBES
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);
extern int arch_trampoline_kprobe(struct kprobe *p);
+static inline void kretprobe_assert(struct kretprobe_instance *ri,
+ unsigned long orig_ret_address, unsigned long trampoline_address)
+{
+ if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
+ printk(KERN_ERR
+ "kretprobe BUG!: Processing kretprobe %p @ %p\n",
+ ri->rp, ri->rp->kp.addr);
+ BUG();
+ }
+}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return disable_kprobe(>kp);
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return enable_kprobe(>kp);
+}
+
#else /* CONFIG_KRETPROBES */
static inline void arch_prepare_kretprobe(struct kretprobe *rp,
struct pt_regs *regs)
@@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct
kprobe *p)
{
return 0;
}
-#endif /* CONFIG_KRETPROBES */
-
-extern struct kretprobe_blackpoint kretprobe_blacklist[];
-
static inline void kretprobe_assert(struct kretprobe_instance *ri,
unsigned long orig_ret_address, unsigned long trampoline_address)
{
- if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
- printk("kretprobe BUG!: Processing kretprobe %p @ %p\n",
- ri->rp, ri->rp->kp.addr);
- BUG();
- }
}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
>>>
>>> No, these should returns -EINVAL or -ENOSYS, since these are user API.
>>
>> OK, thanks, it sounds reasonable to me.
>>
>>> Anyway, I don't think those inlined functions to be changed, because
>>> most of them are internal functions. If CONFIG_KRETPROBES=n, it just
>>> be ignored.
>>>
>>
>> In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
>> disable_kretprobe(), and enable_kretprobe() are not ignored.
>
> Really? where are they called? I mean, those functions do not have
> any instance unless your module uses it (but that is not what the kernel
> itself should help).
>
If what you said correct (I guess so), for me, we still need let them in
CONFIG_KRETPROBES area, and without any dummy outside, just like another
*kprobe* static inline functions have done in "include/linux/kprobes.h".
>>
>>> So, I think you don't need to change kprobes.h.
>>>
>>
>> So "kprobes.h" still need be changed.
>
> Is there any concrete problem you have?
>
No, I just read the code, no additional really issues.
Thanks.
--
Chen Gang
Open, share and attitude like air, water and life which God blessed
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
(2014/02/04 21:07), Chen Gang wrote:
> On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
>> (2014/02/04 14:16), Chen Gang wrote:
>>> When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
>>> are useless, so need move them to CONFIG_KPROBES enabled area.
>>>
>>> Now, *kretprobe* generic implementation are all implemented in 2 files:
>>>
>>> - in "include/linux/kprobes.h":
>>>
>>> move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
>>> move some *kprobe() declarations which kretprobe*() call, to front.
>>> not touch kretprobe_blacklist[] which is architecture's variable.
>>>
>>> - in "kernel/kprobes.c":
>>>
>>> move all kretprobe* to CONFIG_KPROBES area and dummy outside.
>>> define kretprobe_flush_task() to let kprobe_flush_task() call.
>>> define init_kretprobes() to let init_kprobes() call.
>>>
>>> The patch passes compiling (get "kernel/kprobes.o" and "kernel/built-
>>> in.o") under avr32 and x86_64 allmodconfig, and passes building (get
>>> bzImage and Modpost modules) under x86_64 defconfig.
>>
>> Thanks for the fix! and I have some comments below.
>>
>>> Signed-off-by: Chen Gang
>>> ---
>>> include/linux/kprobes.h | 58 +
>>> kernel/kprobes.c| 328
>>> +++-
>>> 2 files changed, 222 insertions(+), 164 deletions(-)
>>>
>>> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
>>> index 925eaf2..c0d1212 100644
>>> --- a/include/linux/kprobes.h
>>> +++ b/include/linux/kprobes.h
>>> @@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
>>> return 1;
>>> }
>>>
>>> +int disable_kprobe(struct kprobe *kp);
>>> +int enable_kprobe(struct kprobe *kp);
>>> +
>>> +void dump_kprobe(struct kprobe *kp);
>>> +
>>> +extern struct kretprobe_blackpoint kretprobe_blacklist[];
>>> +
>>> #ifdef CONFIG_KRETPROBES
>>> extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
>>> struct pt_regs *regs);
>>> extern int arch_trampoline_kprobe(struct kprobe *p);
>>> +static inline void kretprobe_assert(struct kretprobe_instance *ri,
>>> + unsigned long orig_ret_address, unsigned long trampoline_address)
>>> +{
>>> + if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
>>> + printk(KERN_ERR
>>> + "kretprobe BUG!: Processing kretprobe %p @ %p\n",
>>> + ri->rp, ri->rp->kp.addr);
>>> + BUG();
>>> + }
>>> +}
>>> +static inline int disable_kretprobe(struct kretprobe *rp)
>>> +{
>>> + return disable_kprobe(>kp);
>>> +}
>>> +static inline int enable_kretprobe(struct kretprobe *rp)
>>> +{
>>> + return enable_kprobe(>kp);
>>> +}
>>> +
>>> #else /* CONFIG_KRETPROBES */
>>> static inline void arch_prepare_kretprobe(struct kretprobe *rp,
>>> struct pt_regs *regs)
>>> @@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct
>>> kprobe *p)
>>> {
>>> return 0;
>>> }
>>> -#endif /* CONFIG_KRETPROBES */
>>> -
>>> -extern struct kretprobe_blackpoint kretprobe_blacklist[];
>>> -
>>> static inline void kretprobe_assert(struct kretprobe_instance *ri,
>>> unsigned long orig_ret_address, unsigned long trampoline_address)
>>> {
>>> - if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
>>> - printk("kretprobe BUG!: Processing kretprobe %p @ %p\n",
>>> - ri->rp, ri->rp->kp.addr);
>>> - BUG();
>>> - }
>>> }
>>> +static inline int disable_kretprobe(struct kretprobe *rp)
>>> +{
>>> + return 0;
>>> +}
>>> +static inline int enable_kretprobe(struct kretprobe *rp)
>>> +{
>>> + return 0;
>>> +}
>>
>> No, these should returns -EINVAL or -ENOSYS, since these are user API.
>
> OK, thanks, it sounds reasonable to me.
>
>> Anyway, I don't think those inlined functions to be changed, because
>> most of them are internal functions. If CONFIG_KRETPROBES=n, it just
>> be ignored.
>>
>
> In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
> disable_kretprobe(), and enable_kretprobe() are not ignored.
Really? where are they called? I mean, those functions do not have
any instance unless your module uses it (but that is not what the kernel
itself should help).
>
>> So, I think you don't need to change kprobes.h.
>>
>
> So "kprobes.h" still need be changed.
Is there any concrete problem you have?
>
>>> +
>>> +#endif /* CONFIG_KRETPROBES */
>>>
>>> #ifdef CONFIG_KPROBES_SANITY_TEST
>>> extern int init_test_probes(void);
>>> @@ -379,11 +406,6 @@ void unregister_kretprobes(struct kretprobe **rps, int
>>> num);
>>> void kprobe_flush_task(struct task_struct *tk);
>>> 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)
>>> @@ -459,14 +481,6 @@ static inline int enable_kprobe(struct kprobe *kp)
>>> return -ENOSYS;
>>> }
>>> #endif /* CONFIG_KPROBES */
>>> -static
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
> (2014/02/04 14:16), Chen Gang wrote:
>> When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
>> are useless, so need move them to CONFIG_KPROBES enabled area.
>>
>> Now, *kretprobe* generic implementation are all implemented in 2 files:
>>
>> - in "include/linux/kprobes.h":
>>
>> move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
>> move some *kprobe() declarations which kretprobe*() call, to front.
>> not touch kretprobe_blacklist[] which is architecture's variable.
>>
>> - in "kernel/kprobes.c":
>>
>> move all kretprobe* to CONFIG_KPROBES area and dummy outside.
>> define kretprobe_flush_task() to let kprobe_flush_task() call.
>> define init_kretprobes() to let init_kprobes() call.
>>
>> The patch passes compiling (get "kernel/kprobes.o" and "kernel/built-
>> in.o") under avr32 and x86_64 allmodconfig, and passes building (get
>> bzImage and Modpost modules) under x86_64 defconfig.
>
> Thanks for the fix! and I have some comments below.
>
>> Signed-off-by: Chen Gang
>> ---
>> include/linux/kprobes.h | 58 +
>> kernel/kprobes.c| 328
>> +++-
>> 2 files changed, 222 insertions(+), 164 deletions(-)
>>
>> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
>> index 925eaf2..c0d1212 100644
>> --- a/include/linux/kprobes.h
>> +++ b/include/linux/kprobes.h
>> @@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
>> return 1;
>> }
>>
>> +int disable_kprobe(struct kprobe *kp);
>> +int enable_kprobe(struct kprobe *kp);
>> +
>> +void dump_kprobe(struct kprobe *kp);
>> +
>> +extern struct kretprobe_blackpoint kretprobe_blacklist[];
>> +
>> #ifdef CONFIG_KRETPROBES
>> extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
>> struct pt_regs *regs);
>> extern int arch_trampoline_kprobe(struct kprobe *p);
>> +static inline void kretprobe_assert(struct kretprobe_instance *ri,
>> + unsigned long orig_ret_address, unsigned long trampoline_address)
>> +{
>> + if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
>> + printk(KERN_ERR
>> + "kretprobe BUG!: Processing kretprobe %p @ %p\n",
>> + ri->rp, ri->rp->kp.addr);
>> + BUG();
>> + }
>> +}
>> +static inline int disable_kretprobe(struct kretprobe *rp)
>> +{
>> + return disable_kprobe(>kp);
>> +}
>> +static inline int enable_kretprobe(struct kretprobe *rp)
>> +{
>> + return enable_kprobe(>kp);
>> +}
>> +
>> #else /* CONFIG_KRETPROBES */
>> static inline void arch_prepare_kretprobe(struct kretprobe *rp,
>> struct pt_regs *regs)
>> @@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct kprobe
>> *p)
>> {
>> return 0;
>> }
>> -#endif /* CONFIG_KRETPROBES */
>> -
>> -extern struct kretprobe_blackpoint kretprobe_blacklist[];
>> -
>> static inline void kretprobe_assert(struct kretprobe_instance *ri,
>> unsigned long orig_ret_address, unsigned long trampoline_address)
>> {
>> - if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
>> - printk("kretprobe BUG!: Processing kretprobe %p @ %p\n",
>> - ri->rp, ri->rp->kp.addr);
>> - BUG();
>> - }
>> }
>> +static inline int disable_kretprobe(struct kretprobe *rp)
>> +{
>> + return 0;
>> +}
>> +static inline int enable_kretprobe(struct kretprobe *rp)
>> +{
>> + return 0;
>> +}
>
> No, these should returns -EINVAL or -ENOSYS, since these are user API.
OK, thanks, it sounds reasonable to me.
> Anyway, I don't think those inlined functions to be changed, because
> most of them are internal functions. If CONFIG_KRETPROBES=n, it just
> be ignored.
>
In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
disable_kretprobe(), and enable_kretprobe() are not ignored.
> So, I think you don't need to change kprobes.h.
>
So "kprobes.h" still need be changed.
>> +
>> +#endif /* CONFIG_KRETPROBES */
>>
>> #ifdef CONFIG_KPROBES_SANITY_TEST
>> extern int init_test_probes(void);
>> @@ -379,11 +406,6 @@ void unregister_kretprobes(struct kretprobe **rps, int
>> num);
>> void kprobe_flush_task(struct task_struct *tk);
>> 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)
>> @@ -459,14 +481,6 @@ static inline int enable_kprobe(struct kprobe *kp)
>> return -ENOSYS;
>> }
>> #endif /* CONFIG_KPROBES */
>> -static inline int disable_kretprobe(struct kretprobe *rp)
>> -{
>> - return disable_kprobe(>kp);
>> -}
>> -static inline int enable_kretprobe(struct kretprobe *rp)
>> -{
>> - return enable_kprobe(>kp);
>> -}
>> static inline int disable_jprobe(struct jprobe *jp)
>> {
>> return disable_kprobe(>kp);
>> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
>> index ceeadfc..e305a81 100644
>> ---
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
> (2014/02/04 14:16), Chen Gang wrote:
>> When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
>> are useless, so need move them to CONFIG_KPROBES enabled area.
>>
>> Now, *kretprobe* generic implementation are all implemented in 2 files:
>>
>> - in "include/linux/kprobes.h":
>>
>> move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
>> move some *kprobe() declarations which kretprobe*() call, to front.
>> not touch kretprobe_blacklist[] which is architecture's variable.
>>
>> - in "kernel/kprobes.c":
>>
>> move all kretprobe* to CONFIG_KPROBES area and dummy outside.
>> define kretprobe_flush_task() to let kprobe_flush_task() call.
>> define init_kretprobes() to let init_kprobes() call.
>>
>> The patch passes compiling (get "kernel/kprobes.o" and "kernel/built-
>> in.o") under avr32 and x86_64 allmodconfig, and passes building (get
>> bzImage and Modpost modules) under x86_64 defconfig.
>
> Thanks for the fix! and I have some comments below.
>
>> Signed-off-by: Chen Gang
>> ---
>> include/linux/kprobes.h | 58 +
>> kernel/kprobes.c| 328
>> +++-
>> 2 files changed, 222 insertions(+), 164 deletions(-)
>>
>> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
>> index 925eaf2..c0d1212 100644
>> --- a/include/linux/kprobes.h
>> +++ b/include/linux/kprobes.h
>> @@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
>> return 1;
>> }
>>
>> +int disable_kprobe(struct kprobe *kp);
>> +int enable_kprobe(struct kprobe *kp);
>> +
>> +void dump_kprobe(struct kprobe *kp);
>> +
>> +extern struct kretprobe_blackpoint kretprobe_blacklist[];
>> +
>> #ifdef CONFIG_KRETPROBES
>> extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
>> struct pt_regs *regs);
>> extern int arch_trampoline_kprobe(struct kprobe *p);
>> +static inline void kretprobe_assert(struct kretprobe_instance *ri,
>> +unsigned long orig_ret_address, unsigned long trampoline_address)
>> +{
>> +if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
>> +printk(KERN_ERR
>> +"kretprobe BUG!: Processing kretprobe %p @ %p\n",
>> +ri->rp, ri->rp->kp.addr);
>> +BUG();
>> +}
>> +}
>> +static inline int disable_kretprobe(struct kretprobe *rp)
>> +{
>> +return disable_kprobe(>kp);
>> +}
>> +static inline int enable_kretprobe(struct kretprobe *rp)
>> +{
>> +return enable_kprobe(>kp);
>> +}
>> +
>> #else /* CONFIG_KRETPROBES */
>> static inline void arch_prepare_kretprobe(struct kretprobe *rp,
>> struct pt_regs *regs)
>> @@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct kprobe
>> *p)
>> {
>> return 0;
>> }
>> -#endif /* CONFIG_KRETPROBES */
>> -
>> -extern struct kretprobe_blackpoint kretprobe_blacklist[];
>> -
>> static inline void kretprobe_assert(struct kretprobe_instance *ri,
>> unsigned long orig_ret_address, unsigned long trampoline_address)
>> {
>> -if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
>> -printk("kretprobe BUG!: Processing kretprobe %p @ %p\n",
>> -ri->rp, ri->rp->kp.addr);
>> -BUG();
>> -}
>> }
>> +static inline int disable_kretprobe(struct kretprobe *rp)
>> +{
>> +return 0;
>> +}
>> +static inline int enable_kretprobe(struct kretprobe *rp)
>> +{
>> +return 0;
>> +}
>
> No, these should returns -EINVAL or -ENOSYS, since these are user API.
OK, thanks, it sounds reasonable to me.
> Anyway, I don't think those inlined functions to be changed, because
> most of them are internal functions. If CONFIG_KRETPROBES=n, it just
> be ignored.
>
In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
disable_kretprobe(), and enable_kretprobe() are not ignored.
> So, I think you don't need to change kprobes.h.
>
So "kprobes.h" still need be changed.
>> +
>> +#endif /* CONFIG_KRETPROBES */
>>
>> #ifdef CONFIG_KPROBES_SANITY_TEST
>> extern int init_test_probes(void);
>> @@ -379,11 +406,6 @@ void unregister_kretprobes(struct kretprobe **rps, int
>> num);
>> void kprobe_flush_task(struct task_struct *tk);
>> 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)
>> @@ -459,14 +481,6 @@ static inline int enable_kprobe(struct kprobe *kp)
>> return -ENOSYS;
>> }
>> #endif /* CONFIG_KPROBES */
>> -static inline int disable_kretprobe(struct kretprobe *rp)
>> -{
>> -return disable_kprobe(>kp);
>> -}
>> -static inline int enable_kretprobe(struct kretprobe *rp)
>> -{
>> -
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
(2014/02/04 14:16), Chen Gang wrote:
When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
are useless, so need move them to CONFIG_KPROBES enabled area.
Now, *kretprobe* generic implementation are all implemented in 2 files:
- in include/linux/kprobes.h:
move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
move some *kprobe() declarations which kretprobe*() call, to front.
not touch kretprobe_blacklist[] which is architecture's variable.
- in kernel/kprobes.c:
move all kretprobe* to CONFIG_KPROBES area and dummy outside.
define kretprobe_flush_task() to let kprobe_flush_task() call.
define init_kretprobes() to let init_kprobes() call.
The patch passes compiling (get kernel/kprobes.o and kernel/built-
in.o) under avr32 and x86_64 allmodconfig, and passes building (get
bzImage and Modpost modules) under x86_64 defconfig.
Thanks for the fix! and I have some comments below.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
include/linux/kprobes.h | 58 +
kernel/kprobes.c| 328
+++-
2 files changed, 222 insertions(+), 164 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 925eaf2..c0d1212 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
return 1;
}
+int disable_kprobe(struct kprobe *kp);
+int enable_kprobe(struct kprobe *kp);
+
+void dump_kprobe(struct kprobe *kp);
+
+extern struct kretprobe_blackpoint kretprobe_blacklist[];
+
#ifdef CONFIG_KRETPROBES
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);
extern int arch_trampoline_kprobe(struct kprobe *p);
+static inline void kretprobe_assert(struct kretprobe_instance *ri,
+unsigned long orig_ret_address, unsigned long trampoline_address)
+{
+if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
+printk(KERN_ERR
+kretprobe BUG!: Processing kretprobe %p @ %p\n,
+ri-rp, ri-rp-kp.addr);
+BUG();
+}
+}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+return disable_kprobe(rp-kp);
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+return enable_kprobe(rp-kp);
+}
+
#else /* CONFIG_KRETPROBES */
static inline void arch_prepare_kretprobe(struct kretprobe *rp,
struct pt_regs *regs)
@@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct kprobe
*p)
{
return 0;
}
-#endif /* CONFIG_KRETPROBES */
-
-extern struct kretprobe_blackpoint kretprobe_blacklist[];
-
static inline void kretprobe_assert(struct kretprobe_instance *ri,
unsigned long orig_ret_address, unsigned long trampoline_address)
{
-if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
-printk(kretprobe BUG!: Processing kretprobe %p @ %p\n,
-ri-rp, ri-rp-kp.addr);
-BUG();
-}
}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+return 0;
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+return 0;
+}
No, these should returns -EINVAL or -ENOSYS, since these are user API.
OK, thanks, it sounds reasonable to me.
Anyway, I don't think those inlined functions to be changed, because
most of them are internal functions. If CONFIG_KRETPROBES=n, it just
be ignored.
In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
disable_kretprobe(), and enable_kretprobe() are not ignored.
So, I think you don't need to change kprobes.h.
So kprobes.h still need be changed.
+
+#endif /* CONFIG_KRETPROBES */
#ifdef CONFIG_KPROBES_SANITY_TEST
extern int init_test_probes(void);
@@ -379,11 +406,6 @@ void unregister_kretprobes(struct kretprobe **rps, int
num);
void kprobe_flush_task(struct task_struct *tk);
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)
@@ -459,14 +481,6 @@ static inline int enable_kprobe(struct kprobe *kp)
return -ENOSYS;
}
#endif /* CONFIG_KPROBES */
-static inline int disable_kretprobe(struct kretprobe *rp)
-{
-return disable_kprobe(rp-kp);
-}
-static inline int enable_kretprobe(struct kretprobe *rp)
-{
-return enable_kprobe(rp-kp);
-}
static inline int disable_jprobe(struct jprobe *jp)
{
return disable_kprobe(jp-kp);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index ceeadfc..e305a81 100644
--- a/kernel/kprobes.c
+++
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
(2014/02/04 14:16), Chen Gang wrote:
When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
are useless, so need move them to CONFIG_KPROBES enabled area.
Now, *kretprobe* generic implementation are all implemented in 2 files:
- in include/linux/kprobes.h:
move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
move some *kprobe() declarations which kretprobe*() call, to front.
not touch kretprobe_blacklist[] which is architecture's variable.
- in kernel/kprobes.c:
move all kretprobe* to CONFIG_KPROBES area and dummy outside.
define kretprobe_flush_task() to let kprobe_flush_task() call.
define init_kretprobes() to let init_kprobes() call.
The patch passes compiling (get kernel/kprobes.o and kernel/built-
in.o) under avr32 and x86_64 allmodconfig, and passes building (get
bzImage and Modpost modules) under x86_64 defconfig.
Thanks for the fix! and I have some comments below.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
include/linux/kprobes.h | 58 +
kernel/kprobes.c| 328
+++-
2 files changed, 222 insertions(+), 164 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 925eaf2..c0d1212 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
return 1;
}
+int disable_kprobe(struct kprobe *kp);
+int enable_kprobe(struct kprobe *kp);
+
+void dump_kprobe(struct kprobe *kp);
+
+extern struct kretprobe_blackpoint kretprobe_blacklist[];
+
#ifdef CONFIG_KRETPROBES
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);
extern int arch_trampoline_kprobe(struct kprobe *p);
+static inline void kretprobe_assert(struct kretprobe_instance *ri,
+ unsigned long orig_ret_address, unsigned long trampoline_address)
+{
+ if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
+ printk(KERN_ERR
+ kretprobe BUG!: Processing kretprobe %p @ %p\n,
+ ri-rp, ri-rp-kp.addr);
+ BUG();
+ }
+}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return disable_kprobe(rp-kp);
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return enable_kprobe(rp-kp);
+}
+
#else /* CONFIG_KRETPROBES */
static inline void arch_prepare_kretprobe(struct kretprobe *rp,
struct pt_regs *regs)
@@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct kprobe
*p)
{
return 0;
}
-#endif /* CONFIG_KRETPROBES */
-
-extern struct kretprobe_blackpoint kretprobe_blacklist[];
-
static inline void kretprobe_assert(struct kretprobe_instance *ri,
unsigned long orig_ret_address, unsigned long trampoline_address)
{
- if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
- printk(kretprobe BUG!: Processing kretprobe %p @ %p\n,
- ri-rp, ri-rp-kp.addr);
- BUG();
- }
}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
No, these should returns -EINVAL or -ENOSYS, since these are user API.
OK, thanks, it sounds reasonable to me.
Anyway, I don't think those inlined functions to be changed, because
most of them are internal functions. If CONFIG_KRETPROBES=n, it just
be ignored.
In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
disable_kretprobe(), and enable_kretprobe() are not ignored.
So, I think you don't need to change kprobes.h.
So kprobes.h still need be changed.
+
+#endif /* CONFIG_KRETPROBES */
#ifdef CONFIG_KPROBES_SANITY_TEST
extern int init_test_probes(void);
@@ -379,11 +406,6 @@ void unregister_kretprobes(struct kretprobe **rps, int
num);
void kprobe_flush_task(struct task_struct *tk);
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)
@@ -459,14 +481,6 @@ static inline int enable_kprobe(struct kprobe *kp)
return -ENOSYS;
}
#endif /* CONFIG_KPROBES */
-static inline int disable_kretprobe(struct kretprobe *rp)
-{
- return disable_kprobe(rp-kp);
-}
-static inline int enable_kretprobe(struct kretprobe *rp)
-{
- return enable_kprobe(rp-kp);
-}
static inline int disable_jprobe(struct jprobe *jp)
{
return disable_kprobe(jp-kp);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index ceeadfc..e305a81 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
[...]
@@ -1936,8 +1955,44 @@ static int __kprobes pre_handler_kretprobe(struct
kprobe *p,
return 0;
}
+void __kprobes recycle_rp_inst(struct kretprobe_instance *ri,
+ struct hlist_head *head)
+{
+}
+
+void
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
(2014/02/04 21:07), Chen Gang wrote:
On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
(2014/02/04 14:16), Chen Gang wrote:
When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
are useless, so need move them to CONFIG_KPROBES enabled area.
Now, *kretprobe* generic implementation are all implemented in 2 files:
- in include/linux/kprobes.h:
move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
move some *kprobe() declarations which kretprobe*() call, to front.
not touch kretprobe_blacklist[] which is architecture's variable.
- in kernel/kprobes.c:
move all kretprobe* to CONFIG_KPROBES area and dummy outside.
define kretprobe_flush_task() to let kprobe_flush_task() call.
define init_kretprobes() to let init_kprobes() call.
The patch passes compiling (get kernel/kprobes.o and kernel/built-
in.o) under avr32 and x86_64 allmodconfig, and passes building (get
bzImage and Modpost modules) under x86_64 defconfig.
Thanks for the fix! and I have some comments below.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
include/linux/kprobes.h | 58 +
kernel/kprobes.c| 328
+++-
2 files changed, 222 insertions(+), 164 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 925eaf2..c0d1212 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
return 1;
}
+int disable_kprobe(struct kprobe *kp);
+int enable_kprobe(struct kprobe *kp);
+
+void dump_kprobe(struct kprobe *kp);
+
+extern struct kretprobe_blackpoint kretprobe_blacklist[];
+
#ifdef CONFIG_KRETPROBES
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);
extern int arch_trampoline_kprobe(struct kprobe *p);
+static inline void kretprobe_assert(struct kretprobe_instance *ri,
+ unsigned long orig_ret_address, unsigned long trampoline_address)
+{
+ if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
+ printk(KERN_ERR
+ kretprobe BUG!: Processing kretprobe %p @ %p\n,
+ ri-rp, ri-rp-kp.addr);
+ BUG();
+ }
+}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return disable_kprobe(rp-kp);
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return enable_kprobe(rp-kp);
+}
+
#else /* CONFIG_KRETPROBES */
static inline void arch_prepare_kretprobe(struct kretprobe *rp,
struct pt_regs *regs)
@@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct
kprobe *p)
{
return 0;
}
-#endif /* CONFIG_KRETPROBES */
-
-extern struct kretprobe_blackpoint kretprobe_blacklist[];
-
static inline void kretprobe_assert(struct kretprobe_instance *ri,
unsigned long orig_ret_address, unsigned long trampoline_address)
{
- if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
- printk(kretprobe BUG!: Processing kretprobe %p @ %p\n,
- ri-rp, ri-rp-kp.addr);
- BUG();
- }
}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
No, these should returns -EINVAL or -ENOSYS, since these are user API.
OK, thanks, it sounds reasonable to me.
Anyway, I don't think those inlined functions to be changed, because
most of them are internal functions. If CONFIG_KRETPROBES=n, it just
be ignored.
In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
disable_kretprobe(), and enable_kretprobe() are not ignored.
Really? where are they called? I mean, those functions do not have
any instance unless your module uses it (but that is not what the kernel
itself should help).
So, I think you don't need to change kprobes.h.
So kprobes.h still need be changed.
Is there any concrete problem you have?
+
+#endif /* CONFIG_KRETPROBES */
#ifdef CONFIG_KPROBES_SANITY_TEST
extern int init_test_probes(void);
@@ -379,11 +406,6 @@ void unregister_kretprobes(struct kretprobe **rps, int
num);
void kprobe_flush_task(struct task_struct *tk);
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)
@@ -459,14 +481,6 @@ static inline int enable_kprobe(struct kprobe *kp)
return -ENOSYS;
}
#endif /* CONFIG_KPROBES */
-static inline int disable_kretprobe(struct kretprobe *rp)
-{
- return disable_kprobe(rp-kp);
-}
-static inline int enable_kretprobe(struct kretprobe *rp)
-{
- return enable_kprobe(rp-kp);
-}
static inline int disable_jprobe(struct jprobe *jp)
{
return disable_kprobe(jp-kp);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index ceeadfc..e305a81 100644
---
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
On 02/04/2014 09:29 PM, Masami Hiramatsu wrote:
(2014/02/04 21:07), Chen Gang wrote:
On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
(2014/02/04 14:16), Chen Gang wrote:
When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
are useless, so need move them to CONFIG_KPROBES enabled area.
Now, *kretprobe* generic implementation are all implemented in 2 files:
- in include/linux/kprobes.h:
move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
move some *kprobe() declarations which kretprobe*() call, to front.
not touch kretprobe_blacklist[] which is architecture's variable.
- in kernel/kprobes.c:
move all kretprobe* to CONFIG_KPROBES area and dummy outside.
define kretprobe_flush_task() to let kprobe_flush_task() call.
define init_kretprobes() to let init_kprobes() call.
The patch passes compiling (get kernel/kprobes.o and kernel/built-
in.o) under avr32 and x86_64 allmodconfig, and passes building (get
bzImage and Modpost modules) under x86_64 defconfig.
Thanks for the fix! and I have some comments below.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
include/linux/kprobes.h | 58 +
kernel/kprobes.c| 328
+++-
2 files changed, 222 insertions(+), 164 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 925eaf2..c0d1212 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
return 1;
}
+int disable_kprobe(struct kprobe *kp);
+int enable_kprobe(struct kprobe *kp);
+
+void dump_kprobe(struct kprobe *kp);
+
+extern struct kretprobe_blackpoint kretprobe_blacklist[];
+
#ifdef CONFIG_KRETPROBES
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);
extern int arch_trampoline_kprobe(struct kprobe *p);
+static inline void kretprobe_assert(struct kretprobe_instance *ri,
+ unsigned long orig_ret_address, unsigned long trampoline_address)
+{
+ if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
+ printk(KERN_ERR
+ kretprobe BUG!: Processing kretprobe %p @ %p\n,
+ ri-rp, ri-rp-kp.addr);
+ BUG();
+ }
+}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return disable_kprobe(rp-kp);
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return enable_kprobe(rp-kp);
+}
+
#else /* CONFIG_KRETPROBES */
static inline void arch_prepare_kretprobe(struct kretprobe *rp,
struct pt_regs *regs)
@@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct
kprobe *p)
{
return 0;
}
-#endif /* CONFIG_KRETPROBES */
-
-extern struct kretprobe_blackpoint kretprobe_blacklist[];
-
static inline void kretprobe_assert(struct kretprobe_instance *ri,
unsigned long orig_ret_address, unsigned long trampoline_address)
{
- if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
- printk(kretprobe BUG!: Processing kretprobe %p @ %p\n,
- ri-rp, ri-rp-kp.addr);
- BUG();
- }
}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
No, these should returns -EINVAL or -ENOSYS, since these are user API.
OK, thanks, it sounds reasonable to me.
Anyway, I don't think those inlined functions to be changed, because
most of them are internal functions. If CONFIG_KRETPROBES=n, it just
be ignored.
In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
disable_kretprobe(), and enable_kretprobe() are not ignored.
Really? where are they called? I mean, those functions do not have
any instance unless your module uses it (but that is not what the kernel
itself should help).
If what you said correct (I guess so), for me, we still need let them in
CONFIG_KRETPROBES area, and without any dummy outside, just like another
*kprobe* static inline functions have done in include/linux/kprobes.h.
So, I think you don't need to change kprobes.h.
So kprobes.h still need be changed.
Is there any concrete problem you have?
No, I just read the code, no additional really issues.
Thanks.
--
Chen Gang
Open, share and attitude like air, water and life which God blessed
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
(2014/02/04 22:53), Chen Gang wrote:
On 02/04/2014 09:29 PM, Masami Hiramatsu wrote:
(2014/02/04 21:07), Chen Gang wrote:
On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
(2014/02/04 14:16), Chen Gang wrote:
When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
are useless, so need move them to CONFIG_KPROBES enabled area.
Now, *kretprobe* generic implementation are all implemented in 2 files:
- in include/linux/kprobes.h:
move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
move some *kprobe() declarations which kretprobe*() call, to front.
not touch kretprobe_blacklist[] which is architecture's variable.
- in kernel/kprobes.c:
move all kretprobe* to CONFIG_KPROBES area and dummy outside.
define kretprobe_flush_task() to let kprobe_flush_task() call.
define init_kretprobes() to let init_kprobes() call.
The patch passes compiling (get kernel/kprobes.o and kernel/built-
in.o) under avr32 and x86_64 allmodconfig, and passes building (get
bzImage and Modpost modules) under x86_64 defconfig.
Thanks for the fix! and I have some comments below.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
include/linux/kprobes.h | 58 +
kernel/kprobes.c| 328
+++-
2 files changed, 222 insertions(+), 164 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 925eaf2..c0d1212 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
return 1;
}
+int disable_kprobe(struct kprobe *kp);
+int enable_kprobe(struct kprobe *kp);
+
+void dump_kprobe(struct kprobe *kp);
+
+extern struct kretprobe_blackpoint kretprobe_blacklist[];
+
#ifdef CONFIG_KRETPROBES
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);
extern int arch_trampoline_kprobe(struct kprobe *p);
+static inline void kretprobe_assert(struct kretprobe_instance *ri,
+ unsigned long orig_ret_address, unsigned long trampoline_address)
+{
+ if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
+ printk(KERN_ERR
+ kretprobe BUG!: Processing kretprobe %p @ %p\n,
+ ri-rp, ri-rp-kp.addr);
+ BUG();
+ }
+}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return disable_kprobe(rp-kp);
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return enable_kprobe(rp-kp);
+}
+
#else /* CONFIG_KRETPROBES */
static inline void arch_prepare_kretprobe(struct kretprobe *rp,
struct pt_regs *regs)
@@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct
kprobe *p)
{
return 0;
}
-#endif /* CONFIG_KRETPROBES */
-
-extern struct kretprobe_blackpoint kretprobe_blacklist[];
-
static inline void kretprobe_assert(struct kretprobe_instance *ri,
unsigned long orig_ret_address, unsigned long trampoline_address)
{
- if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
- printk(kretprobe BUG!: Processing kretprobe %p @ %p\n,
- ri-rp, ri-rp-kp.addr);
- BUG();
- }
}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
No, these should returns -EINVAL or -ENOSYS, since these are user API.
OK, thanks, it sounds reasonable to me.
Anyway, I don't think those inlined functions to be changed, because
most of them are internal functions. If CONFIG_KRETPROBES=n, it just
be ignored.
In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
disable_kretprobe(), and enable_kretprobe() are not ignored.
Really? where are they called? I mean, those functions do not have
any instance unless your module uses it (but that is not what the kernel
itself should help).
If what you said correct (I guess so), for me, we still need let them in
CONFIG_KRETPROBES area, and without any dummy outside, just like another
*kprobe* static inline functions have done in include/linux/kprobes.h.
kretprobe_assert() is only for the internal check. So we don't need to care
about, and disable/enable_kretprobe() are anyway returns -EINVAL because
kretprobe can not be registered. And all of them are inlined functions.
In that case, we don't need to care about it.
I just concerned that it is a waste of memory if there are useless kretprobe
related instances are built when CONFIG_KRETPROBES=n.
Thank you,
So, I think you don't need to change kprobes.h.
So kprobes.h still need be changed.
Is there any concrete problem you have?
No, I just read the code, no additional really issues.
Thanks.
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
On 02/04/2014 11:39 PM, Masami Hiramatsu wrote:
(2014/02/04 22:53), Chen Gang wrote:
On 02/04/2014 09:29 PM, Masami Hiramatsu wrote:
(2014/02/04 21:07), Chen Gang wrote:
On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
(2014/02/04 14:16), Chen Gang wrote:
When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
are useless, so need move them to CONFIG_KPROBES enabled area.
Now, *kretprobe* generic implementation are all implemented in 2 files:
- in include/linux/kprobes.h:
move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
move some *kprobe() declarations which kretprobe*() call, to front.
not touch kretprobe_blacklist[] which is architecture's variable.
- in kernel/kprobes.c:
move all kretprobe* to CONFIG_KPROBES area and dummy outside.
define kretprobe_flush_task() to let kprobe_flush_task() call.
define init_kretprobes() to let init_kprobes() call.
The patch passes compiling (get kernel/kprobes.o and kernel/built-
in.o) under avr32 and x86_64 allmodconfig, and passes building (get
bzImage and Modpost modules) under x86_64 defconfig.
Thanks for the fix! and I have some comments below.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
include/linux/kprobes.h | 58 +
kernel/kprobes.c| 328
+++-
2 files changed, 222 insertions(+), 164 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 925eaf2..c0d1212 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
return 1;
}
+int disable_kprobe(struct kprobe *kp);
+int enable_kprobe(struct kprobe *kp);
+
+void dump_kprobe(struct kprobe *kp);
+
+extern struct kretprobe_blackpoint kretprobe_blacklist[];
+
#ifdef CONFIG_KRETPROBES
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);
extern int arch_trampoline_kprobe(struct kprobe *p);
+static inline void kretprobe_assert(struct kretprobe_instance *ri,
+ unsigned long orig_ret_address, unsigned long trampoline_address)
+{
+ if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
+ printk(KERN_ERR
+ kretprobe BUG!: Processing kretprobe %p @ %p\n,
+ ri-rp, ri-rp-kp.addr);
+ BUG();
+ }
+}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return disable_kprobe(rp-kp);
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return enable_kprobe(rp-kp);
+}
+
#else /* CONFIG_KRETPROBES */
static inline void arch_prepare_kretprobe(struct kretprobe *rp,
struct pt_regs *regs)
@@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct
kprobe *p)
{
return 0;
}
-#endif /* CONFIG_KRETPROBES */
-
-extern struct kretprobe_blackpoint kretprobe_blacklist[];
-
static inline void kretprobe_assert(struct kretprobe_instance *ri,
unsigned long orig_ret_address, unsigned long trampoline_address)
{
- if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
- printk(kretprobe BUG!: Processing kretprobe %p @ %p\n,
- ri-rp, ri-rp-kp.addr);
- BUG();
- }
}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
No, these should returns -EINVAL or -ENOSYS, since these are user API.
OK, thanks, it sounds reasonable to me.
Anyway, I don't think those inlined functions to be changed, because
most of them are internal functions. If CONFIG_KRETPROBES=n, it just
be ignored.
In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
disable_kretprobe(), and enable_kretprobe() are not ignored.
Really? where are they called? I mean, those functions do not have
any instance unless your module uses it (but that is not what the kernel
itself should help).
If what you said correct (I guess so), for me, we still need let them in
CONFIG_KRETPROBES area, and without any dummy outside, just like another
*kprobe* static inline functions have done in include/linux/kprobes.h.
kretprobe_assert() is only for the internal check. So we don't need to care
about, and disable/enable_kretprobe() are anyway returns -EINVAL because
kretprobe can not be registered. And all of them are inlined functions.
In that case, we don't need to care about it.
Hmm... it is related with code 'consistency':
- these static inline functions are kretprobe generic implementation,
and we are trying to let all kretprobe generic implementation within
CONFIG_KRETPROBES area.
- And original kprobe static inline functions have done like that,
in same header file, if no obvious reasons, we can try to follow.
I just concerned that it is a waste of memory if there are useless kretprobe
related instances are built when CONFIG_KRETPROBES=n.
Yeah, that is also one of reason (3rd reason).
And if
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
(2014/02/05 9:18), Chen Gang wrote:
On 02/04/2014 11:39 PM, Masami Hiramatsu wrote:
(2014/02/04 22:53), Chen Gang wrote:
On 02/04/2014 09:29 PM, Masami Hiramatsu wrote:
(2014/02/04 21:07), Chen Gang wrote:
On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
(2014/02/04 14:16), Chen Gang wrote:
When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
are useless, so need move them to CONFIG_KPROBES enabled area.
Now, *kretprobe* generic implementation are all implemented in 2 files:
- in include/linux/kprobes.h:
move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
move some *kprobe() declarations which kretprobe*() call, to front.
not touch kretprobe_blacklist[] which is architecture's variable.
- in kernel/kprobes.c:
move all kretprobe* to CONFIG_KPROBES area and dummy outside.
define kretprobe_flush_task() to let kprobe_flush_task() call.
define init_kretprobes() to let init_kprobes() call.
The patch passes compiling (get kernel/kprobes.o and kernel/built-
in.o) under avr32 and x86_64 allmodconfig, and passes building (get
bzImage and Modpost modules) under x86_64 defconfig.
Thanks for the fix! and I have some comments below.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
include/linux/kprobes.h | 58 +
kernel/kprobes.c| 328
+++-
2 files changed, 222 insertions(+), 164 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 925eaf2..c0d1212 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
return 1;
}
+int disable_kprobe(struct kprobe *kp);
+int enable_kprobe(struct kprobe *kp);
+
+void dump_kprobe(struct kprobe *kp);
+
+extern struct kretprobe_blackpoint kretprobe_blacklist[];
+
#ifdef CONFIG_KRETPROBES
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);
extern int arch_trampoline_kprobe(struct kprobe *p);
+static inline void kretprobe_assert(struct kretprobe_instance *ri,
+ unsigned long orig_ret_address, unsigned long trampoline_address)
+{
+ if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
+ printk(KERN_ERR
+ kretprobe BUG!: Processing kretprobe %p @ %p\n,
+ ri-rp, ri-rp-kp.addr);
+ BUG();
+ }
+}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return disable_kprobe(rp-kp);
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return enable_kprobe(rp-kp);
+}
+
#else /* CONFIG_KRETPROBES */
static inline void arch_prepare_kretprobe(struct kretprobe *rp,
struct pt_regs *regs)
@@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct
kprobe *p)
{
return 0;
}
-#endif /* CONFIG_KRETPROBES */
-
-extern struct kretprobe_blackpoint kretprobe_blacklist[];
-
static inline void kretprobe_assert(struct kretprobe_instance *ri,
unsigned long orig_ret_address, unsigned long trampoline_address)
{
- if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
- printk(kretprobe BUG!: Processing kretprobe %p @ %p\n,
- ri-rp, ri-rp-kp.addr);
- BUG();
- }
}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
No, these should returns -EINVAL or -ENOSYS, since these are user API.
OK, thanks, it sounds reasonable to me.
Anyway, I don't think those inlined functions to be changed, because
most of them are internal functions. If CONFIG_KRETPROBES=n, it just
be ignored.
In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
disable_kretprobe(), and enable_kretprobe() are not ignored.
Really? where are they called? I mean, those functions do not have
any instance unless your module uses it (but that is not what the kernel
itself should help).
If what you said correct (I guess so), for me, we still need let them in
CONFIG_KRETPROBES area, and without any dummy outside, just like another
*kprobe* static inline functions have done in include/linux/kprobes.h.
kretprobe_assert() is only for the internal check. So we don't need to care
about, and disable/enable_kretprobe() are anyway returns -EINVAL because
kretprobe can not be registered. And all of them are inlined functions.
In that case, we don't need to care about it.
Hmm... it is related with code 'consistency':
- these static inline functions are kretprobe generic implementation,
and we are trying to let all kretprobe generic implementation within
CONFIG_KRETPROBES area.
No, actually, kretprobe is just built on the kprobe. enable/disable_kretprobe
just wrapped the kprobe methods. And kretprobe_assert() is just for kretprobe
internal use. that is not an API. Moving only the kretprobe_assert() into the
CONFIG_KRETPROBE area is not bad, but since it is just a
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
On 02/05/2014 09:21 AM, Masami Hiramatsu wrote:
(2014/02/05 9:18), Chen Gang wrote:
On 02/04/2014 11:39 PM, Masami Hiramatsu wrote:
(2014/02/04 22:53), Chen Gang wrote:
On 02/04/2014 09:29 PM, Masami Hiramatsu wrote:
(2014/02/04 21:07), Chen Gang wrote:
On 02/04/2014 03:17 PM, Masami Hiramatsu wrote:
(2014/02/04 14:16), Chen Gang wrote:
When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
are useless, so need move them to CONFIG_KPROBES enabled area.
Now, *kretprobe* generic implementation are all implemented in 2 files:
- in include/linux/kprobes.h:
move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
move some *kprobe() declarations which kretprobe*() call, to
front.
not touch kretprobe_blacklist[] which is architecture's variable.
- in kernel/kprobes.c:
move all kretprobe* to CONFIG_KPROBES area and dummy outside.
define kretprobe_flush_task() to let kprobe_flush_task() call.
define init_kretprobes() to let init_kprobes() call.
The patch passes compiling (get kernel/kprobes.o and kernel/built-
in.o) under avr32 and x86_64 allmodconfig, and passes building (get
bzImage and Modpost modules) under x86_64 defconfig.
Thanks for the fix! and I have some comments below.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
include/linux/kprobes.h | 58 +
kernel/kprobes.c| 328
+++-
2 files changed, 222 insertions(+), 164 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 925eaf2..c0d1212 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
return 1;
}
+int disable_kprobe(struct kprobe *kp);
+int enable_kprobe(struct kprobe *kp);
+
+void dump_kprobe(struct kprobe *kp);
+
+extern struct kretprobe_blackpoint kretprobe_blacklist[];
+
#ifdef CONFIG_KRETPROBES
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);
extern int arch_trampoline_kprobe(struct kprobe *p);
+static inline void kretprobe_assert(struct kretprobe_instance *ri,
+ unsigned long orig_ret_address, unsigned long trampoline_address)
+{
+ if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
+ printk(KERN_ERR
+ kretprobe BUG!: Processing kretprobe %p @ %p\n,
+ ri-rp, ri-rp-kp.addr);
+ BUG();
+ }
+}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return disable_kprobe(rp-kp);
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return enable_kprobe(rp-kp);
+}
+
#else /* CONFIG_KRETPROBES */
static inline void arch_prepare_kretprobe(struct kretprobe *rp,
struct pt_regs *regs)
@@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct
kprobe *p)
{
return 0;
}
-#endif /* CONFIG_KRETPROBES */
-
-extern struct kretprobe_blackpoint kretprobe_blacklist[];
-
static inline void kretprobe_assert(struct kretprobe_instance *ri,
unsigned long orig_ret_address, unsigned long trampoline_address)
{
- if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
- printk(kretprobe BUG!: Processing kretprobe %p @ %p\n,
- ri-rp, ri-rp-kp.addr);
- BUG();
- }
}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
No, these should returns -EINVAL or -ENOSYS, since these are user API.
OK, thanks, it sounds reasonable to me.
Anyway, I don't think those inlined functions to be changed, because
most of them are internal functions. If CONFIG_KRETPROBES=n, it just
be ignored.
In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(),
disable_kretprobe(), and enable_kretprobe() are not ignored.
Really? where are they called? I mean, those functions do not have
any instance unless your module uses it (but that is not what the kernel
itself should help).
If what you said correct (I guess so), for me, we still need let them in
CONFIG_KRETPROBES area, and without any dummy outside, just like another
*kprobe* static inline functions have done in include/linux/kprobes.h.
kretprobe_assert() is only for the internal check. So we don't need to care
about, and disable/enable_kretprobe() are anyway returns -EINVAL because
kretprobe can not be registered. And all of them are inlined functions.
In that case, we don't need to care about it.
Hmm... it is related with code 'consistency':
- these static inline functions are kretprobe generic implementation,
and we are trying to let all kretprobe generic implementation within
CONFIG_KRETPROBES area.
No, actually, kretprobe is just built on the kprobe. enable/disable_kretprobe
just wrapped the kprobe methods. And kretprobe_assert() is just for kretprobe
internal use. that is not an API. Moving only the kretprobe_assert() into the
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
(2014/02/05 12:08), Chen Gang wrote: Anyway, I don't think those inlined functions to be changed, because most of them are internal functions. If CONFIG_KRETPROBES=n, it just be ignored. In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(), disable_kretprobe(), and enable_kretprobe() are not ignored. Really? where are they called? I mean, those functions do not have any instance unless your module uses it (but that is not what the kernel itself should help). If what you said correct (I guess so), for me, we still need let them in CONFIG_KRETPROBES area, and without any dummy outside, just like another *kprobe* static inline functions have done in include/linux/kprobes.h. kretprobe_assert() is only for the internal check. So we don't need to care about, and disable/enable_kretprobe() are anyway returns -EINVAL because kretprobe can not be registered. And all of them are inlined functions. In that case, we don't need to care about it. Hmm... it is related with code 'consistency': - these static inline functions are kretprobe generic implementation, and we are trying to let all kretprobe generic implementation within CONFIG_KRETPROBES area. No, actually, kretprobe is just built on the kprobe. enable/disable_kretprobe just wrapped the kprobe methods. And kretprobe_assert() is just for kretprobe internal use. that is not an API. Moving only the kretprobe_assert() into the CONFIG_KRETPROBE area is not bad, but since it is just a static inline function, if there is no caller, it just be ignored, no side effect. OK, I can understand. And do you mean enable/disable_kretprobe() are API? if so, we have to implement them whether CONFIG_KRETPROBES enabled or disabled. And when CONFIG_KRETPROBES=n, just like what you originally said: we need returns -EINVAL directly (either, I am not quite sure whether the input parameter will be NULL, in this case). Both are API, and when implementing it I had also considered that, but I decided to stay it in inline-function wrapper. The reason why is, that enable/disable_k*probe require the registered k*probes. If the kernel hacker uses those functions, they must ensure registering his k*probes, otherwise it does not work correctly. If the CONFIG_KRETPROBES=n, register_kretprobe() always fails, this means that the code has no chance to call those functions (it must be). - And original kprobe static inline functions have done like that, in same header file, if no obvious reasons, we can try to follow. It is no reasons to follow that too. Please keep your patch simple as much as possible. keep our patch simple as much as posssible sounds reasonable to me. After skip include/linux/kprobe.h, our patch's subject (include comments) also need be changed (I will/should change it). For me, include/linux/kprobe.h can also be improved, but it can be another patch for it (not only for kretprobe, but also for jpobe). if that improvement means simplify, it is acceptable. Now I don't like ifdefs of CONFIG_KPROBES and dummy functions, since if CONFIG_KPROBES=n, other kernel modules can also check the kconfig and decide what they do (or don't). Perhaps, what we've really needed is just enough able to compile, not the fully covered dummy APIs. I just concerned that it is a waste of memory if there are useless kretprobe related instances are built when CONFIG_KRETPROBES=n. Yeah, that is also one of reason (3rd reason). And if necessary, please help check what we have done whether already let all kretprobe generic implementation within CONFIG_KRETPROBES area (exclude declaration, struct/union definition, and architecture implementation). As I commented, your changes in kernel/kprobes.c are good to me except two functions. That's all what we need to fix :) I will send a patch for it (since subject changed, we need not mark patch v2), thanks. :-) OK, I'll review that. Thank you! -- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu...@hitachi.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
On 02/05/2014 12:57 PM, Masami Hiramatsu wrote: (2014/02/05 12:08), Chen Gang wrote: Anyway, I don't think those inlined functions to be changed, because most of them are internal functions. If CONFIG_KRETPROBES=n, it just be ignored. In original implementation, if CONFIG_KRETPROBES=n, kretprobe_assert(), disable_kretprobe(), and enable_kretprobe() are not ignored. Really? where are they called? I mean, those functions do not have any instance unless your module uses it (but that is not what the kernel itself should help). If what you said correct (I guess so), for me, we still need let them in CONFIG_KRETPROBES area, and without any dummy outside, just like another *kprobe* static inline functions have done in include/linux/kprobes.h. kretprobe_assert() is only for the internal check. So we don't need to care about, and disable/enable_kretprobe() are anyway returns -EINVAL because kretprobe can not be registered. And all of them are inlined functions. In that case, we don't need to care about it. Hmm... it is related with code 'consistency': - these static inline functions are kretprobe generic implementation, and we are trying to let all kretprobe generic implementation within CONFIG_KRETPROBES area. No, actually, kretprobe is just built on the kprobe. enable/disable_kretprobe just wrapped the kprobe methods. And kretprobe_assert() is just for kretprobe internal use. that is not an API. Moving only the kretprobe_assert() into the CONFIG_KRETPROBE area is not bad, but since it is just a static inline function, if there is no caller, it just be ignored, no side effect. OK, I can understand. And do you mean enable/disable_kretprobe() are API? if so, we have to implement them whether CONFIG_KRETPROBES enabled or disabled. And when CONFIG_KRETPROBES=n, just like what you originally said: we need returns -EINVAL directly (either, I am not quite sure whether the input parameter will be NULL, in this case). Both are API, and when implementing it I had also considered that, but I decided to stay it in inline-function wrapper. The reason why is, that enable/disable_k*probe require the registered k*probes. If the kernel hacker uses those functions, they must ensure registering his k*probes, otherwise it does not work correctly. If the CONFIG_KRETPROBES=n, register_kretprobe() always fails, this means that the code has no chance to call those functions (it must be). OK, thank you for your explanations. - And original kprobe static inline functions have done like that, in same header file, if no obvious reasons, we can try to follow. It is no reasons to follow that too. Please keep your patch simple as much as possible. keep our patch simple as much as posssible sounds reasonable to me. After skip include/linux/kprobe.h, our patch's subject (include comments) also need be changed (I will/should change it). For me, include/linux/kprobe.h can also be improved, but it can be another patch for it (not only for kretprobe, but also for jpobe). if that improvement means simplify, it is acceptable. Now I don't like ifdefs of CONFIG_KPROBES and dummy functions, since if CONFIG_KPROBES=n, other kernel modules can also check the kconfig and decide what they do (or don't). Perhaps, what we've really needed is just enough able to compile, not the fully covered dummy APIs. Hmm... for me, I still try to send a patch for include/linux/kprobe.h. For API (although it is kernel internal API), I have a hobby to try to let it 'beautiful' as much as possible. I just concerned that it is a waste of memory if there are useless kretprobe related instances are built when CONFIG_KRETPROBES=n. Yeah, that is also one of reason (3rd reason). And if necessary, please help check what we have done whether already let all kretprobe generic implementation within CONFIG_KRETPROBES area (exclude declaration, struct/union definition, and architecture implementation). As I commented, your changes in kernel/kprobes.c are good to me except two functions. That's all what we need to fix :) I will send a patch for it (since subject changed, we need not mark patch v2), thanks. :-) OK, I'll review that. Thanks. Thank you! Thanks. -- Chen Gang Open, share and attitude like air, water and life which God blessed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
(2014/02/04 14:16), Chen Gang wrote:
> When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
> are useless, so need move them to CONFIG_KPROBES enabled area.
>
> Now, *kretprobe* generic implementation are all implemented in 2 files:
>
> - in "include/linux/kprobes.h":
>
> move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
> move some *kprobe() declarations which kretprobe*() call, to front.
> not touch kretprobe_blacklist[] which is architecture's variable.
>
> - in "kernel/kprobes.c":
>
> move all kretprobe* to CONFIG_KPROBES area and dummy outside.
> define kretprobe_flush_task() to let kprobe_flush_task() call.
> define init_kretprobes() to let init_kprobes() call.
>
> The patch passes compiling (get "kernel/kprobes.o" and "kernel/built-
> in.o") under avr32 and x86_64 allmodconfig, and passes building (get
> bzImage and Modpost modules) under x86_64 defconfig.
Thanks for the fix! and I have some comments below.
> Signed-off-by: Chen Gang
> ---
> include/linux/kprobes.h | 58 +
> kernel/kprobes.c| 328
> +++-
> 2 files changed, 222 insertions(+), 164 deletions(-)
>
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 925eaf2..c0d1212 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
> return 1;
> }
>
> +int disable_kprobe(struct kprobe *kp);
> +int enable_kprobe(struct kprobe *kp);
> +
> +void dump_kprobe(struct kprobe *kp);
> +
> +extern struct kretprobe_blackpoint kretprobe_blacklist[];
> +
> #ifdef CONFIG_KRETPROBES
> extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
> struct pt_regs *regs);
> extern int arch_trampoline_kprobe(struct kprobe *p);
> +static inline void kretprobe_assert(struct kretprobe_instance *ri,
> + unsigned long orig_ret_address, unsigned long trampoline_address)
> +{
> + if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
> + printk(KERN_ERR
> + "kretprobe BUG!: Processing kretprobe %p @ %p\n",
> + ri->rp, ri->rp->kp.addr);
> + BUG();
> + }
> +}
> +static inline int disable_kretprobe(struct kretprobe *rp)
> +{
> + return disable_kprobe(>kp);
> +}
> +static inline int enable_kretprobe(struct kretprobe *rp)
> +{
> + return enable_kprobe(>kp);
> +}
> +
> #else /* CONFIG_KRETPROBES */
> static inline void arch_prepare_kretprobe(struct kretprobe *rp,
> struct pt_regs *regs)
> @@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct kprobe
> *p)
> {
> return 0;
> }
> -#endif /* CONFIG_KRETPROBES */
> -
> -extern struct kretprobe_blackpoint kretprobe_blacklist[];
> -
> static inline void kretprobe_assert(struct kretprobe_instance *ri,
> unsigned long orig_ret_address, unsigned long trampoline_address)
> {
> - if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
> - printk("kretprobe BUG!: Processing kretprobe %p @ %p\n",
> - ri->rp, ri->rp->kp.addr);
> - BUG();
> - }
> }
> +static inline int disable_kretprobe(struct kretprobe *rp)
> +{
> + return 0;
> +}
> +static inline int enable_kretprobe(struct kretprobe *rp)
> +{
> + return 0;
> +}
No, these should returns -EINVAL or -ENOSYS, since these are user API.
Anyway, I don't think those inlined functions to be changed, because
most of them are internal functions. If CONFIG_KRETPROBES=n, it just
be ignored.
So, I think you don't need to change kprobes.h.
> +
> +#endif /* CONFIG_KRETPROBES */
>
> #ifdef CONFIG_KPROBES_SANITY_TEST
> extern int init_test_probes(void);
> @@ -379,11 +406,6 @@ void unregister_kretprobes(struct kretprobe **rps, int
> num);
> void kprobe_flush_task(struct task_struct *tk);
> 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)
> @@ -459,14 +481,6 @@ static inline int enable_kprobe(struct kprobe *kp)
> return -ENOSYS;
> }
> #endif /* CONFIG_KPROBES */
> -static inline int disable_kretprobe(struct kretprobe *rp)
> -{
> - return disable_kprobe(>kp);
> -}
> -static inline int enable_kretprobe(struct kretprobe *rp)
> -{
> - return enable_kprobe(>kp);
> -}
> static inline int disable_jprobe(struct jprobe *jp)
> {
> return disable_kprobe(>kp);
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index ceeadfc..e305a81 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
[...]
> @@ -1936,8 +1955,44 @@ static int __kprobes pre_handler_kretprobe(struct
> kprobe *p,
> return 0;
> }
>
>
[PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
are useless, so need move them to CONFIG_KPROBES enabled area.
Now, *kretprobe* generic implementation are all implemented in 2 files:
- in "include/linux/kprobes.h":
move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
move some *kprobe() declarations which kretprobe*() call, to front.
not touch kretprobe_blacklist[] which is architecture's variable.
- in "kernel/kprobes.c":
move all kretprobe* to CONFIG_KPROBES area and dummy outside.
define kretprobe_flush_task() to let kprobe_flush_task() call.
define init_kretprobes() to let init_kprobes() call.
The patch passes compiling (get "kernel/kprobes.o" and "kernel/built-
in.o") under avr32 and x86_64 allmodconfig, and passes building (get
bzImage and Modpost modules) under x86_64 defconfig.
Signed-off-by: Chen Gang
---
include/linux/kprobes.h | 58 +
kernel/kprobes.c| 328 +++-
2 files changed, 222 insertions(+), 164 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 925eaf2..c0d1212 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
return 1;
}
+int disable_kprobe(struct kprobe *kp);
+int enable_kprobe(struct kprobe *kp);
+
+void dump_kprobe(struct kprobe *kp);
+
+extern struct kretprobe_blackpoint kretprobe_blacklist[];
+
#ifdef CONFIG_KRETPROBES
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);
extern int arch_trampoline_kprobe(struct kprobe *p);
+static inline void kretprobe_assert(struct kretprobe_instance *ri,
+ unsigned long orig_ret_address, unsigned long trampoline_address)
+{
+ if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
+ printk(KERN_ERR
+ "kretprobe BUG!: Processing kretprobe %p @ %p\n",
+ ri->rp, ri->rp->kp.addr);
+ BUG();
+ }
+}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return disable_kprobe(>kp);
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return enable_kprobe(>kp);
+}
+
#else /* CONFIG_KRETPROBES */
static inline void arch_prepare_kretprobe(struct kretprobe *rp,
struct pt_regs *regs)
@@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct kprobe *p)
{
return 0;
}
-#endif /* CONFIG_KRETPROBES */
-
-extern struct kretprobe_blackpoint kretprobe_blacklist[];
-
static inline void kretprobe_assert(struct kretprobe_instance *ri,
unsigned long orig_ret_address, unsigned long trampoline_address)
{
- if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
- printk("kretprobe BUG!: Processing kretprobe %p @ %p\n",
- ri->rp, ri->rp->kp.addr);
- BUG();
- }
}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
+
+#endif /* CONFIG_KRETPROBES */
#ifdef CONFIG_KPROBES_SANITY_TEST
extern int init_test_probes(void);
@@ -379,11 +406,6 @@ void unregister_kretprobes(struct kretprobe **rps, int
num);
void kprobe_flush_task(struct task_struct *tk);
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)
@@ -459,14 +481,6 @@ static inline int enable_kprobe(struct kprobe *kp)
return -ENOSYS;
}
#endif /* CONFIG_KPROBES */
-static inline int disable_kretprobe(struct kretprobe *rp)
-{
- return disable_kprobe(>kp);
-}
-static inline int enable_kretprobe(struct kretprobe *rp)
-{
- return enable_kprobe(>kp);
-}
static inline int disable_jprobe(struct jprobe *jp)
{
return disable_kprobe(>kp);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index ceeadfc..e305a81 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -69,7 +69,6 @@
static int kprobes_initialized;
static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE];
-static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
/* NOTE: change this value only with kprobe_mutex held */
static bool kprobes_all_disarmed;
@@ -77,14 +76,6 @@ static bool kprobes_all_disarmed;
/* This protects kprobe_table and optimizing_list */
static DEFINE_MUTEX(kprobe_mutex);
static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
-static struct {
- raw_spinlock_t lock cacheline_aligned_in_smp;
-} kretprobe_table_locks[KPROBE_TABLE_SIZE];
-
-static raw_spinlock_t *kretprobe_table_lock_ptr(unsigned long hash)
-{
- return
[PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
are useless, so need move them to CONFIG_KPROBES enabled area.
Now, *kretprobe* generic implementation are all implemented in 2 files:
- in include/linux/kprobes.h:
move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
move some *kprobe() declarations which kretprobe*() call, to front.
not touch kretprobe_blacklist[] which is architecture's variable.
- in kernel/kprobes.c:
move all kretprobe* to CONFIG_KPROBES area and dummy outside.
define kretprobe_flush_task() to let kprobe_flush_task() call.
define init_kretprobes() to let init_kprobes() call.
The patch passes compiling (get kernel/kprobes.o and kernel/built-
in.o) under avr32 and x86_64 allmodconfig, and passes building (get
bzImage and Modpost modules) under x86_64 defconfig.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
include/linux/kprobes.h | 58 +
kernel/kprobes.c| 328 +++-
2 files changed, 222 insertions(+), 164 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 925eaf2..c0d1212 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
return 1;
}
+int disable_kprobe(struct kprobe *kp);
+int enable_kprobe(struct kprobe *kp);
+
+void dump_kprobe(struct kprobe *kp);
+
+extern struct kretprobe_blackpoint kretprobe_blacklist[];
+
#ifdef CONFIG_KRETPROBES
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);
extern int arch_trampoline_kprobe(struct kprobe *p);
+static inline void kretprobe_assert(struct kretprobe_instance *ri,
+ unsigned long orig_ret_address, unsigned long trampoline_address)
+{
+ if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
+ printk(KERN_ERR
+ kretprobe BUG!: Processing kretprobe %p @ %p\n,
+ ri-rp, ri-rp-kp.addr);
+ BUG();
+ }
+}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return disable_kprobe(rp-kp);
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return enable_kprobe(rp-kp);
+}
+
#else /* CONFIG_KRETPROBES */
static inline void arch_prepare_kretprobe(struct kretprobe *rp,
struct pt_regs *regs)
@@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct kprobe *p)
{
return 0;
}
-#endif /* CONFIG_KRETPROBES */
-
-extern struct kretprobe_blackpoint kretprobe_blacklist[];
-
static inline void kretprobe_assert(struct kretprobe_instance *ri,
unsigned long orig_ret_address, unsigned long trampoline_address)
{
- if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
- printk(kretprobe BUG!: Processing kretprobe %p @ %p\n,
- ri-rp, ri-rp-kp.addr);
- BUG();
- }
}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
+
+#endif /* CONFIG_KRETPROBES */
#ifdef CONFIG_KPROBES_SANITY_TEST
extern int init_test_probes(void);
@@ -379,11 +406,6 @@ void unregister_kretprobes(struct kretprobe **rps, int
num);
void kprobe_flush_task(struct task_struct *tk);
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)
@@ -459,14 +481,6 @@ static inline int enable_kprobe(struct kprobe *kp)
return -ENOSYS;
}
#endif /* CONFIG_KPROBES */
-static inline int disable_kretprobe(struct kretprobe *rp)
-{
- return disable_kprobe(rp-kp);
-}
-static inline int enable_kretprobe(struct kretprobe *rp)
-{
- return enable_kprobe(rp-kp);
-}
static inline int disable_jprobe(struct jprobe *jp)
{
return disable_kprobe(jp-kp);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index ceeadfc..e305a81 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -69,7 +69,6 @@
static int kprobes_initialized;
static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE];
-static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
/* NOTE: change this value only with kprobe_mutex held */
static bool kprobes_all_disarmed;
@@ -77,14 +76,6 @@ static bool kprobes_all_disarmed;
/* This protects kprobe_table and optimizing_list */
static DEFINE_MUTEX(kprobe_mutex);
static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
-static struct {
- raw_spinlock_t lock cacheline_aligned_in_smp;
-} kretprobe_table_locks[KPROBE_TABLE_SIZE];
-
-static raw_spinlock_t *kretprobe_table_lock_ptr(unsigned long hash)
-{
-
Re: [PATCH] kernel: kprobe: move all *kretprobe* generic implementation to CONFIG_KRETPROBES enabled area
(2014/02/04 14:16), Chen Gang wrote:
When CONFIG_KRETPROBES disabled, all *kretprobe* generic implementation
are useless, so need move them to CONFIG_KPROBES enabled area.
Now, *kretprobe* generic implementation are all implemented in 2 files:
- in include/linux/kprobes.h:
move inline kretprobe*() to CONFIG_KPROBES area and dummy outside.
move some *kprobe() declarations which kretprobe*() call, to front.
not touch kretprobe_blacklist[] which is architecture's variable.
- in kernel/kprobes.c:
move all kretprobe* to CONFIG_KPROBES area and dummy outside.
define kretprobe_flush_task() to let kprobe_flush_task() call.
define init_kretprobes() to let init_kprobes() call.
The patch passes compiling (get kernel/kprobes.o and kernel/built-
in.o) under avr32 and x86_64 allmodconfig, and passes building (get
bzImage and Modpost modules) under x86_64 defconfig.
Thanks for the fix! and I have some comments below.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
include/linux/kprobes.h | 58 +
kernel/kprobes.c| 328
+++-
2 files changed, 222 insertions(+), 164 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 925eaf2..c0d1212 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -223,10 +223,36 @@ static inline int kprobes_built_in(void)
return 1;
}
+int disable_kprobe(struct kprobe *kp);
+int enable_kprobe(struct kprobe *kp);
+
+void dump_kprobe(struct kprobe *kp);
+
+extern struct kretprobe_blackpoint kretprobe_blacklist[];
+
#ifdef CONFIG_KRETPROBES
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);
extern int arch_trampoline_kprobe(struct kprobe *p);
+static inline void kretprobe_assert(struct kretprobe_instance *ri,
+ unsigned long orig_ret_address, unsigned long trampoline_address)
+{
+ if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
+ printk(KERN_ERR
+ kretprobe BUG!: Processing kretprobe %p @ %p\n,
+ ri-rp, ri-rp-kp.addr);
+ BUG();
+ }
+}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return disable_kprobe(rp-kp);
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return enable_kprobe(rp-kp);
+}
+
#else /* CONFIG_KRETPROBES */
static inline void arch_prepare_kretprobe(struct kretprobe *rp,
struct pt_regs *regs)
@@ -236,19 +262,20 @@ static inline int arch_trampoline_kprobe(struct kprobe
*p)
{
return 0;
}
-#endif /* CONFIG_KRETPROBES */
-
-extern struct kretprobe_blackpoint kretprobe_blacklist[];
-
static inline void kretprobe_assert(struct kretprobe_instance *ri,
unsigned long orig_ret_address, unsigned long trampoline_address)
{
- if (!orig_ret_address || (orig_ret_address == trampoline_address)) {
- printk(kretprobe BUG!: Processing kretprobe %p @ %p\n,
- ri-rp, ri-rp-kp.addr);
- BUG();
- }
}
+static inline int disable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
+static inline int enable_kretprobe(struct kretprobe *rp)
+{
+ return 0;
+}
No, these should returns -EINVAL or -ENOSYS, since these are user API.
Anyway, I don't think those inlined functions to be changed, because
most of them are internal functions. If CONFIG_KRETPROBES=n, it just
be ignored.
So, I think you don't need to change kprobes.h.
+
+#endif /* CONFIG_KRETPROBES */
#ifdef CONFIG_KPROBES_SANITY_TEST
extern int init_test_probes(void);
@@ -379,11 +406,6 @@ void unregister_kretprobes(struct kretprobe **rps, int
num);
void kprobe_flush_task(struct task_struct *tk);
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)
@@ -459,14 +481,6 @@ static inline int enable_kprobe(struct kprobe *kp)
return -ENOSYS;
}
#endif /* CONFIG_KPROBES */
-static inline int disable_kretprobe(struct kretprobe *rp)
-{
- return disable_kprobe(rp-kp);
-}
-static inline int enable_kretprobe(struct kretprobe *rp)
-{
- return enable_kprobe(rp-kp);
-}
static inline int disable_jprobe(struct jprobe *jp)
{
return disable_kprobe(jp-kp);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index ceeadfc..e305a81 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
[...]
@@ -1936,8 +1955,44 @@ static int __kprobes pre_handler_kretprobe(struct
kprobe *p,
return 0;
}
+void __kprobes recycle_rp_inst(struct kretprobe_instance *ri,
+ struct hlist_head *head)
+{
