lkml.org 
[lkml]   [2022]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] kprobes: Rollback kprobe flags on failed arm_kprobe
Thanks for your quick reply.
I'm very, very sorry for sending multiple emails. I am submitting a
patch for the first time.

On Fri, Jun 3, 2022 at 11:03 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> This should go to stable, so add below tag. (No need to CC to stable)
>
> Fixes: 12310e343755 ("kprobes: Propagate error from arm_kprobe_ftrace()")
> Cc: stable@vger.kernel.org

Thanks for your kind reminder.

>
> And could you also update this patch as below?
>
> > ---
> > kernel/kprobes.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index f214f8c088ed..96c75e23113c 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -2422,8 +2422,10 @@ int enable_kprobe(struct kprobe *kp)
> > if (!kprobes_all_disarmed && kprobe_disabled(p)) {
> > p->flags &= ~KPROBE_FLAG_DISABLED;
> > ret = arm_kprobe(p);
> > - if (ret)
> > + if (ret) {
> > p->flags |= KPROBE_FLAG_DISABLED;
>
> Here, can you add a check?
>
> if (p != kp)
>
> > + kp->flags |= KPROBE_FLAG_DISABLED;

Well, I also thought about it. This already covers conditions `p ==
kp` and `p != kp`:
---
kp->flags |= KPROBE_FLAG_DISABLED;
---

When p is equal to kp, `kp->flags` is assigned to KPROBE_FLAG_DISABLED twice.
However, if you add a check that p is not equal to kp, this makes the
code read more clearly. Anything is OK, what is your suggestion?

>
> Thus is is clear that this is corresponding to
> ---
> if (p != kp)
> kp->flags &= ~KPROBE_FLAG_DISABLED;
> ---
>
> Thank you,
>
> > + }
> > }
> > out:
> > mutex_unlock(&kprobe_mutex);
> > --
> > 2.34.1
> >
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

Have a wonderful day!

\
 
 \ /
  Last update: 2022-06-04 04:17    [W:0.050 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site