Re: [PATCH 5/7] uprobes: Fix utask->xol_vaddr leak in pre_ssout()

2013-01-10 Thread Srikar Dronamraju
> > >   utask->vaddr = bp_vaddr;
> > >
> > > - return arch_uprobe_pre_xol(>arch, regs);
> > > + err = arch_uprobe_pre_xol(>arch, regs);
> > > + if (unlikely(err)) {
> > > + xol_free_insn_slot(current);
> > > + return err;
> > > + }
> > > +
> > > + return 0;
> > >  }
> >
> > Nit: we could reduce a line or two with
> >
> > err = arch_uprobe_pre_xol(>arch, regs);
> > if (unlikely(err))
> > xol_free_insn_slot(current);
> >
> > return err;
> 
> Yes, but this is also preparation for the next patch which adds more
> code after arch_uprobe_pre_xol() == 0.
> 

Agree

-- 
Thanks and Regards
Srikar

--
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 5/7] uprobes: Fix utask-xol_vaddr leak in pre_ssout()

2013-01-10 Thread Srikar Dronamraju
 utask-vaddr = bp_vaddr;
  
   - return arch_uprobe_pre_xol(uprobe-arch, regs);
   + err = arch_uprobe_pre_xol(uprobe-arch, regs);
   + if (unlikely(err)) {
   + xol_free_insn_slot(current);
   + return err;
   + }
   +
   + return 0;
}
 
  Nit: we could reduce a line or two with
 
  err = arch_uprobe_pre_xol(uprobe-arch, regs);
  if (unlikely(err))
  xol_free_insn_slot(current);
 
  return err;
 
 Yes, but this is also preparation for the next patch which adds more
 code after arch_uprobe_pre_xol() == 0.
 

Agree

-- 
Thanks and Regards
Srikar

--
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 5/7] uprobes: Fix utask->xol_vaddr leak in pre_ssout()

2013-01-08 Thread Oleg Nesterov
On 01/08, Srikar Dronamraju wrote:
>
> * Oleg Nesterov  [2012-12-31 18:52:26]:
>
> > pre_ssout() should do xol_free_insn_slot() if arch_uprobe_pre_xol()
> > fails, otherwise nobody will free the allocated slot.
> >
> > Signed-off-by: Oleg Nesterov 
>
> Acked-by: Srikar Dronamraju 

Thanks!

> (one nit below)
> ...
> > @@ -1316,7 +1317,13 @@ pre_ssout(struct uprobe *uprobe, struct pt_regs 
> > *regs, unsigned long bp_vaddr)
> > utask->xol_vaddr = xol_vaddr;
> > utask->vaddr = bp_vaddr;
> >
> > -   return arch_uprobe_pre_xol(>arch, regs);
> > +   err = arch_uprobe_pre_xol(>arch, regs);
> > +   if (unlikely(err)) {
> > +   xol_free_insn_slot(current);
> > +   return err;
> > +   }
> > +
> > +   return 0;
> >  }
>
> Nit: we could reduce a line or two with
>
>   err = arch_uprobe_pre_xol(>arch, regs);
>   if (unlikely(err))
>   xol_free_insn_slot(current);
>
>   return err;

Yes, but this is also preparation for the next patch which adds more
code after arch_uprobe_pre_xol() == 0.

Oleg.

--
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 5/7] uprobes: Fix utask->xol_vaddr leak in pre_ssout()

2013-01-08 Thread Srikar Dronamraju
* Oleg Nesterov  [2012-12-31 18:52:26]:

> pre_ssout() should do xol_free_insn_slot() if arch_uprobe_pre_xol()
> fails, otherwise nobody will free the allocated slot.
> 
> Signed-off-by: Oleg Nesterov 

Acked-by: Srikar Dronamraju 

(one nit below)
> ---
>  kernel/events/uprobes.c |9 -
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 2ed6239..bd94d2c 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -1306,6 +1306,7 @@ pre_ssout(struct uprobe *uprobe, struct pt_regs *regs, 
> unsigned long bp_vaddr)
>  {
>   struct uprobe_task *utask;
>   unsigned long xol_vaddr;
> + int err;
> 
>   utask = current->utask;
> 
> @@ -1316,7 +1317,13 @@ pre_ssout(struct uprobe *uprobe, struct pt_regs *regs, 
> unsigned long bp_vaddr)
>   utask->xol_vaddr = xol_vaddr;
>   utask->vaddr = bp_vaddr;
> 
> - return arch_uprobe_pre_xol(>arch, regs);
> + err = arch_uprobe_pre_xol(>arch, regs);
> + if (unlikely(err)) {
> + xol_free_insn_slot(current);
> + return err;
> + }
> +
> + return 0;
>  }

Nit: we could reduce a line or two with

err = arch_uprobe_pre_xol(>arch, regs);
if (unlikely(err))
xol_free_insn_slot(current);

