diff mbox series

[v2] riscv/kprobe: reclaim insn_slot on kprobe unregistration

Message ID 20220525014424.20717-1-liaochang1@huawei.com (mailing list archive)
State New, archived
Headers show
Series [v2] riscv/kprobe: reclaim insn_slot on kprobe unregistration | expand

Commit Message

Liao, Chang May 25, 2022, 1:44 a.m. UTC
On kprobe registration kernel allocate one insn_slot for new kprobe,
but it forget to reclaim the insn_slot on unregistration, leading to a
potential leakage.

Reported-by: Chen Guokai <chenguokai17@mails.ucas.ac.cn>
Signed-off-by: Liao Chang <liaochang1@huawei.com>
---
v2:
  Add Reported-by tag

 arch/riscv/kernel/probes/kprobes.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Guo Ren May 25, 2022, 6:23 a.m. UTC | #1
On Wed, May 25, 2022 at 9:46 AM Liao Chang <liaochang1@huawei.com> wrote:
>
> On kprobe registration kernel allocate one insn_slot for new kprobe,
> but it forget to reclaim the insn_slot on unregistration, leading to a
> potential leakage.
>
> Reported-by: Chen Guokai <chenguokai17@mails.ucas.ac.cn>
> Signed-off-by: Liao Chang <liaochang1@huawei.com>
> ---
> v2:
>   Add Reported-by tag
>
>  arch/riscv/kernel/probes/kprobes.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
> index e6e950b7cf32..f12eb1fbb52c 100644
> --- a/arch/riscv/kernel/probes/kprobes.c
> +++ b/arch/riscv/kernel/probes/kprobes.c
> @@ -110,6 +110,10 @@ void __kprobes arch_disarm_kprobe(struct kprobe *p)
>
>  void __kprobes arch_remove_kprobe(struct kprobe *p)
>  {
> +       if (p->ainsn.api.insn) {
> +               free_insn_slot(p->ainsn.api.insn, 0);
> +               p->ainsn.api.insn = NULL;
> +       }
Thx reviewed-by: Guo Ren <guoren@kernel.org>

You also could give a fixup patch to csky, thx:

diff --git a/arch/csky/kernel/probes/kprobes.c
b/arch/csky/kernel/probes/kprobes.c
index 42920f25e73c..661da54b418f 100644
--- a/arch/csky/kernel/probes/kprobes.c
+++ b/arch/csky/kernel/probes/kprobes.c
@@ -124,6 +124,10 @@ void __kprobes arch_disarm_kprobe(struct kprobe *p)

 void __kprobes arch_remove_kprobe(struct kprobe *p)
 {
+       if (p->ainsn.api.insn) {
+               free_insn_slot(p->ainsn.api.insn, 0);
+               p->ainsn.api.insn = NULL;
+       }
 }

 static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)

>  }
>
>  static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)
> --
> 2.17.1
>
Liao, Chang May 25, 2022, 7:59 a.m. UTC | #2
在 2022/5/25 14:23, Guo Ren 写道:
> On Wed, May 25, 2022 at 9:46 AM Liao Chang <liaochang1@huawei.com> wrote:
>>
>> On kprobe registration kernel allocate one insn_slot for new kprobe,
>> but it forget to reclaim the insn_slot on unregistration, leading to a
>> potential leakage.
>>
>> Reported-by: Chen Guokai <chenguokai17@mails.ucas.ac.cn>
>> Signed-off-by: Liao Chang <liaochang1@huawei.com>
>> ---
>> v2:
>>   Add Reported-by tag
>>
>>  arch/riscv/kernel/probes/kprobes.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
>> index e6e950b7cf32..f12eb1fbb52c 100644
>> --- a/arch/riscv/kernel/probes/kprobes.c
>> +++ b/arch/riscv/kernel/probes/kprobes.c
>> @@ -110,6 +110,10 @@ void __kprobes arch_disarm_kprobe(struct kprobe *p)
>>
>>  void __kprobes arch_remove_kprobe(struct kprobe *p)
>>  {
>> +       if (p->ainsn.api.insn) {
>> +               free_insn_slot(p->ainsn.api.insn, 0);
>> +               p->ainsn.api.insn = NULL;
>> +       }
> Thx reviewed-by: Guo Ren <guoren@kernel.org>
> 
> You also could give a fixup patch to csky, thx:
> 
> diff --git a/arch/csky/kernel/probes/kprobes.c
> b/arch/csky/kernel/probes/kprobes.c
> index 42920f25e73c..661da54b418f 100644
> --- a/arch/csky/kernel/probes/kprobes.c
> +++ b/arch/csky/kernel/probes/kprobes.c
> @@ -124,6 +124,10 @@ void __kprobes arch_disarm_kprobe(struct kprobe *p)
> 
>  void __kprobes arch_remove_kprobe(struct kprobe *p)
>  {
> +       if (p->ainsn.api.insn) {
> +               free_insn_slot(p->ainsn.api.insn, 0);
> +               p->ainsn.api.insn = NULL;
> +       }
>  }

Sure, I will, thanks.
> 
>  static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)
> 
>>  }
>>
>>  static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)
>> --
>> 2.17.1
>>
> 
>
Jisheng Zhang May 25, 2022, 1:33 p.m. UTC | #3
On Wed, May 25, 2022 at 02:23:50PM +0800, Guo Ren wrote:
> On Wed, May 25, 2022 at 9:46 AM Liao Chang <liaochang1@huawei.com> wrote:
> >
> > On kprobe registration kernel allocate one insn_slot for new kprobe,
> > but it forget to reclaim the insn_slot on unregistration, leading to a
> > potential leakage.
> >
> > Reported-by: Chen Guokai <chenguokai17@mails.ucas.ac.cn>
> > Signed-off-by: Liao Chang <liaochang1@huawei.com>

