Hi,
This is the second posting of patches previously posted at
[0]. Although the patches were reviewed / acked in the previous cycle
but for some didn't end up getting picked up for this cycle.
This posting rebases the patches to 5.14-rc3 and makes some minor
improvements to the commit log in Patch 1. I've also added the tags as
appropriate from the previous posting.
It would be great if the patches can be picked up this time around.
Thanks,
Punit
[0] https://lore.kernel.org/linux-csky/[email protected]/
Punit Agrawal (5):
kprobes: Do not use local variable when creating debugfs file
kprobes: Use helper to parse boolean input from userspace
kprobe: Simplify prepare_kprobe() by dropping redundant version
csky: ftrace: Drop duplicate implementation of
arch_check_ftrace_location()
kprobes: Make arch_check_ftrace_location static
arch/csky/kernel/probes/ftrace.c | 7 ----
include/linux/kprobes.h | 7 ++--
kernel/kprobes.c | 58 ++++++++++----------------------
3 files changed, 23 insertions(+), 49 deletions(-)
--
2.30.2
The function prepare_kprobe() is called during kprobe registration and
is responsible for ensuring any architecture related preparation for
the kprobe is done before returning.
One of two versions of prepare_kprobe() is chosen depending on the
availability of KPROBE_ON_FTRACE in the kernel configuration.
Simplify the code by dropping the version when KPROBE_ON_FTRACE is not
selected - instead relying on kprobe_ftrace() to return false when
KPROBE_ON_FTRACE is not set.
No functional change.
Signed-off-by: Punit Agrawal <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
---
include/linux/kprobes.h | 5 +++++
kernel/kprobes.c | 23 +++++++++--------------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index e4f3bfe08757..0b75549b2815 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -354,6 +354,11 @@ static inline void wait_for_kprobe_optimizer(void) { }
extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *ops, struct ftrace_regs *fregs);
extern int arch_prepare_kprobe_ftrace(struct kprobe *p);
+#else
+static inline int arch_prepare_kprobe_ftrace(struct kprobe *p)
+{
+ return -EINVAL;
+}
#endif
int arch_check_ftrace_location(struct kprobe *p);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 26fc9904c3b1..cfa9d3c263eb 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1033,15 +1033,6 @@ static struct ftrace_ops kprobe_ipmodify_ops __read_mostly = {
static int kprobe_ipmodify_enabled;
static int kprobe_ftrace_enabled;
-/* Must ensure p->addr is really on ftrace */
-static int prepare_kprobe(struct kprobe *p)
-{
- if (!kprobe_ftrace(p))
- return arch_prepare_kprobe(p);
-
- return arch_prepare_kprobe_ftrace(p);
-}
-
/* Caller must lock kprobe_mutex */
static int __arm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
int *cnt)
@@ -1113,11 +1104,6 @@ static int disarm_kprobe_ftrace(struct kprobe *p)
ipmodify ? &kprobe_ipmodify_enabled : &kprobe_ftrace_enabled);
}
#else /* !CONFIG_KPROBES_ON_FTRACE */
-static inline int prepare_kprobe(struct kprobe *p)
-{
- return arch_prepare_kprobe(p);
-}
-
static inline int arm_kprobe_ftrace(struct kprobe *p)
{
return -ENODEV;
@@ -1129,6 +1115,15 @@ static inline int disarm_kprobe_ftrace(struct kprobe *p)
}
#endif
+static int prepare_kprobe(struct kprobe *p)
+{
+ /* Must ensure p->addr is really on ftrace */
+ if (kprobe_ftrace(p))
+ return arch_prepare_kprobe_ftrace(p);
+
+ return arch_prepare_kprobe(p);
+}
+
/* Arm a kprobe with text_mutex */
static int arm_kprobe(struct kprobe *kp)
{
--
2.30.2
arch_check_ftrace_location() was introduced as a weak function in
commit f7f242ff004499 ("kprobes: introduce weak
arch_check_ftrace_location() helper function") to allow architectures
to handle kprobes call site on their own.
Recently, the only architecture (csky) to implement
arch_check_ftrace_location() was migrated to using the common
version.
As a result, further cleanup the code to drop the weak attribute and
rename the function to remove the architecture specific
implementation.
Signed-off-by: Punit Agrawal <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
---
include/linux/kprobes.h | 2 --
kernel/kprobes.c | 4 ++--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 0b75549b2815..8a9412bb0d5e 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -361,8 +361,6 @@ static inline int arch_prepare_kprobe_ftrace(struct kprobe *p)
}
#endif
-int arch_check_ftrace_location(struct kprobe *p);
-
/* Get the kprobe at this addr (if any) - called with preemption disabled */
struct kprobe *get_kprobe(void *addr);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index cfa9d3c263eb..30199bfcc74a 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1524,7 +1524,7 @@ static inline int warn_kprobe_rereg(struct kprobe *p)
return ret;
}
-int __weak arch_check_ftrace_location(struct kprobe *p)
+static int check_ftrace_location(struct kprobe *p)
{
unsigned long ftrace_addr;
@@ -1547,7 +1547,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
{
int ret;
- ret = arch_check_ftrace_location(p);
+ ret = check_ftrace_location(p);
if (ret)
return ret;
jump_label_lock();
--
2.30.2
The "enabled" file provides a debugfs interface to arm / disarm
kprobes in the kernel. In order to parse the buffer containing the
values written from userspace, the callback manually parses the user
input to convert it to a boolean value.
As taking a string value from userspace and converting it to boolean
is a common operation, a helper kstrtobool_from_user() is already
available in the kernel. Update the callback to use the common helper
to parse the write buffer from userspace.
Signed-off-by: Punit Agrawal <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
---
kernel/kprobes.c | 28 ++++++----------------------
1 file changed, 6 insertions(+), 22 deletions(-)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 1cf8bca1ea86..26fc9904c3b1 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -2770,30 +2770,14 @@ static ssize_t read_enabled_file_bool(struct file *file,
static ssize_t write_enabled_file_bool(struct file *file,
const char __user *user_buf, size_t count, loff_t *ppos)
{
- char buf[32];
- size_t buf_size;
- int ret = 0;
-
- buf_size = min(count, (sizeof(buf)-1));
- if (copy_from_user(buf, user_buf, buf_size))
- return -EFAULT;
+ bool enable;
+ int ret;
- buf[buf_size] = '\0';
- switch (buf[0]) {
- case 'y':
- case 'Y':
- case '1':
- ret = arm_all_kprobes();
- break;
- case 'n':
- case 'N':
- case '0':
- ret = disarm_all_kprobes();
- break;
- default:
- return -EINVAL;
- }
+ ret = kstrtobool_from_user(user_buf, count, &enable);
+ if (ret)
+ return ret;
+ ret = enable ? arm_all_kprobes() : disarm_all_kprobes();
if (ret)
return ret;
--
2.30.2
The csky specific arch_check_ftrace_location() shadows a weak
implementation of the function in core code that offers the same
functionality but with additional error checking.
Drop the architecture specific function as a step towards further
cleanup in core code.
Signed-off-by: Punit Agrawal <[email protected]>
Acked-by: Guo Ren <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
---
arch/csky/kernel/probes/ftrace.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/arch/csky/kernel/probes/ftrace.c b/arch/csky/kernel/probes/ftrace.c
index ef2bb9bd9605..b388228abbf2 100644
--- a/arch/csky/kernel/probes/ftrace.c
+++ b/arch/csky/kernel/probes/ftrace.c
@@ -2,13 +2,6 @@
#include <linux/kprobes.h>
-int arch_check_ftrace_location(struct kprobe *p)
-{
- if (ftrace_location((unsigned long)p->addr))
- p->flags |= KPROBE_FLAG_FTRACE;
- return 0;
-}
-
/* Ftrace callback handler for kprobes -- called under preepmt disabled */
void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *ops, struct ftrace_regs *fregs)
--
2.30.2
Hi Punit,
Thanks for resending this series.
Ingo, could you pick this patch too?
I'll resend my series on this series.
Thank you,
On Tue, 27 Jul 2021 22:34:21 +0900
Punit Agrawal <[email protected]> wrote:
> Hi,
>
> This is the second posting of patches previously posted at
> [0]. Although the patches were reviewed / acked in the previous cycle
> but for some didn't end up getting picked up for this cycle.
>
> This posting rebases the patches to 5.14-rc3 and makes some minor
> improvements to the commit log in Patch 1. I've also added the tags as
> appropriate from the previous posting.
>
> It would be great if the patches can be picked up this time around.
>
> Thanks,
> Punit
>
>
> [0] https://lore.kernel.org/linux-csky/[email protected]/
>
> Punit Agrawal (5):
> kprobes: Do not use local variable when creating debugfs file
> kprobes: Use helper to parse boolean input from userspace
> kprobe: Simplify prepare_kprobe() by dropping redundant version
> csky: ftrace: Drop duplicate implementation of
> arch_check_ftrace_location()
> kprobes: Make arch_check_ftrace_location static
>
> arch/csky/kernel/probes/ftrace.c | 7 ----
> include/linux/kprobes.h | 7 ++--
> kernel/kprobes.c | 58 ++++++++++----------------------
> 3 files changed, 23 insertions(+), 49 deletions(-)
>
> --
> 2.30.2
>
--
Masami Hiramatsu <[email protected]>
Acked, thx.
On Tue, Jul 27, 2021 at 9:35 PM Punit Agrawal <[email protected]> wrote:
>
> The csky specific arch_check_ftrace_location() shadows a weak
> implementation of the function in core code that offers the same
> functionality but with additional error checking.
>
> Drop the architecture specific function as a step towards further
> cleanup in core code.
>
> Signed-off-by: Punit Agrawal <[email protected]>
> Acked-by: Guo Ren <[email protected]>
> Acked-by: Masami Hiramatsu <[email protected]>
> ---
> arch/csky/kernel/probes/ftrace.c | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/arch/csky/kernel/probes/ftrace.c b/arch/csky/kernel/probes/ftrace.c
> index ef2bb9bd9605..b388228abbf2 100644
> --- a/arch/csky/kernel/probes/ftrace.c
> +++ b/arch/csky/kernel/probes/ftrace.c
> @@ -2,13 +2,6 @@
>
> #include <linux/kprobes.h>
>
> -int arch_check_ftrace_location(struct kprobe *p)
> -{
> - if (ftrace_location((unsigned long)p->addr))
> - p->flags |= KPROBE_FLAG_FTRACE;
> - return 0;
> -}
> -
> /* Ftrace callback handler for kprobes -- called under preepmt disabled */
> void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> struct ftrace_ops *ops, struct ftrace_regs *fregs)
> --
> 2.30.2
>
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
Masami Hiramatsu <[email protected]> writes:
> Hi Punit,
>
> Thanks for resending this series.
>
> Ingo, could you pick this patch too?
> I'll resend my series on this series.
I couldn't find the patches on any of the branches in tip - have they
been queued?
Please let me know if there anything else I need to do in order for them
to be picked up.
Thanks,
Punit
>
> Thank you,
>
> On Tue, 27 Jul 2021 22:34:21 +0900
> Punit Agrawal <[email protected]> wrote:
>
>> Hi,
>>
>> This is the second posting of patches previously posted at
>> [0]. Although the patches were reviewed / acked in the previous cycle
>> but for some didn't end up getting picked up for this cycle.
>>
>> This posting rebases the patches to 5.14-rc3 and makes some minor
>> improvements to the commit log in Patch 1. I've also added the tags as
>> appropriate from the previous posting.
>>
>> It would be great if the patches can be picked up this time around.
>>
>> Thanks,
>> Punit
>>
>>
>> [0] https://lore.kernel.org/linux-csky/[email protected]/
>>
>> Punit Agrawal (5):
>> kprobes: Do not use local variable when creating debugfs file
>> kprobes: Use helper to parse boolean input from userspace
>> kprobe: Simplify prepare_kprobe() by dropping redundant version
>> csky: ftrace: Drop duplicate implementation of
>> arch_check_ftrace_location()
>> kprobes: Make arch_check_ftrace_location static
>>
>> arch/csky/kernel/probes/ftrace.c | 7 ----
>> include/linux/kprobes.h | 7 ++--
>> kernel/kprobes.c | 58 ++++++++++----------------------
>> 3 files changed, 23 insertions(+), 49 deletions(-)
>>
>> --
>> 2.30.2
>>