return err;
> 
>  /*
> -- 
> 1.5.5.1
> 

--
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 5/7] uprobes: Fix utask-xol_vaddr leak in pre_ssout()

2013-01-08 Thread Srikar Dronamraju
* Oleg Nesterov o...@redhat.com [2012-12-31 18:52:26]:

 pre_ssout() should do xol_free_insn_slot() if arch_uprobe_pre_xol()
 fails, otherwise nobody will free the allocated slot.
 
 Signed-off-by: Oleg Nesterov o...@redhat.com

Acked-by: Srikar Dronamraju sri...@linux.vnet.ibm.com

(one nit below)
 ---
  kernel/events/uprobes.c |9 -
  1 files changed, 8 insertions(+), 1 deletions(-)
 
 diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
 index 2ed6239..bd94d2c 100644
 --- a/kernel/events/uprobes.c
 +++ b/kernel/events/uprobes.c
 @@ -1306,6 +1306,7 @@ pre_ssout(struct uprobe *uprobe, struct pt_regs *regs, 
 unsigned long bp_vaddr)
  {
   struct uprobe_task *utask;
   unsigned long xol_vaddr;
 + int err;
 
   utask = current-utask;
 
 @@ -1316,7 +1317,13 @@ pre_ssout(struct uprobe *uprobe, struct pt_regs *regs, 
 unsigned long bp_vaddr)
   utask-xol_vaddr = xol_vaddr;
   utask-vaddr = bp_vaddr;
 
 - return arch_uprobe_pre_xol(uprobe-arch, regs);
 + err = arch_uprobe_pre_xol(uprobe-arch, regs);
 + if (unlikely(err)) {
 + xol_free_insn_slot(current);
 + return err;
 + }
 +
 + return 0;
  }

Nit: we could reduce a line or two with

err = arch_uprobe_pre_xol(uprobe-arch, regs);
if (unlikely(err))
xol_free_insn_slot(current);

return err;
 
  /*
 -- 
 1.5.5.1
 

--
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 5/7] uprobes: Fix utask-xol_vaddr leak in pre_ssout()

2013-01-08 Thread Oleg Nesterov
On 01/08, Srikar Dronamraju wrote:

 * Oleg Nesterov o...@redhat.com [2012-12-31 18:52:26]:

  pre_ssout() should do xol_free_insn_slot() if arch_uprobe_pre_xol()
  fails, otherwise nobody will free the allocated slot.
 
  Signed-off-by: Oleg Nesterov o...@redhat.com

 Acked-by: Srikar Dronamraju sri...@linux.vnet.ibm.com

Thanks!

 (one nit below)
 ...
  @@ -1316,7 +1317,13 @@ pre_ssout(struct uprobe *uprobe, struct pt_regs 
  *regs, unsigned long bp_vaddr)
  utask-xol_vaddr = xol_vaddr;
  utask-vaddr = bp_vaddr;
 
  -   return arch_uprobe_pre_xol(uprobe-arch, regs);
  +   err = arch_uprobe_pre_xol(uprobe-arch, regs);
  +   if (unlikely(err)) {
  +   xol_free_insn_slot(current);
  +   return err;
  +   }
  +
  +   return 0;
   }

 Nit: we could reduce a line or two with

   err = arch_uprobe_pre_xol(uprobe-arch, regs);
   if (unlikely(err))
   xol_free_insn_slot(current);

   return err;

Yes, but this is also preparation for the next patch which adds more
code after arch_uprobe_pre_xol() == 0.

Oleg.

--
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/


[PATCH 5/7] uprobes: Fix utask->xol_vaddr leak in pre_ssout()

2012-12-31 Thread Oleg Nesterov
pre_ssout() should do xol_free_insn_slot() if arch_uprobe_pre_xol()
fails, otherwise nobody will free the allocated slot.

Signed-off-by: Oleg Nesterov 
---
 kernel/events/uprobes.c |9 -
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 2ed6239..bd94d2c 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1306,6 +1306,7 @@ pre_ssout(struct uprobe *uprobe, struct pt_regs *regs, 
unsigned long bp_vaddr)
 {
struct uprobe_task *utask;
unsigned long xol_vaddr;
+   int err;
 
utask = current->utask;
 
@@ -1316,7 +1317,13 @@ pre_ssout(struct uprobe *uprobe, struct pt_regs *regs, 
unsigned long bp_vaddr)
utask->xol_vaddr = xol_vaddr;
utask->vaddr = bp_vaddr;
 
-   return arch_uprobe_pre_xol(>arch, regs);
+   err = arch_uprobe_pre_xol(>arch, regs);
+   if (unlikely(err)) {
+   xol_free_insn_slot(current);
+   return err;
+   }
+
+   return 0;
 }
 
 /*
-- 
1.5.5.1

--
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/


[PATCH 5/7] uprobes: Fix utask-xol_vaddr leak in pre_ssout()

2012-12-31 Thread Oleg Nesterov
pre_ssout() should do xol_free_insn_slot() if arch_uprobe_pre_xol()
fails, otherwise nobody will free the allocated slot.

Signed-off-by: Oleg Nesterov o...@redhat.com
---
 kernel/events/uprobes.c |9 -
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 2ed6239..bd94d2c 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1306,6 +1306,7 @@ pre_ssout(struct uprobe *uprobe, struct pt_regs *regs, 
unsigned long bp_vaddr)
 {
struct uprobe_task *utask;
unsigned long xol_vaddr;
+   int err;
 
utask = current-utask;
 
@@ -1316,7 +1317,13 @@ pre_ssout(struct uprobe *uprobe, struct pt_regs *regs, 
unsigned long bp_vaddr)
utask-xol_vaddr = xol_vaddr;
utask-vaddr = bp_vaddr;
 
-   return arch_uprobe_pre_xol(uprobe-arch, regs);
+   err = arch_uprobe_pre_xol(uprobe-arch, regs);
+   if (unlikely(err)) {
+   xol_free_insn_slot(current);
+   return err;
+   }
+
+   return 0;
 }
 
 /*
-- 
1.5.5.1

--
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/