Reviewed-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> > v2:
> >   Add Reported-by tag
> >
> >  arch/riscv/kernel/probes/kprobes.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
> > index e6e950b7cf32..f12eb1fbb52c 100644
> > --- a/arch/riscv/kernel/probes/kprobes.c
> > +++ b/arch/riscv/kernel/probes/kprobes.c
> > @@ -110,6 +110,10 @@ void __kprobes arch_disarm_kprobe(struct kprobe *p)
> >
> >  void __kprobes arch_remove_kprobe(struct kprobe *p)
> >  {
> > +       if (p->ainsn.api.insn) {
> > +               free_insn_slot(p->ainsn.api.insn, 0);
> > +               p->ainsn.api.insn = NULL;
> > +       }
> Thx reviewed-by: Guo Ren <guoren@kernel.org>
> 
> You also could give a fixup patch to csky, thx:
> 
> diff --git a/arch/csky/kernel/probes/kprobes.c
> b/arch/csky/kernel/probes/kprobes.c
> index 42920f25e73c..661da54b418f 100644
> --- a/arch/csky/kernel/probes/kprobes.c
> +++ b/arch/csky/kernel/probes/kprobes.c
> @@ -124,6 +124,10 @@ void __kprobes arch_disarm_kprobe(struct kprobe *p)
> 
>  void __kprobes arch_remove_kprobe(struct kprobe *p)
>  {
> +       if (p->ainsn.api.insn) {
> +               free_insn_slot(p->ainsn.api.insn, 0);
> +               p->ainsn.api.insn = NULL;
> +       }
>  }
> 
>  static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)
> 
> >  }
> >
> >  static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)
> > --
> > 2.17.1
> >
> 
> 
> -- 
> Best Regards
>  Guo Ren
> 
> ML: https://lore.kernel.org/linux-csky/
Masami Hiramatsu (Google) May 26, 2022, 3:33 p.m. UTC | #4
On Wed, 25 May 2022 09:44:24 +0800
Liao Chang <liaochang1@huawei.com> wrote:

> On kprobe registration kernel allocate one insn_slot for new kprobe,
> but it forget to reclaim the insn_slot on unregistration, leading to a
> potential leakage.
> 
> Reported-by: Chen Guokai <chenguokai17@mails.ucas.ac.cn>
> Signed-off-by: Liao Chang <liaochang1@huawei.com>

Looks good to me.

Fixes: c22b0bcb1dd0 ("riscv: Add kprobes supported")
Cc: stable@vger.kernel.org
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you,


> ---
> v2:
>   Add Reported-by tag
> 
>  arch/riscv/kernel/probes/kprobes.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
> index e6e950b7cf32..f12eb1fbb52c 100644
> --- a/arch/riscv/kernel/probes/kprobes.c
> +++ b/arch/riscv/kernel/probes/kprobes.c
> @@ -110,6 +110,10 @@ void __kprobes arch_disarm_kprobe(struct kprobe *p)
>  
>  void __kprobes arch_remove_kprobe(struct kprobe *p)
>  {
> +	if (p->ainsn.api.insn) {
> +		free_insn_slot(p->ainsn.api.insn, 0);
> +		p->ainsn.api.insn = NULL;
> +	}
>  }
>  
>  static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)
> -- 
> 2.17.1
>
diff mbox series

Patch

diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
index e6e950b7cf32..f12eb1fbb52c 100644
--- a/arch/riscv/kernel/probes/kprobes.c
+++ b/arch/riscv/kernel/probes/kprobes.c
@@ -110,6 +110,10 @@  void __kprobes arch_disarm_kprobe(struct kprobe *p)
 
 void __kprobes arch_remove_kprobe(struct kprobe *p)
 {
+	if (p->ainsn.api.insn) {
+		free_insn_slot(p->ainsn.api.insn, 0);
+		p->ainsn.api.insn = NULL;
+	}
 }
 
 static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)